Skip to content

Link and ensure actions conflict, which disallows some users from signing in#539

Merged
bheesham merged 11 commits into
mozilla-iam:masterfrom
bheesham:link-and-ensure-are-fighting
Jun 10, 2026
Merged

Link and ensure actions conflict, which disallows some users from signing in#539
bheesham merged 11 commits into
mozilla-iam:masterfrom
bheesham:link-and-ensure-are-fighting

Conversation

@bheesham

@bheesham bheesham commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

This fixes an edge case where a user could have had 3 identities (or more!). This situation is possible because of the ordering of actions.

When an @mozilla.com user signs in with a non-LDAP (e.g. FxA, Google, GitHub), we error out and ask them to use LDAP instead. You can repeat this process to create more unlinked accounts. Example:

  1. User signs in with FxA -- asked to sign in with LDAP instead (accounts: 1)
  2. User signs in with Google -- asked to sign in with LDAP instead (accounts: 2)
  3. User signs in with LDAP -- code merge assumption broken, and we bail.

@gcoxmoz worked on this fixup the bad assumption we were making, and added support to link as many related users as we've found.

Jira: IAM-1761

Comment thread tf/variables.tf Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an Auth0 Actions ordering/linking edge case where a single person can accumulate 3+ unlinked identities (e.g., FxA + Google + LDAP), causing the LDAP sign-in “merge assumption” to break and block login.

Changes:

  • Reorders the action flow so linkUserByEmail runs before ensureLdapUsersUseLdap (dev + prod).
  • Updates linkUserByEmail to (a) only consider verified-email accounts and (b) link multiple secondary accounts into a single primary (preferring LDAP when present).
  • Extends unit tests to cover linking scenarios with 2+ accounts (including non-LDAP-only cases), and adds local dev tooling config via mise.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tf/variables.tf Reorders Actions execution so account linking runs before LDAP-enforcement.
tf/actions/linkUserByEmail.js Implements multi-account linking logic and filters to verified-email profiles.
tf/tests/linkUserByEmail.test.js Updates/extends tests for new multi-link behavior and non-LDAP linking.
mise.toml Adds Biome + pnpm tooling configuration via mise settings.
mise.lock Locks mise tool versions for reproducible tool installs.
.github/workflows/test-rules.yaml Runs Jest with --silent to reduce console noise in CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tf/actions/linkUserByEmail.js
Comment thread tf/actions/linkUserByEmail.js
Comment thread tf/actions/linkUserByEmail.js Outdated
Comment thread tf/actions/linkUserByEmail.js Outdated
Comment thread tf/actions/linkUserByEmail.js Outdated
Comment thread tf/tests/linkUserByEmail.test.js
Comment thread mise.toml Outdated
Comment on lines +3 to +7
"npm:@biomejs/biome" = "2.4.16"
pnpm = "latest"

[settings]
npm.package_manager = "pnpm"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repo supports both Node 18 and Node 22. The pnpm version we want supports only Node 22. So for now, there's a bit of drift here.

Comment thread tf/actions/linkUserByEmail.js
@bheesham bheesham force-pushed the link-and-ensure-are-fighting branch 3 times, most recently from 2ba4150 to ac4cb5e Compare June 8, 2026 14:26
@bheesham bheesham force-pushed the link-and-ensure-are-fighting branch from ac4cb5e to 2a1b2e3 Compare June 8, 2026 14:29
bheesham added 4 commits June 8, 2026 16:24
It's easier to test on the objects we have instead of log lines we'll
emit.

Testing on those log lines is made slightly more complicated because of
how JS objects and references to them work. It can be fixed by using
deep clones. But, it's easier to use the objects directly instead.

Jira: IAM-1761
Jira: IAM-1761
@bheesham bheesham force-pushed the link-and-ensure-are-fighting branch from 2a1b2e3 to 61bf4ca Compare June 8, 2026 20:24

