Repository navigation
Conversation
- Templates stored in the database were passed to the renderer as a filename, so any body longer than the filesystem name limit threw ENAMETOOLONG. Password signup failed this way on the welcome email. - Update validation excludes a document from unique checks by an id in the body, which clients don't send, so saving a user with an unchanged email reported the address as taken. The user routes now supply it. - The login screen sent "password" and "authChannel" to /1/auth/otp/send, which rejects both, breaking AUTH_TYPE=code.
API ChangesNo changes. |
The softUnique validator excludes a document by an id in the request body, which clients don't send on updates, so a user collided with their own row when saving an unchanged email or phone. This was handled by a middleware wired into the two user update routes by hand. Move it to the middleware that already resolves the target document (fetchByParam, fetchByParamWithSlug, isSelf) so every update route gets it without opting in, and any model that later gains a unique field is covered.
kaareal
commented
Sep 23, 2026
Comment on lines
+44
to
+52
| // Unique checks in update validation exclude the target document by an id in | ||
| // the body, which clients have no reason to send. Take it from the document the | ||
| // route already resolved. Update validation strips it again before assign. | ||
| function excludeFromUniqueChecks(ctx, doc) { | ||
| if (ctx.method === 'PATCH' || ctx.method === 'PUT') { | ||
| ctx.request.body.id = doc.id; | ||
| } | ||
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
@andrewplummer can you take a look to see if this best solution
The problem is that right, if you try to patch users/me
You get this, this because the patch doesnt set the id in the body.
if there is no id in the body the uniqueness check triggers and fails
| // Unique checks in update validation exclude the target document by an id in | ||
| // the body, which clients have no reason to send. Take it from the document the | ||
| // route already resolved. Update validation strips it again before assign. | ||
| function excludeFromUniqueChecks(ctx, doc) { |
Collaborator
There was a problem hiding this comment.
not sure I understand this
Collaborator
Author
There was a problem hiding this comment.
The copy is bad ?
Or you dont understand the problem ?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three bugs found while testing the auth flows in the UI. Each one is reproducible on
mastertoday. Every fix has a test that fails without it.renderTemplatepassed a body loaded from the database to the renderer as a filename. Withdirset the renderer resolves it as a path, and anything longer than the filesystem name limit throwsENAMETOOLONG. The welcome template is a database template, so every password signup hit it. The account was created first, so it looked half-broken.dironly when the template really is a file.idin the body, and no client sends one. Any save that included the unchanged email failed, which blocked role editing in the admin UI.AUTH_TYPE=codelogin fails with "Unknown field password"password, and sent the channel asauthChannel.email,typeandchannel.Why the tests didn't catch these
The template test used a short body, which stays under the name limit and falls back to the literal string. The signup test passes because no database template exists in the test database, so the file is used. The new test uses a long multi-line body like the real templates.
Verified in the browser
Signup, admin user edit, role assignment and the full code-login flow (request, emailed code, confirm), plus the API and web suites.