feat!: improve how we handle changing email addresses and users becoming unverified when account linking requires verification

CHANGELOG.md

@@ -12,7 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Breaking changes
 
 -   Defined the entry points of the library using the "exports" field in package.json to make ESM imports more comfortable. This can cause some issues for applications using directory imports from the `lib/build` directory. In those cases we recommend adding `index.js` to the import path.
-
+-   `isEmailChangeAllowed` now returns false for unverified addresses if input user is a primary user and there exists another user with the same email address and linking requires verification
+-   Generating a password reset token is now denied if all of the following is true:
+    -   a linked email password user exists
+    -   the email address is not verified
+    -   the user has another email address or phone number associated with it
+-   Account linking based on emails now require the email to be verified in both users if `shouldRequireVerification` is set to `true` instead of only requiring it for the recipe user.
 -   The access token cookie expiry has been changed from 100 years to 1 year due to some browsers capping the maximum expiry at 400 days. No action is needed on your part.
 
 ### Changes

lib/build/recipe/accountlinking/index.js

@@ -129,13 +129,14 @@ class Wrapper {
     }
     static async isEmailChangeAllowed(recipeUserId, newEmail, isVerified, session, userContext) {
         const user = await __1.getUser(recipeUserId.getAsString(), userContext);
-        return await recipe_1.default.getInstance().isEmailChangeAllowed({
+        const res = await recipe_1.default.getInstance().isEmailChangeAllowed({
             user,
             newEmail,
             isVerified,
             session,
             userContext: utils_1.getUserContext(userContext),
         });
+        return res.allowed;
     }
 }
 exports.default = Wrapper;

lib/build/recipe/accountlinking/recipe.d.ts

@@ -104,7 +104,15 @@ export default class Recipe extends RecipeModule {
         isVerified: boolean;
         session: SessionContainerInterface | undefined;
         userContext: UserContext;
-    }) => Promise<boolean>;
+    }) => Promise<
+        | {
+              allowed: true;
+          }
+        | {
+              allowed: false;
+              reason: "PRIMARY_USER_CONFLICT" | "ACCOUNT_TAKEOVER_RISK";
+          }
+    >;
     verifyEmailForRecipeUserIfLinkedAccountsAreVerified: (input: {
         user: User;
         recipeUserId: RecipeUserId;

lib/build/recipe/accountlinking/recipe.js

@@ -384,52 +384,90 @@ class Recipe extends recipeModule_1.default {
              * on the link by mistake, causing account linking to happen which can result
              * in account take over if this recipe user is malicious.
              */
-            let user = input.user;
-            if (user === undefined) {
+            let inputUser = input.user;
+            if (inputUser === undefined) {
                 throw new Error("Passed in recipe user id does not exist");
             }
-            for (const tenantId of user.tenantIds) {
+            for (const tenantId of inputUser.tenantIds) {
                 let existingUsersWithNewEmail = await this.recipeInterfaceImpl.listUsersByAccountInfo({
-                    tenantId: user.tenantIds[0],
+                    tenantId: inputUser.tenantIds[0],
                     accountInfo: {
                         email: input.newEmail,
                     },
                     doUnionOfAccountInfo: false,
                     userContext: input.userContext,
                 });
-                let primaryUserForNewEmail = existingUsersWithNewEmail.filter((u) => u.isPrimaryUser);
-                if (primaryUserForNewEmail.length > 1) {
+                let otherUsersWithNewEmail = existingUsersWithNewEmail.filter((u) => u.id !== inputUser.id);
+                let otherPrimaryUserForNewEmail = otherUsersWithNewEmail.filter((u) => u.isPrimaryUser);
+                if (otherPrimaryUserForNewEmail.length > 1) {
                     throw new Error("You found a bug. Please report it on github.com/supertokens/supertokens-node");
                 }
-                if (user.isPrimaryUser) {
+                if (inputUser.isPrimaryUser) {
                     // this is condition one from the above comment.
-                    if (primaryUserForNewEmail.length === 1 && primaryUserForNewEmail[0].id !== user.id) {
+                    if (otherPrimaryUserForNewEmail.length !== 0) {
                         logger_1.logDebugMessage(
-                            "isEmailChangeAllowed: returning false cause email change will lead to two primary users having same email"
+                            `isEmailChangeAllowed: returning false cause email change will lead to two primary users having same email on ${tenantId}`
                         );
-                        return false;
+                        return { allowed: false, reason: "PRIMARY_USER_CONFLICT" };
+                    }
+                    if (input.isVerified) {
+                        logger_1.logDebugMessage(
+                            `isEmailChangeAllowed: can change on ${tenantId} cause input user is primary, new email is verified and doesn't belong to any other primary user`
+                        );
+                        continue;
+                    }
+                    if (inputUser.loginMethods.some((lm) => lm.hasSameEmailAs(input.newEmail) && lm.verified)) {
+                        logger_1.logDebugMessage(
+                            `isEmailChangeAllowed: can change on ${tenantId} cause input user is primary, new email is verified in another login method and doesn't belong to any other primary user`
+                        );
+                        continue;
+                    }
+                    if (otherUsersWithNewEmail.length === 0) {
+                        logger_1.logDebugMessage(
+                            `isEmailChangeAllowed: can change on ${tenantId} cause input user is primary and the new email doesn't belong to any other user (primary or non-primary)`
+                        );
+                        continue;
+                    }
+                    const shouldDoAccountLinking = await this.config.shouldDoAutomaticAccountLinking(
+                        otherUsersWithNewEmail[0].loginMethods[0],
+                        inputUser,
+                        input.session,
+                        tenantId,
+                        input.userContext
+                    );
+                    if (!shouldDoAccountLinking.shouldAutomaticallyLink) {
+                        logger_1.logDebugMessage(
+                            `isEmailChangeAllowed: can change on ${tenantId} cause linking is disabled`
+                        );
+                        continue;
+                    }
+                    if (!shouldDoAccountLinking.shouldRequireVerification) {
+                        logger_1.logDebugMessage(
+                            `isEmailChangeAllowed: can change on ${tenantId} cause linking is doesn't require email verification`
+                        );
+                        continue;
                     }
                     logger_1.logDebugMessage(
-                        `isEmailChangeAllowed: can change on ${tenantId} cause input user is primary and new email doesn't belong to any other primary user`
+                        `isEmailChangeAllowed: returning false because the user hasn't verified the new email address and there exists another user with it on ${tenantId} and linking requires verification`
                     );
-                    continue;
+                    return { allowed: false, reason: "ACCOUNT_TAKEOVER_RISK" };
                 } else {
                     if (input.isVerified) {
                         logger_1.logDebugMessage(
                             `isEmailChangeAllowed: can change on ${tenantId} cause input user is not a primary and new email is verified`
                         );
                         continue;
                     }
-                    if (user.loginMethods[0].hasSameEmailAs(input.newEmail)) {
+                    if (inputUser.loginMethods[0].hasSameEmailAs(input.newEmail)) {
                         logger_1.logDebugMessage(
                             `isEmailChangeAllowed: can change on ${tenantId} cause input user is not a primary and new email is same as the older one`
                         );
                         continue;
                     }
-                    if (primaryUserForNewEmail.length === 1) {
+                    if (otherPrimaryUserForNewEmail.length === 1) {
                         let shouldDoAccountLinking = await this.config.shouldDoAutomaticAccountLinking(
-                            user.loginMethods[0],
-                            primaryUserForNewEmail[0],
+                            inputUser.loginMethods[0],
+                            otherPrimaryUserForNewEmail[0],
                             input.session,
                             tenantId,
                             input.userContext
@@ -449,7 +487,7 @@ class Recipe extends recipeModule_1.default {
                         logger_1.logDebugMessage(
                             "isEmailChangeAllowed: returning false cause input user is not a primary there exists a primary user exists with the new email."
                         );
-                        return false;
+                        return { allowed: false, reason: "ACCOUNT_TAKEOVER_RISK" };
                     }
                     logger_1.logDebugMessage(
                         `isEmailChangeAllowed: can change on ${tenantId} cause input user is not a primary no primary user exists with the new email`
@@ -460,7 +498,7 @@ class Recipe extends recipeModule_1.default {
             logger_1.logDebugMessage(
                 "isEmailChangeAllowed: returning true cause email change can happen on all tenants the user is part of"
             );
-            return true;
+            return { allowed: true };
         };
         this.verifyEmailForRecipeUserIfLinkedAccountsAreVerified = async (input) => {
             try {
@@ -651,9 +689,20 @@ class Recipe extends recipeModule_1.default {
                         );
                         return { status: "NO_LINK" };
                     }
-                    if (shouldDoAccountLinking.shouldRequireVerification && !inputUser.loginMethods[0].verified) {
+                    const accountInfoVerifiedInPrimUser = primaryUserThatCanBeLinkedToTheInputUser.loginMethods.some(
+                        (lm) =>
+                            (inputUser.loginMethods[0].email !== undefined &&
+                                lm.hasSameEmailAs(inputUser.loginMethods[0].email)) ||
+                            (inputUser.loginMethods[0].phoneNumber !== undefined &&
+                                lm.hasSamePhoneNumberAs(inputUser.loginMethods[0].phoneNumber) &&
+                                lm.verified)
+                    );
+                    if (
+                        shouldDoAccountLinking.shouldRequireVerification &&
+                        (!inputUser.loginMethods[0].verified || !accountInfoVerifiedInPrimUser)
+                    ) {
                         logger_1.logDebugMessage(
-                            "tryLinkingByAccountInfoOrCreatePrimaryUser: not linking because shouldRequireVerification is true but the login method is not verified"
+                            "tryLinkingByAccountInfoOrCreatePrimaryUser: not linking because shouldRequireVerification is true but the login method is not verified in the new or the primary user"
                         );
                         return { status: "NO_LINK" };
                     }

lib/build/recipe/emailpassword/api/implementation.js

@@ -122,6 +122,31 @@ function getAPIImplementation() {
                     emailPasswordAccount.recipeUserId
                 );
             }
+            // Next we check if there is any login method in which the input email is verified.
+            // If that is the case, then it's proven that the user owns the email and we can
+            // trust linking of the email password account.
+            let emailVerified =
+                primaryUserAssociatedWithEmail.loginMethods.find((lm) => {
+                    return lm.hasSameEmailAs(email) && lm.verified;
+                }) !== undefined;
+            // finally, we check if the primary user has any other email / phone number
+            // associated with this account - and if it does, then it means that
+            // there is a risk of account takeover, so we do not allow the token to be generated
+            let hasOtherEmailOrPhone =
+                primaryUserAssociatedWithEmail.loginMethods.find((lm) => {
+                    // we do the extra undefined check below cause
+                    // hasSameEmailAs returns false if the lm.email is undefined, and
+                    // we want to check that the email is different as opposed to email
+                    // not existing in lm.
+                    return (lm.email !== undefined && !lm.hasSameEmailAs(email)) || lm.phoneNumber !== undefined;
+                }) !== undefined;
+            if (!emailVerified && hasOtherEmailOrPhone) {
+                return {
+                    status: "PASSWORD_RESET_NOT_ALLOWED",
+                    reason:
+                        "Reset password link was not created because of account take over risk. Please contact support. (ERR_CODE_001)",
+                };
+            }
             let shouldDoAccountLinkingResponse = await recipe_1.default
                 .getInstance()
                 .config.shouldDoAutomaticAccountLinking(
@@ -228,51 +253,10 @@ function getAPIImplementation() {
                     emailPasswordAccount.recipeUserId
                 );
             }
-            // Now we start the required security checks. First we check if the primary user
-            // it has just one linked account. And if that's true, then we continue
-            // cause then there is no scope for account takeover
-            if (primaryUserAssociatedWithEmail.loginMethods.length === 1) {
-                return await generateAndSendPasswordResetToken(
-                    primaryUserAssociatedWithEmail.id,
-                    emailPasswordAccount.recipeUserId
-                );
-            }
-            // Next we check if there is any login method in which the input email is verified.
-            // If that is the case, then it's proven that the user owns the email and we can
-            // trust linking of the email password account.
-            let emailVerified =
-                primaryUserAssociatedWithEmail.loginMethods.find((lm) => {
-                    return lm.hasSameEmailAs(email) && lm.verified;
-                }) !== undefined;
-            if (emailVerified) {
-                return await generateAndSendPasswordResetToken(
-                    primaryUserAssociatedWithEmail.id,
-                    emailPasswordAccount.recipeUserId
-                );
-            }
-            // finally, we check if the primary user has any other email / phone number
-            // associated with this account - and if it does, then it means that
-            // there is a risk of account takeover, so we do not allow the token to be generated
-            let hasOtherEmailOrPhone =
-                primaryUserAssociatedWithEmail.loginMethods.find((lm) => {
-                    // we do the extra undefined check below cause
-                    // hasSameEmailAs returns false if the lm.email is undefined, and
-                    // we want to check that the email is different as opposed to email
-                    // not existing in lm.
-                    return (lm.email !== undefined && !lm.hasSameEmailAs(email)) || lm.phoneNumber !== undefined;
-                }) !== undefined;
-            if (hasOtherEmailOrPhone) {
-                return {
-                    status: "PASSWORD_RESET_NOT_ALLOWED",
-                    reason:
-                        "Reset password link was not created because of account take over risk. Please contact support. (ERR_CODE_001)",
-                };
-            } else {
-                return await generateAndSendPasswordResetToken(
-                    primaryUserAssociatedWithEmail.id,
-                    emailPasswordAccount.recipeUserId
-                );
-            }
+            return await generateAndSendPasswordResetToken(
+                primaryUserAssociatedWithEmail.id,
+                emailPasswordAccount.recipeUserId
+            );
         },
         passwordResetPOST: async function ({ formFields, token, tenantId, options, userContext }) {
             async function markEmailAsVerified(recipeUserId, email) {

lib/build/recipe/emailpassword/recipeImplementation.js

@@ -6,6 +6,7 @@ var __importDefault =
     };
 Object.defineProperty(exports, "__esModule", { value: true });
 const recipe_1 = __importDefault(require("../accountlinking/recipe"));
+const recipe_2 = __importDefault(require("../emailverification/recipe"));
 const normalisedURLPath_1 = __importDefault(require("../../normalisedURLPath"));
 const __1 = require("../..");
 const constants_1 = require("./constants");
@@ -160,6 +161,38 @@ function getRecipeInterface(querier, getEmailPasswordConfig) {
             );
         },
         updateEmailOrPassword: async function (input) {
+            const accountLinking = recipe_1.default.getInstance();
+            if (input.email) {
+                const user = await __1.getUser(input.recipeUserId.getAsString(), input.userContext);
+                if (user === undefined) {
+                    return { status: "UNKNOWN_USER_ID_ERROR" };
+                }
+                const evInstance = recipe_2.default.getInstance();
+                let isEmailVerified = false;
+                if (evInstance) {
+                    isEmailVerified = await evInstance.recipeInterfaceImpl.isEmailVerified({
+                        recipeUserId: input.recipeUserId,
+                        email: input.email,
+                        userContext: input.userContext,
+                    });
+                }
+                const isEmailChangeAllowed = await accountLinking.isEmailChangeAllowed({
+                    user,
+                    isVerified: isEmailVerified,
+                    newEmail: input.email,
+                    session: undefined,
+                    userContext: input.userContext,
+                });
+                if (!isEmailChangeAllowed.allowed) {
+                    return {
+                        status: "EMAIL_CHANGE_NOT_ALLOWED_ERROR",
+                        reason:
+                            isEmailChangeAllowed.reason === "ACCOUNT_TAKEOVER_RISK"
+                                ? "New email cannot be applied to existing account because of account takeover risks."
+                                : "New email cannot be applied to existing account because of there is another primary user with the same email address.",
+                    };
+                }
+            }
             if (input.applyPasswordPolicy || input.applyPasswordPolicy === undefined) {
                 let formFields = getEmailPasswordConfig().signUpFeature.formFields;
                 if (input.password !== undefined) {