From 1da544a4d76eb1c531d440d9d7c826e732203301 Mon Sep 17 00:00:00 2001 From: Thomas Norling Date: Wed, 1 May 2024 15:28:22 -0700 Subject: [PATCH 1/3] Fix uncaught exception when using iframes --- .../src/controllers/StandardController.ts | 37 ++++++++----------- .../test/app/PublicClientApplication.spec.ts | 37 +++++++++++++++++++ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts index 89ccc8e0e5..0a0e328de2 100644 --- a/lib/msal-browser/src/controllers/StandardController.ts +++ b/lib/msal-browser/src/controllers/StandardController.ts @@ -158,7 +158,7 @@ export class StandardController implements IController { >; // Active Iframe request - private activeIframeRequest: [Promise, string] | undefined; + private activeIframeRequest: [Promise, string] | undefined; private ssoSilentMeasurement?: InProgressPerformanceEvent; private acquireTokenByCodeAsyncMeasurement?: InProgressPerformanceEvent; @@ -2039,13 +2039,11 @@ export class StandardController implements IController { if (shouldTryToResolveSilently) { if (!this.activeIframeRequest) { - let _resolve: () => void, - _reject: (reason?: AuthError | Error) => void; + let _resolve: (result: boolean) => void; // Always set the active request tracker immediately after checking it to prevent races this.activeIframeRequest = [ - new Promise((resolve, reject) => { + new Promise((resolve) => { _resolve = resolve; - _reject = reject; }), silentRequest.correlationId, ]; @@ -2061,11 +2059,11 @@ export class StandardController implements IController { silentRequest.correlationId )(silentRequest) .then((iframeResult) => { - _resolve(); + _resolve(true); return iframeResult; }) .catch((e) => { - _reject(e); + _resolve(false); throw e; }) .finally(() => { @@ -2087,20 +2085,11 @@ export class StandardController implements IController { awaitIframeCorrelationId: activeCorrelationId, }); - // Await for errors first so we can distinguish errors thrown by activePromise versus errors thrown by .then below - await activePromise.catch(() => { - awaitConcurrentIframeMeasure.end({ - success: false, - }); - this.logger.info( - `Iframe request with correlationId: ${activeCorrelationId} failed. Interaction is required.` - ); - // If previous iframe request failed, it's unlikely to succeed this time. Throw original error. - throw refreshTokenError; + const activePromiseResult = await activePromise; + awaitConcurrentIframeMeasure.end({ + success: activePromiseResult, }); - - return activePromise.then(() => { - awaitConcurrentIframeMeasure.end({ success: true }); + if (activePromiseResult) { this.logger.verbose( `Parallel iframe request with correlationId: ${activeCorrelationId} succeeded. Retrying cache and/or RT redemption`, silentRequest.correlationId @@ -2110,7 +2099,13 @@ export class StandardController implements IController { silentRequest, cacheLookupPolicy ); - }); + } else { + this.logger.info( + `Iframe request with correlationId: ${activeCorrelationId} failed. Interaction is required.` + ); + // If previous iframe request failed, it's unlikely to succeed this time. Throw original error. + throw refreshTokenError; + } } else { // Cache policy set to skip and another iframe request is already in progress this.logger.warning( diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 1fd1a6b3ab..e9836d79a4 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -4817,6 +4817,43 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }); }); + it("throws iframe error if iframe renewal throws", (done) => { + const testAccount: AccountInfo = { + homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID, + localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID, + environment: "login.windows.net", + tenantId: "testTenantId", + username: "username@contoso.com", + }; + + jest.spyOn( + RefreshTokenClient.prototype, + "acquireTokenByRefreshToken" + ).mockRejectedValue( + createInteractionRequiredAuthError( + InteractionRequiredAuthErrorCodes.refreshTokenExpired + ) + ); + + const testIframeError = new InteractionRequiredAuthError( + "interaction_required", + "interaction is required" + ); + + jest.spyOn( + SilentIframeClient.prototype, + "acquireToken" + ).mockRejectedValue(testIframeError); + + pca.acquireTokenSilent({ + scopes: ["Scope1"], + account: testAccount, + }).catch((e) => { + expect(e).toEqual(testIframeError); + done(); + }); + }); + it("Falls back to silent handler if thrown error is a refresh token expired error", async () => { const invalidGrantError: ServerError = new ServerError( "invalid_grant", From fece2842bc416e6a7cea1927b8d52cd2547847d0 Mon Sep 17 00:00:00 2001 From: Thomas Norling Date: Wed, 1 May 2024 15:33:49 -0700 Subject: [PATCH 2/3] Change files --- ...-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json diff --git a/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json b/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json new file mode 100644 index 0000000000..a79905d82b --- /dev/null +++ b/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fix uncaught exceptions in acquireTokenSilent", + "packageName": "@azure/msal-browser", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} From ae208f6657697fe0364d87da455b16fea08b7d4c Mon Sep 17 00:00:00 2001 From: Thomas Norling Date: Wed, 1 May 2024 15:37:40 -0700 Subject: [PATCH 3/3] Update change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json --- ...azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json b/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json index a79905d82b..26a27a19f2 100644 --- a/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json +++ b/change/@azure-msal-browser-94058809-8f02-4078-bdc6-02ff59c54ae3.json @@ -1,6 +1,6 @@ { "type": "patch", - "comment": "Fix uncaught exceptions in acquireTokenSilent", + "comment": "Fix uncaught exceptions in acquireTokenSilent #7073", "packageName": "@azure/msal-browser", "email": "thomas.norling@microsoft.com", "dependentChangeType": "patch"