From 489dd0f28e4a289e0d54133698263d5c496d66e8 Mon Sep 17 00:00:00 2001 From: Greg Cox Date: Wed, 27 May 2026 03:53:48 +0000 Subject: [PATCH 01/11] Comment update within linkUserByEmail.js --- tf/actions/linkUserByEmail.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tf/actions/linkUserByEmail.js b/tf/actions/linkUserByEmail.js index 4177ead..2d77617 100644 --- a/tf/actions/linkUserByEmail.js +++ b/tf/actions/linkUserByEmail.js @@ -70,8 +70,10 @@ exports.onExecutePostLogin = async (event, api) => { }; 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. + // sanity check if the logging-in account and the found account are both LDAP. + // We should never link two LDAP accounts. + // Also, this should never happen. Like, why do we have two LDAPs and a collision? + // Log the event, 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") @@ -101,11 +103,12 @@ exports.onExecutePostLogin = async (event, api) => { `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 + // CAUTION / DEBT? + // We do not keep the user_metadata or 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. + // metadata should prevail. However, 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. // Link the accounts try { From e18bc385789fc93295f8b5c3e959731bcbcf1580 Mon Sep 17 00:00:00 2001 From: Greg Cox Date: Wed, 27 May 2026 04:06:33 +0000 Subject: [PATCH 02/11] Correct/enhance tests for linkUserByEmail.js --- tf/tests/linkUserByEmail.test.js | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/tf/tests/linkUserByEmail.test.js b/tf/tests/linkUserByEmail.test.js index e9e6ad8..ab83bdd 100644 --- a/tf/tests/linkUserByEmail.test.js +++ b/tf/tests/linkUserByEmail.test.js @@ -135,7 +135,7 @@ test("If one usersByEmail is found, expect no account linking", async () => { expect(api.authentication.setPrimaryUser).not.toHaveBeenCalled(); }); -test("If two usersByEmail is found, expect account linking", async () => { +test("If two usersByEmail are found, expect account linking when one is LDAP", async () => { // Define user profiles to return with usersByEmail.getByEmail users = [_ldapUser, _emailUser]; _event.user = _ldapUser; @@ -146,7 +146,30 @@ test("If two usersByEmail is found, expect account linking", async () => { // Execute onExecutePostLogin await onExecutePostLogin(_event, api); - // Expect linking not to have been called + // Expect linking to have been called + expect(mockManagementClient.users.link).toHaveBeenCalled(); + expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); + + // Collect console logs and expect searchString to exist in logs + const combinedLogs = combineLog(consoleLogSpy.mock.calls); + const searchString = `Linking secondary identity ${_emailUser.user_id} into primary identity ${_event.user.user_id}`; + expect(combinedLogs.some((element) => element.includes(searchString))).toBe( + true + ); +}); + +test("If two usersByEmail are found, expect account linking even when neither are LDAP", async () => { + // Define user profiles to return with usersByEmail.getByEmail + users = [_emailUser, _firefoxaccountUser]; + _event.user = _emailUser; + + // Mock implementation of ManagementClient.usersByEmail.getByEmail() + mockManagementClient.usersByEmail.getByEmail.mockImplementation(getByEmail); + + // Execute onExecutePostLogin + await onExecutePostLogin(_event, api); + + // Expect linking to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); From 8c61b287c18ea8fd9cd8dad60fc44148b998c6b6 Mon Sep 17 00:00:00 2001 From: Greg Cox Date: Wed, 27 May 2026 06:33:02 +0000 Subject: [PATCH 03/11] Rewrite linkUserByEmail.js to not be limited by the account-found count --- tf/actions/linkUserByEmail.js | 103 +++++++++++++++++----------------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/tf/actions/linkUserByEmail.js b/tf/actions/linkUserByEmail.js index 2d77617..10424d5 100644 --- a/tf/actions/linkUserByEmail.js +++ b/tf/actions/linkUserByEmail.js @@ -69,48 +69,12 @@ exports.onExecutePostLogin = async (event, api) => { return mergedDataProfiles; }; - const linkAccount = async (otherProfile) => { - // sanity check if the logging-in account and the found account are both LDAP. - // We should never link two LDAP accounts. - // Also, this should never happen. Like, why do we have two LDAPs and a collision? - // Log the event, 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}` ); - // CAUTION / DEBT? - // We do not keep the user_metadata or app_metadata from the secondary account - // that is being linked. If the primary account is LDAP, then its existing - // metadata should prevail. However, 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. - - // Link the accounts try { await mgmtClient.users.link( { id: String(primaryUser.user_id) }, @@ -146,25 +110,64 @@ exports.onExecutePostLogin = async (event, api) => { 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: + var 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) { + var 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. + var error_message = "Impossible Default Case reached" console.error(error_message); throw new Error(error_message); } + + for (var 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)); From 3a9c6a311b0511c3d6403acd59734df2b6db0b97 Mon Sep 17 00:00:00 2001 From: Greg Cox Date: Wed, 27 May 2026 06:34:21 +0000 Subject: [PATCH 04/11] Move linkUserByEmail as an action before ensureLdapUsersUseLdap --- tf/variables.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tf/variables.tf b/tf/variables.tf index 7677910..429e329 100644 --- a/tf/variables.tf +++ b/tf/variables.tf @@ -4,9 +4,9 @@ variable "action_flow_dev" { "continueEndPoint", "samlMappings", "gheGroups", + "linkUserByEmail", "ensureLdapUsersUseLdap", "accessRules", - "linkUserByEmail", "activateNewUsersInCIS", "OIDCConformanceWorkaround", "configurationDumper" @@ -19,10 +19,10 @@ variable "action_flow_prod" { "continueEndPoint", "samlMappings", "gheGroups", + "linkUserByEmail", "ensureLdapUsersUseLdap", "accessRules", "awsSaml", - "linkUserByEmail", "activateNewUsersInCIS", "OIDCConformanceWorkaround" ] From c8c1fea2809c1e0cb9b4846e85ee38919a356a57 Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Wed, 27 May 2026 17:21:54 -0400 Subject: [PATCH 05/11] We only ever care about verified emails, so search for those too Jira: IAM-1761 --- tf/actions/linkUserByEmail.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tf/actions/linkUserByEmail.js b/tf/actions/linkUserByEmail.js index 10424d5..3f5aba8 100644 --- a/tf/actions/linkUserByEmail.js +++ b/tf/actions/linkUserByEmail.js @@ -41,10 +41,12 @@ exports.onExecutePostLogin = async (event, api) => { // 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 }) ); @@ -66,7 +68,7 @@ exports.onExecutePostLogin = async (event, api) => { return acc.concat(response.data); }, []); - return mergedDataProfiles; + return mergedDataProfiles.filter((u) => u.email_verified === true); }; const linkAccount = async (primaryUser, secondaryUser) => { @@ -98,10 +100,7 @@ exports.onExecutePostLogin = async (event, api) => { // Main try { // Search for multiple accounts of the same user to link - let userAccountList = await searchMultipleEmailCases(); - - // Ignore non-verified users - userAccountList = userAccountList.filter((u) => u.email_verified); + const userAccountList = await searchMultipleEmailCases(); if (userAccountList.length <= 1) { // The user logged in with an identity which is the only one Auth0 knows about From 2cd238f217d0a675dcbd87b0e15e081b536196ef Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Mon, 1 Jun 2026 13:39:49 -0400 Subject: [PATCH 06/11] Address biome lints for linkUserByEmail.js Jira: IAM-1761 --- tf/actions/linkUserByEmail.js | 13 +++++-------- tf/tests/linkUserByEmail.test.js | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/tf/actions/linkUserByEmail.js b/tf/actions/linkUserByEmail.js index 3f5aba8..a5b2d74 100644 --- a/tf/actions/linkUserByEmail.js +++ b/tf/actions/linkUserByEmail.js @@ -110,12 +110,12 @@ exports.onExecutePostLogin = async (event, api) => { } // We found multiple user accounts. Let's break it down: - var userAccountListLdapList = userAccountList.filter((u) => u.user_id.startsWith("ad|Mozilla-LDAP")) + const userAccountListLdapList = userAccountList.filter((u) => u.user_id.startsWith("ad|Mozilla-LDAP")) // 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) { - var userstring = userAccountListLdapList.map(u => u.user_id).join(' '); + const userstring = userAccountListLdapList.map(u => u.user_id).join(' '); console.error( `Error: ${userstring} are LDAP Primary accounts. Linking will not occur.` ); @@ -156,16 +156,13 @@ exports.onExecutePostLogin = async (event, api) => { } else { // This is unnecessary but wards off any mistaken fallthroughs. - var error_message = "Impossible Default Case reached" + const error_message = "Impossible Default Case reached" console.error(error_message); throw new Error(error_message); } - for (var mergeProfile of mergeList) { - await linkAccount( - mainProfile, - mergeProfile, - ); + for (const mergeProfile of mergeList) { + await linkAccount(mainProfile, mergeProfile); } } catch (err) { console.error("An error occurred while linking accounts:", err); diff --git a/tf/tests/linkUserByEmail.test.js b/tf/tests/linkUserByEmail.test.js index ab83bdd..a1e77c3 100644 --- a/tf/tests/linkUserByEmail.test.js +++ b/tf/tests/linkUserByEmail.test.js @@ -14,7 +14,7 @@ jest.mock("auth0"); // Take all log enteries and combine them into a single array const combineLog = (consoleLogs) => { - let combinedLog = []; + const combinedLog = []; for (let i = 0; i < consoleLogs.length; i++) { singleStr = consoleLogs[i].join(" "); combinedLog.push(singleStr); @@ -61,7 +61,7 @@ beforeEach(() => { // Mock sendUserTo api.redirect.sendUserTo.mockImplementation((uri) => { - _event.transaction["redirect_uri"] = uri; + _event.transaction.redirect_uri = uri; }); // Mock implementation of ManagementClient From 673e98da6f6828508ed7b010ab197fae13621502 Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Mon, 8 Jun 2026 09:33:55 -0400 Subject: [PATCH 07/11] linkUserByEmail: fixup tests to use objects instead of console logs 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 --- tf/tests/linkUserByEmail.test.js | 62 ++++++-------------------------- 1 file changed, 10 insertions(+), 52 deletions(-) diff --git a/tf/tests/linkUserByEmail.test.js b/tf/tests/linkUserByEmail.test.js index a1e77c3..a1880e5 100644 --- a/tf/tests/linkUserByEmail.test.js +++ b/tf/tests/linkUserByEmail.test.js @@ -12,16 +12,6 @@ const { onExecutePostLogin } = require("../actions/linkUserByEmail.js"); // Mock auth0 module jest.mock("auth0"); -// Take all log enteries and combine them into a single array -const combineLog = (consoleLogs) => { - const combinedLog = []; - for (let i = 0; i < consoleLogs.length; i++) { - singleStr = consoleLogs[i].join(" "); - combinedLog.push(singleStr); - } - return combinedLog; -}; - beforeEach(() => { // Clone the event object to be used before each test _event = _.cloneDeep(eventObj); @@ -84,19 +74,11 @@ beforeEach(() => { // Mock the constructor of ManagementClient to return the mocked instance auth0Sdk.ManagementClient = jest.fn(() => mockManagementClient); - - // Spy on console - consoleLogSpy = jest.spyOn(console, "log").mockImplementation(() => {}); - consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); - consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); }); afterEach(() => { // Clean up after each test jest.clearAllMocks(); - consoleLogSpy.mockRestore(); - consoleWarnSpy.mockRestore(); - consoleErrorSpy.mockRestore(); }); test("Expect onExecutePostLogin to be defined", async () => { @@ -138,7 +120,7 @@ test("If one usersByEmail is found, expect no account linking", async () => { test("If two usersByEmail are found, expect account linking when one is LDAP", async () => { // Define user profiles to return with usersByEmail.getByEmail users = [_ldapUser, _emailUser]; - _event.user = _ldapUser; + _event.user = _.cloneDeep(_ldapUser); // Mock implementation of ManagementClient.usersByEmail.getByEmail() mockManagementClient.usersByEmail.getByEmail.mockImplementation(getByEmail); @@ -149,19 +131,13 @@ test("If two usersByEmail are found, expect account linking when one is LDAP", a // Expect linking to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); - - // Collect console logs and expect searchString to exist in logs - const combinedLogs = combineLog(consoleLogSpy.mock.calls); - const searchString = `Linking secondary identity ${_emailUser.user_id} into primary identity ${_event.user.user_id}`; - expect(combinedLogs.some((element) => element.includes(searchString))).toBe( - true - ); + expect(_event.user.user_id).toEqual(_ldapUser.user_id); }); test("If two usersByEmail are found, expect account linking even when neither are LDAP", async () => { // Define user profiles to return with usersByEmail.getByEmail users = [_emailUser, _firefoxaccountUser]; - _event.user = _emailUser; + _event.user = _.cloneDeep(_emailUser); // Mock implementation of ManagementClient.usersByEmail.getByEmail() mockManagementClient.usersByEmail.getByEmail.mockImplementation(getByEmail); @@ -172,16 +148,10 @@ test("If two usersByEmail are found, expect account linking even when neither ar // Expect linking to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); - - // Collect console logs and expect searchString to exist in logs - const combinedLogs = combineLog(consoleLogSpy.mock.calls); - const searchString = `Linking secondary identity ${_emailUser.user_id} into primary identity ${_event.user.user_id}`; - expect(combinedLogs.some((element) => element.includes(searchString))).toBe( - true - ); + expect(_event.user.user_id).toEqual(_emailUser.user_id); }); -test("If more than 2 usersByEmail is found, expect error", async () => { +test("If more than 2 usersByEmail is found, expect account linking", async () => { // Define user profiles to return with usersByEmail.getByEmail users = [_ldapUser, _emailUser, _firefoxaccountUser]; _event.user = _ldapUser; @@ -192,16 +162,11 @@ test("If more than 2 usersByEmail is found, expect error", async () => { // Execute onExecutePostLogin await onExecutePostLogin(_event, api); - // Expect linking not to have been called - expect(mockManagementClient.users.link).not.toHaveBeenCalled(); - expect(api.authentication.setPrimaryUser).not.toHaveBeenCalled(); - - // Collect console logs and expect searchString to exist in logs - const combinedErrorLogs = combineLog(consoleErrorSpy.mock.calls); - const searchString = `Error linking account ${_event.user.user_id} as there are over 2 identities`; - expect( - combinedErrorLogs.some((element) => element.startsWith(searchString)) - ).toBe(true); + // Expect linking to have been called + expect(mockManagementClient.users.link).toHaveBeenCalledTimes(2); + expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(2); + // The user_id should be the LDAP account. + expect(_event.user.user_id).toEqual(_ldapUser.user_id); }); test("Test for email of different case, expect account linking", async () => { @@ -256,11 +221,4 @@ test("Test attempting to link 2 LDAP accounts, expect error", async () => { // Expect linking not to have been called expect(mockManagementClient.users.link).not.toHaveBeenCalled(); expect(api.authentication.setPrimaryUser).not.toHaveBeenCalled(); - - // Collect console logs and expect searchString to exist in logs - const combinedErrorLogs = combineLog(consoleErrorSpy.mock.calls); - const searchString = `Error: both ${_event.user.user_id} and ${_firefoxaccountUser.user_id} are LDAP Primary accounts. Linking will not occur.`; - expect( - combinedErrorLogs.some((element) => element.includes(searchString)) - ).toBe(true); }); From ec2df1b5806861e1f63855560c3f78a51359e2e8 Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Mon, 8 Jun 2026 09:47:38 -0400 Subject: [PATCH 08/11] ci: don't print console logs in CI Jira: IAM-1761 --- .github/workflows/test-rules.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-rules.yaml b/.github/workflows/test-rules.yaml index 247944b..6fac2e7 100644 --- a/.github/workflows/test-rules.yaml +++ b/.github/workflows/test-rules.yaml @@ -22,4 +22,4 @@ jobs: - name: Install dependencies run: npm ci - name: Test Rules - run: npm run tests + run: npm run tests -- --silent From e766f3a1fdc1815bfef095162d211ee01f748dc8 Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Mon, 8 Jun 2026 10:02:51 -0400 Subject: [PATCH 09/11] Revert "Move linkUserByEmail as an action before ensureLdapUsersUseLdap" This reverts commit 3a9c6a311b0511c3d6403acd59734df2b6db0b97. --- tf/variables.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tf/variables.tf b/tf/variables.tf index 429e329..7677910 100644 --- a/tf/variables.tf +++ b/tf/variables.tf @@ -4,9 +4,9 @@ variable "action_flow_dev" { "continueEndPoint", "samlMappings", "gheGroups", - "linkUserByEmail", "ensureLdapUsersUseLdap", "accessRules", + "linkUserByEmail", "activateNewUsersInCIS", "OIDCConformanceWorkaround", "configurationDumper" @@ -19,10 +19,10 @@ variable "action_flow_prod" { "continueEndPoint", "samlMappings", "gheGroups", - "linkUserByEmail", "ensureLdapUsersUseLdap", "accessRules", "awsSaml", + "linkUserByEmail", "activateNewUsersInCIS", "OIDCConformanceWorkaround" ] From 61bf4ca0cd0e0140e5a9e4648ec09c678eec5100 Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Mon, 8 Jun 2026 10:06:53 -0400 Subject: [PATCH 10/11] chore: run formatter Jira: IAM-1761 --- tf/actions/linkUserByEmail.js | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tf/actions/linkUserByEmail.js b/tf/actions/linkUserByEmail.js index a5b2d74..b4fedaf 100644 --- a/tf/actions/linkUserByEmail.js +++ b/tf/actions/linkUserByEmail.js @@ -110,12 +110,16 @@ exports.onExecutePostLogin = async (event, api) => { } // We found multiple user accounts. Let's break it down: - const userAccountListLdapList = userAccountList.filter((u) => u.user_id.startsWith("ad|Mozilla-LDAP")) + const userAccountListLdapList = userAccountList.filter((u) => + u.user_id.startsWith("ad|Mozilla-LDAP") + ); // 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(' '); + const userstring = userAccountListLdapList + .map((u) => u.user_id) + .join(" "); console.error( `Error: ${userstring} are LDAP Primary accounts. Linking will not occur.` ); @@ -128,16 +132,15 @@ exports.onExecutePostLogin = async (event, api) => { 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")) - + 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. // @@ -151,12 +154,13 @@ exports.onExecutePostLogin = async (event, api) => { // 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) - + mainProfile = event.user; + mergeList = userAccountList.filter( + (u) => u.user_id !== event.user.user_id + ); } else { // This is unnecessary but wards off any mistaken fallthroughs. - const error_message = "Impossible Default Case reached" + const error_message = "Impossible Default Case reached"; console.error(error_message); throw new Error(error_message); } From 4277119be0790bcee4861608adc8f1e6b34f16db Mon Sep 17 00:00:00 2001 From: Bheesham Persaud Date: Wed, 10 Jun 2026 10:19:38 -0400 Subject: [PATCH 11/11] link: we can only call setPrimaryUser once per transaction Jira: IAM-1761 --- tf/actions/linkUserByEmail.js | 10 ++++++---- tf/tests/linkUserByEmail.test.js | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/tf/actions/linkUserByEmail.js b/tf/actions/linkUserByEmail.js index b4fedaf..fb7a71b 100644 --- a/tf/actions/linkUserByEmail.js +++ b/tf/actions/linkUserByEmail.js @@ -85,10 +85,6 @@ exports.onExecutePostLogin = async (event, api) => { 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; @@ -168,6 +164,12 @@ exports.onExecutePostLogin = async (event, api) => { for (const mergeProfile of mergeList) { await linkAccount(mainProfile, mergeProfile); } + + // Auth0 Action api object provides a method for updating the current + // authenticated user to the new user_id after account linking has taken place. + // This can only be called once per transaction, so we call it at the end + // once we've merged all profiles. + api.authentication.setPrimaryUser(mainProfile.user_id); } catch (err) { console.error("An error occurred while linking accounts:", err); return api.access.deny(err.message || String(err)); diff --git a/tf/tests/linkUserByEmail.test.js b/tf/tests/linkUserByEmail.test.js index a1880e5..94d7034 100644 --- a/tf/tests/linkUserByEmail.test.js +++ b/tf/tests/linkUserByEmail.test.js @@ -130,7 +130,7 @@ test("If two usersByEmail are found, expect account linking when one is LDAP", a // Expect linking to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); - expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); + expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(1); expect(_event.user.user_id).toEqual(_ldapUser.user_id); }); @@ -147,7 +147,7 @@ test("If two usersByEmail are found, expect account linking even when neither ar // Expect linking to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); - expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); + expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(1); expect(_event.user.user_id).toEqual(_emailUser.user_id); }); @@ -164,7 +164,7 @@ test("If more than 2 usersByEmail is found, expect account linking", async () => // Expect linking to have been called expect(mockManagementClient.users.link).toHaveBeenCalledTimes(2); - expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(2); + expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(1); // The user_id should be the LDAP account. expect(_event.user.user_id).toEqual(_ldapUser.user_id); }); @@ -183,7 +183,7 @@ test("Test for email of different case, expect account linking", async () => { // Expect linking not to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); - expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); + expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(1); }); test("Test for email of different case and where primary is swapped, expect account linking", async () => { @@ -200,7 +200,7 @@ test("Test for email of different case and where primary is swapped, expect acco // Expect linking not to have been called expect(mockManagementClient.users.link).toHaveBeenCalled(); - expect(api.authentication.setPrimaryUser).toHaveBeenCalled(); + expect(api.authentication.setPrimaryUser).toHaveBeenCalledTimes(1); // Expect Primary user to be set to LDAP user expect(_event.user.user_id).toEqual(_ldapUser.user_id);