@gcoxmoz gcoxmoz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little self-serving that I am approving this, since I wrote the first-pass draft.
I consider it only fair to approve since Bhee did the heavy lifting of sorting out making it work, without once hitting me up with a "bro, do you even js?"

@bheesham

bheesham commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author
Plan for dev
Terraform will perform the following actions:

  # auth0_action.linkUserByEmail will be updated in-place
  ~ resource "auth0_action" "linkUserByEmail" {
      ~ code       = <<-EOT
            /**
             * @title Link Accounts with Same Email Address while Merging Metadata
             * @overview Link any accounts that have the same email address while merging metadata.
             * @gallery true
             * @category access control
             *
             * This rule will link any accounts that have the same email address while merging metadata.
             * Source/Original: https://github.com/auth0/rules/blob/master/src/rules/link-users-by-email-with-metadata.js
             *
             * Please see https://github.com/mozilla-iam/mozilla-iam/blob/master/docs/deratcheting-user-flows.md#user-logs-in-with-the-mozilla-iam-system-for-the-first-time
             * for detailed explanation of what happens here.
             *
             */

            const auth0Sdk = require("auth0");

            exports.onExecutePostLogin = async (event, api) => {
              console.log("Running actions:", "linkUsersByEmail");

              // Check if email is verified, we shouldn't automatically
              // merge accounts if this is not the case.
              if (!event.user.email || !event.user.email_verified) {
                return;
              }

              const mgmtAuth0Domain =
                event.tenant.id === "dev"
                  ? "dev.mozilla-dev.auth0.com"
                  : "auth.mozilla.auth0.com";

              // Create an Auth0 Management API Client
              const ManagementClient = auth0Sdk.ManagementClient;
              const mgmtClient = new ManagementClient({
                domain: mgmtAuth0Domain,
                clientId: event.secrets.mgmtClientId,
                clientSecret: event.secrets.mgmtClientSecret,
                scope: "update:users",
              });

              // Since email addresses within auth0 are allowed to be mixed case and the /user-by-email search endpoint
              // is case sensitive, we need to search for both situations.  In the first search we search by "this" users email
              // which might be mixed case (or not).  Our second search is for the lowercase equivalent but only if two searches
              // would be different.
          +   //
          +   // This code only ever cares about accounts with verified email addresses.
              const searchMultipleEmailCases = async () => {
          -     let userAccountsFound = [];
          +     const userAccountsFound = [];

          -     // Push the
          +     // Push the results for the email as specified.
                userAccountsFound.push(
                  mgmtClient.usersByEmail.getByEmail({ email: event.user.email })
                );

                // if this user is mixed case, we need to also search for the lower case equivalent
                if (event.user.email !== event.user.email.toLowerCase()) {
                  userAccountsFound.push(
                    mgmtClient.usersByEmail.getByEmail({
                      email: event.user.email.toLowerCase(),
                    })
                  );
                }

                // await all json responses promises to resolve
                const allJSONResponses = await Promise.all(userAccountsFound);

                // flatten the array of arrays to get one array of profiles
                const mergedDataProfiles = allJSONResponses.reduce((acc, response) => {
                  return acc.concat(response.data);
                }, []);

          -     return mergedDataProfiles;
          +     return mergedDataProfiles.filter((u) => u.email_verified === true);
              };

          -   const linkAccount = async (otherProfile) => {
          -     // sanity check if both accounts have LDAP as primary
          -     // we should NOT link these accounts and simply allow the user to continue logging in.
          -     if (
          -       event.user.user_id.startsWith("ad|Mozilla-LDAP") &&
          -       otherProfile.user_id.startsWith("ad|Mozilla-LDAP")
          -     ) {
          -       console.error(
          -         `Error: both ${event.user.user_id} and ${otherProfile.user_id} are LDAP Primary accounts. Linking will not occur.`
          -       );
          -       return; // Continue with user login without account linking
          -     }
          -
          -     // LDAP takes priority being the primary identity
          -     // So we need to determine if one or neither are LDAP
          -     // If both are non-primary, linking order doesn't matter
          -     let primaryUser;
          -     let secondaryUser;
          -
          -     if (event.user.user_id.startsWith("ad|Mozilla-LDAP")) {
          -       primaryUser = event.user;
          -       secondaryUser = otherProfile;
          -     } else {
          -       primaryUser = otherProfile;
          -       secondaryUser = event.user;
          -     }
          -
          +   const linkAccount = async (primaryUser, secondaryUser) => {
                // Link the secondary account into the primary account                                                                console.log(
                  `Linking secondary identity ${secondaryUser.user_id} into primary identity ${primaryUser.user_id}`
                );

          -     // We no longer keep the user_metadata nor app_metadata from the secondary account
          -     // that is being linked.  If the primary account is LDAP, then its existing
          -     // metadata should prevail.  And in the case of both, primary and secondary being
          -     // non-ldap, account priority does not matter and neither does the metadata of
          -     // the secondary account.
          -
          -     // Link the accounts
                try {
                  await mgmtClient.users.link(
                    { id: String(primaryUser.user_id) },
                    {
                      provider: secondaryUser.identities[0].provider,
                      user_id: secondaryUser.identities[0].user_id,
                    }
                  );

                  // Auth0 Action api object provides a method for updating the current
                  // authenticated user to the new user_id after account linking has taken place
                  api.authentication.setPrimaryUser(primaryUser.user_id);
                } catch (err) {
                  console.error("An unknown error occurred while linking accounts:", err);
                  throw err;
                }

                return;
              };

              // Main
              try {
                // Search for multiple accounts of the same user to link
          -     let userAccountList = await searchMultipleEmailCases();
          +     const userAccountList = await searchMultipleEmailCases();

          -     // Ignore non-verified users
          -     userAccountList = userAccountList.filter((u) => u.email_verified);
          -
                if (userAccountList.length <= 1) {
                  // The user logged in with an identity which is the only one Auth0 knows about
                  // or no data returned
                  // Do not perform any account linking
                  return;
                }

          -     if (userAccountList.length === 2) {
          -       // Auth0 is aware of 2 identities with the same email address which means
          -       // that the user just logged in with a new identity that hasn't been linked
          -       // into the other existing identity.  Here we pass the other account to the
          -       // linking function
          +     // We found multiple user accounts.  Let's break it down:
          +     const userAccountListLdapList = userAccountList.filter((u) =>
          +       u.user_id.startsWith("ad|Mozilla-LDAP")
          +     );

          -       await linkAccount(
          -         userAccountList.filter((u) => u.user_id !== event.user.user_id)[0]
          +     // There should never be more than one LDAP account found with the same name.
          +     // If there is, we wouldn't know what to do in automation, so log it.
          +     if (userAccountListLdapList.length > 1) {
          +       const userstring = userAccountListLdapList
          +         .map((u) => u.user_id)
          +         .join(" ");
          +       console.error(
          +         `Error: ${userstring} are LDAP Primary accounts. Linking will not occur.`
                  );
          +       return; // Continue with user login without account linking
          +     }
          +
          +     // While it's unlikely we'll ever need to merge many accounts in one go, 2024-2026 proved
          +     // we can absolutely get ourselves into such a situation, so let's fix that.  Allow
          +     // multiple merges into one main profile.
          +     let mainProfile;
          +     let mergeList;
          +     if (userAccountListLdapList.length === 1) {
          +       // There is only one LDAP profile, so merge the non-LDAP users into the LDAP user.
          +       // This should be an obvious decision.
          +       // We do not keep the user_metadata or app_metadata from the secondary account(s).
          +       // The LDAP account's metadata should prevail.
          +       mainProfile = userAccountListLdapList[0];
          +       mergeList = userAccountList.filter(
          +         (u) => !u.user_id.startsWith("ad|Mozilla-LDAP")
          +       );
          +     } else if (userAccountListLdapList.length === 0) {
          +       // There is no LDAP profile, so merge into the other users into the current event's user.
          +       // Think about this one a bit.  Clearly if there was an LDAP user, we'd prefer that.
          +       //
          +       // But here, you have a kind of 'now what?' path.   Do you:
          +       // * merge an older user into the current-event user?  You risk losing any metadata
          +       // from an older user.
          +       // * merge the current user into an older user?  You basically prefer the first user
          +       // identity that was ever found, and never move off of it.
          +       //
          +       // Honestly, there's no good choice here.  Each has pitfalls.
          +       // CAUTION / DEBT?
          +       // The merge of two non-LDAPs could be a long-lurking bug: if both accounts have groups
          +       // on PMO, that completely desyncs from PMO and then nothing good will happen.
          +       mainProfile = event.user;
          +       mergeList = userAccountList.filter(
          +         (u) => u.user_id !== event.user.user_id
          +       );
                } else {
          -       // data.length is > 2 which, post November 2020 when all identities were
          -       // force linked manually, shouldn't be possible
          -       var error_message =
          -         `Error linking account ${event.user.user_id} as there are ` +
          -         `over 2 identities with the email address ${event.user.email} ` +
          -         userAccountList.map((x) => x.user_id).join();
          +       // This is unnecessary but wards off any mistaken fallthroughs.
          +       const error_message = "Impossible Default Case reached";
                  console.error(error_message);
                  throw new Error(error_message);
                }
          +
          +     for (const mergeProfile of mergeList) {
          +       await linkAccount(mainProfile, mergeProfile);
          +     }
              } catch (err) {
                console.error("An error occurred while linking accounts:", err);
                return api.access.deny(err.message || String(err));
              }

              return;
            };
        EOT
        id         = "441f971f-8ca7-451a-921d-1c031a3a2179"
        name       = "linkUserByEmail"
        # (3 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
Plan for prod
Terraform will perform the following actions:

  # auth0_action.linkUserByEmail will be updated in-place
  ~ resource "auth0_action" "linkUserByEmail" {
      ~ code       = <<-EOT
            /**
             * @title Link Accounts with Same Email Address while Merging Metadata
             * @overview Link any accounts that have the same email address while merging metadata.
             * @gallery true
             * @category access control
             *
             * This rule will link any accounts that have the same email address while merging metadata.
             * Source/Original: https://github.com/auth0/rules/blob/master/src/rules/link-users-by-email-with-metadata.js
             *
             * Please see https://github.com/mozilla-iam/mozilla-iam/blob/master/docs/deratcheting-user-flows.md#user-logs-in-with-the-mozilla-iam-system-for-the-first-time
             * for detailed explanation of what happens here.
             *
             */

            const auth0Sdk = require("auth0");
            exports.onExecutePostLogin = async (event, api) => {
              console.log("Running actions:", "linkUsersByEmail");

              // Check if email is verified, we shouldn't automatically
              // merge accounts if this is not the case.
              if (!event.user.email || !event.user.email_verified) {
                return;
              }

              const mgmtAuth0Domain =
                event.tenant.id === "dev"
                  ? "dev.mozilla-dev.auth0.com"
                  : "auth.mozilla.auth0.com";

              // Create an Auth0 Management API Client
              const ManagementClient = auth0Sdk.ManagementClient;
              const mgmtClient = new ManagementClient({
                domain: mgmtAuth0Domain,
                clientId: event.secrets.mgmtClientId,
                clientSecret: event.secrets.mgmtClientSecret,
                scope: "update:users",
              });

              // Since email addresses within auth0 are allowed to be mixed case and the /user-by-email search endpoint
              // is case sensitive, we need to search for both situations.  In the first search we search by "this" users email
              // which might be mixed case (or not).  Our second search is for the lowercase equivalent but only if two searches
              // would be different.
          +   //
          +   // This code only ever cares about accounts with verified email addresses.
              const searchMultipleEmailCases = async () => {
          -     let userAccountsFound = [];
          +     const userAccountsFound = [];

          -     // Push the
          +     // Push the results for the email as specified.
                userAccountsFound.push(
                  mgmtClient.usersByEmail.getByEmail({ email: event.user.email })
                );

                // if this user is mixed case, we need to also search for the lower case equivalent
                if (event.user.email !== event.user.email.toLowerCase()) {
                  userAccountsFound.push(
                    mgmtClient.usersByEmail.getByEmail({
                      email: event.user.email.toLowerCase(),
                    })
                  );
                }

                // await all json responses promises to resolve
                const allJSONResponses = await Promise.all(userAccountsFound);

                // flatten the array of arrays to get one array of profiles
                const mergedDataProfiles = allJSONResponses.reduce((acc, response) => {
                  return acc.concat(response.data);
                }, []);

          -     return mergedDataProfiles;
          +     return mergedDataProfiles.filter((u) => u.email_verified === true);
              };

          -   const linkAccount = async (otherProfile) => {
          -     // sanity check if both accounts have LDAP as primary
          -     // we should NOT link these accounts and simply allow the user to continue logging in.
          -     if (
          -       event.user.user_id.startsWith("ad|Mozilla-LDAP") &&
          -       otherProfile.user_id.startsWith("ad|Mozilla-LDAP")
          -     ) {
          -       console.error(
          -         `Error: both ${event.user.user_id} and ${otherProfile.user_id} are LDAP Primary accounts. Linking will not occur.`
          -       );
          -       return; // Continue with user login without account linking
          -     }
          -
          -     // LDAP takes priority being the primary identity
          -     // So we need to determine if one or neither are LDAP
          -     // If both are non-primary, linking order doesn't matter
          -     let primaryUser;
          -     let secondaryUser;
          -
          -     if (event.user.user_id.startsWith("ad|Mozilla-LDAP")) {
          -       primaryUser = event.user;
          -       secondaryUser = otherProfile;
          -     } else {
          -       primaryUser = otherProfile;
          -       secondaryUser = event.user;
          -     }
          -
          +   const linkAccount = async (primaryUser, secondaryUser) => {
                // Link the secondary account into the primary account
                console.log(
                  `Linking secondary identity ${secondaryUser.user_id} into primary identity ${primaryUser.user_id}`
                );

          -     // We no longer keep the user_metadata nor app_metadata from the secondary account
          -     // that is being linked.  If the primary account is LDAP, then its existing
          -     // metadata should prevail.  And in the case of both, primary and secondary being
          -     // non-ldap, account priority does not matter and neither does the metadata of
          -     // the secondary account.
          -
          -     // Link the accounts
                try {
                  await mgmtClient.users.link(
                    { id: String(primaryUser.user_id) },
                    {
                      provider: secondaryUser.identities[0].provider,
                      user_id: secondaryUser.identities[0].user_id,
                    }
                  );

                  // Auth0 Action api object provides a method for updating the current
                  // authenticated user to the new user_id after account linking has taken place
                  api.authentication.setPrimaryUser(primaryUser.user_id);
                } catch (err) {
                  console.error("An unknown error occurred while linking accounts:", err);
                  throw err;
                }

                return;
              };

              // Main
              try {
                // Search for multiple accounts of the same user to link
          -     let userAccountList = await searchMultipleEmailCases();
          +     const userAccountList = await searchMultipleEmailCases();

          -     // Ignore non-verified users
          -     userAccountList = userAccountList.filter((u) => u.email_verified);
          -
                if (userAccountList.length <= 1) {
                  // The user logged in with an identity which is the only one Auth0 knows about
                  // or no data returned
                  // Do not perform any account linking
                  return;
                }

          -     if (userAccountList.length === 2) {
          -       // Auth0 is aware of 2 identities with the same email address which means
          -       // that the user just logged in with a new identity that hasn't been linked
          -       // into the other existing identity.  Here we pass the other account to the
          -       // linking function
          +     // We found multiple user accounts.  Let's break it down:
          +     const userAccountListLdapList = userAccountList.filter((u) =>
          +       u.user_id.startsWith("ad|Mozilla-LDAP")
          +     );

          -       await linkAccount(
          -         userAccountList.filter((u) => u.user_id !== event.user.user_id)[0]
          +     // There should never be more than one LDAP account found with the same name.
          +     // If there is, we wouldn't know what to do in automation, so log it.
          +     if (userAccountListLdapList.length > 1) {
          +       const userstring = userAccountListLdapList
          +         .map((u) => u.user_id)
          +         .join(" ");
          +       console.error(
          +         `Error: ${userstring} are LDAP Primary accounts. Linking will not occur.`
                  );
          +       return; // Continue with user login without account linking
          +     }
          +
          +     // While it's unlikely we'll ever need to merge many accounts in one go, 2024-2026 proved
          +     // we can absolutely get ourselves into such a situation, so let's fix that.  Allow
          +     // multiple merges into one main profile.
          +     let mainProfile;
          +     let mergeList;
          +     if (userAccountListLdapList.length === 1) {
          +       // There is only one LDAP profile, so merge the non-LDAP users into the LDAP user.
          +       // This should be an obvious decision.
          +       // We do not keep the user_metadata or app_metadata from the secondary account(s).
          +       // The LDAP account's metadata should prevail.
          +       mainProfile = userAccountListLdapList[0];
          +       mergeList = userAccountList.filter(
          +         (u) => !u.user_id.startsWith("ad|Mozilla-LDAP")
          +       );
          +     } else if (userAccountListLdapList.length === 0) {
          +       // There is no LDAP profile, so merge into the other users into the current event's user.
          +       // Think about this one a bit.  Clearly if there was an LDAP user, we'd prefer that.
          +       //
          +       // But here, you have a kind of 'now what?' path.   Do you:
          +       // * merge an older user into the current-event user?  You risk losing any metadata
          +       // from an older user.
          +       // * merge the current user into an older user?  You basically prefer the first user
          +       // identity that was ever found, and never move off of it.
          +       //
          +       // Honestly, there's no good choice here.  Each has pitfalls.
          +       // CAUTION / DEBT?
          +       // The merge of two non-LDAPs could be a long-lurking bug: if both accounts have groups
          +       // on PMO, that completely desyncs from PMO and then nothing good will happen.
          +       mainProfile = event.user;
          +       mergeList = userAccountList.filter(
          +         (u) => u.user_id !== event.user.user_id
          +       );
                } else {
          -       // data.length is > 2 which, post November 2020 when all identities were
          -       // force linked manually, shouldn't be possible
          -       var error_message =
          -         `Error linking account ${event.user.user_id} as there are ` +
          -         `over 2 identities with the email address ${event.user.email} ` +
          -         userAccountList.map((x) => x.user_id).join();
          +       // This is unnecessary but wards off any mistaken fallthroughs.
          +       const error_message = "Impossible Default Case reached";
                  console.error(error_message);
                  throw new Error(error_message);
                }
          +
          +     for (const mergeProfile of mergeList) {
          +       await linkAccount(mainProfile, mergeProfile);
          +     }
              } catch (err) {
                console.error("An error occurred while linking accounts:", err);
                return api.access.deny(err.message || String(err));
              }

              return;
            };
        EOT
        id         = "e6215dbe-13e1-4d5c-a2d8-592677b7f827"
        name       = "linkUserByEmail"
        # (3 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

@bheesham bheesham merged commit 2fff335 into mozilla-iam:master Jun 10, 2026
3 checks passed
@bheesham bheesham deleted the link-and-ensure-are-fighting branch June 16, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants