Adds allowedUserFields option to define the only data that can be returned to client by dbAuth functions
#9374
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.
This is the result of a report to our security email. dbAuth has a function
_sanitizeUser()which removes sensitive fields from being returned to the client when calling functions likeloginorforgotPassword. It removes fields likehashedPasswordandsalt. However, this is a deny list, not an allow list, so if you added your own potentially sensitive fields, they could be sent along in responses, depending on what data you chose to return from those functions.This PR reverses the strategy and instead makes you list the only fields that are allowed to be sent back. This will be a breaking change in two ways:
allowedUserFieldsoption is not defined we'll assume["id", "email"]by default, which is probably the smallest list that most people are using. So the majority of users can install this release, make no code changes, and be fine. But, if people were counting on additional fields being returned by default, they now won't be, thus the breaking-ness.forgotPasswordhandler has changed, adding a second argument. See the forgotPassword Handler Signature section below.The config set in
api/src/functions/auth.jswill now look something like this (comments removed for clarity):const forgotPasswordOptions = { - handler: async (user) => { + handler: async (user, resetToken) => { return user }, expires: 60 * 60 * 24, errors: { usernameNotFound: 'Username not found', usernameRequired: 'Username is required', }, // more config... const authHandler = new DbAuthHandler(event, context, { db: db, + allowedUserFields: ["id", "email"], authModelAccessor: 'user', authFields: { id: 'id', username: 'email', hashedPassword: 'hashedPassword', salt: 'salt', resetToken: 'resetToken', resetTokenExpiresAt: 'resetTokenExpiresAt', }, cookie: { HttpOnly: true, Path: '/', SameSite: 'Strict', Secure: process.env.NODE_ENV !== 'development', }, forgotPassword: forgotPasswordOptions, login: loginOptions, resetPassword: resetPasswordOptions, signup: signupOptions, })These new options have been added to the template for the file that's created when running
yarn rw setup auth dbAuthso new apps will get these options automatically.forgotPasswordHandler SignatureThis PR includes an argument change to the
forgotPasswordhandler. Previously, a singleuserobject was passed which would contain theresetTokento be emailed to the user. Since we're now removing everything that isn't in theallowedUserFieldslist, this value would have been stripped out and not made available without some hacky workarounds. Instead, it's now sent in a second argument to keep theuserobject "pure," and a comment has been added to the template explaining how to use it.Codemod?
I don't think we can really codemod the new
allowedUserFieldsoption because it will depend on which fields you may or may not have decided to return in those functions. We could simply add theallowedUserFields: ["id", "email"],line, but it will now strip out any custom fields you may have been returning, and this would be unexpected: the spirit of codemods is "this will do all the work for you, don't worry" but in this case that's not really possible.We could codemod the new
resetTokenargument, although what you did with it inside of that function could be harder. We could assume that any previous usage was ofuser.resetTokenand just change it toresetToken, but if they did anything else special with it the codemod wouldn't know what to do there.I propose we just make a thorough Release Notes entry that explains the minimal changes to make.
TODO