Devs calling updateEmailOrPassword function or manuallyCreateOrUpdateUser (third party recipe) function do not get the security checks associated with the isEmailChangeAllowed function. They have to remember to call this themselves, which, they can forget.
The security checks for producing ERR_CODE_001 is not properly implemented:
1) Create email/password account with email - e1@test.com and verified it.
2) Create social account via Google with email - e1@test.com. Both accounts are now linked.
3) Change email/password account email to e2@test.com.
4) I can still reset password for e2@test.com. It’s also strange that when resetting the password for e2@test.com, this email address is not verified.
[ ] Change isEmailChangeAllowed to check (along with the current checks) if email verification is required for linking and if:
if the input recipe user id is a primary user; and
the new email is unverified across login methods; and
there exists another user (has to be a recipe user id, not linked to the input user) with the same email (verified or unverified)
then we deny the email change.
This fixes ACCT1 and ACCT2
[ ] We update the recipe functions updateEmailOrPassword and manuallyCreateOrUpdateUser to call isEmailChangeAllowed. The output of these functions should not need to change since they already return that EMAIL_CHANGE_NOT_ALLOWED output.
This implies that updateEmailOrPassword will need to compute - isEmailVerified, which it can by calling the core to check the email verification status of for the new email and recipeUserId combination (there can be a case where the user is changing the email back to an older one which at some point had been verified).
[ ] Logic for if account linking is allowed should take into account the email verification status of both sides in case email verification is required. This means that if new account has email A, we check for if that is verified AND if any of the login methods for the primary user has email A which is verified.
We need to see what happens in case of existing flows. This can maybe be done via running all tests.
Since this is not 100% required to fix the identified scenarios (ACCT1 and ACCT2), we will do this depending on the scope of implied changes.
Flows like the following should still hold:
attacker signs up with email e2 using ep, unverified cause e2 belongs to the victim.
users tries and signs up google with email e2 and is the primary user.
we will prevent google sign up cause if we allowed it, then the attacker can send an email verification email, which if the user clicks on by mistake, the two accounts will be linked, causing the attacker to get access to the user's account.
[ ] Password reset token generation API should reject if the email is unverified, and is linked to login methods which have different emails OR (the same email but is also unverified). Can be the same error code as OO1.
[ ] Remove this if statement. It has no effect, since if primaryUserAssociatedWithEmail.loginMethods.length === 1 is true, then below, hasOtherEmailOrPhone must be false.
Other TODOs:
[ ] Need to check docs to make sure all the described scenarios of the error codes are valid.
[ ] In docs, need to remove call to isEmailChangeAllowed in the update email API docs.
[ ] This test should continue to work without any change.
Summary of issue
updateEmailOrPasswordfunction ormanuallyCreateOrUpdateUser(third party recipe) function do not get the security checks associated with theisEmailChangeAllowedfunction. They have to remember to call this themselves, which, they can forget.ERR_CODE_001is not properly implemented:Implementation fixes
isEmailChangeAllowedto check (along with the current checks) if email verification is required for linking and if:isEmailVerified, which it can by calling the core to check the email verification status of for the new email and recipeUserId combination (there can be a case where the user is changing the email back to an older one which at some point had been verified).A, we check for if that is verified AND if any of the login methods for the primary user has emailAwhich is verified.primaryUserAssociatedWithEmail.loginMethods.length === 1is true, then below,hasOtherEmailOrPhonemust befalse.Other TODOs:
isEmailChangeAllowedin the update email API docs.