Handle bad_token by removing bad refresh token from cache (#6757)

This PR: - Catches `bad_token` sub error and handles it by immediately removing the refresh token used in the refresh request, then lets the error bubble up to be handled as it is today - Adds test for this scenario --------- Co-authored-by: Thomas Norling <[email protected]>
AzureAD · Dec 12, 2023 · 793b5e6 · 793b5e6
1 parent 162a8c1
commit 793b5e6
Show file tree

Hide file tree

Showing 11 changed files with 194 additions and 41 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -4,7 +4,7 @@
     "files.eol": "\n",
     "github-pr.targetBranch": "dev",
     "editor.codeActionsOnSave": {
-        "source.fixAll.eslint": true
+        "source.fixAll.eslint": "explicit"
     },
     "files.associations": {
         "**/build/*.yml": "azure-pipelines",

diff --git a/change/@azure-msal-browser-a4d64499-b9c4-4f59-9817-dabd465a177f.json b/change/@azure-msal-browser-a4d64499-b9c4-4f59-9817-dabd465a177f.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Handle bad_token by removing bad refresh token from cache #6757",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@azure-msal-common-6eecae28-3257-440b-84cc-09de6236fd41.json b/change/@azure-msal-common-6eecae28-3257-440b-84cc-09de6236fd41.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Handle bad_token by removing bad refresh token from cache #6757",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts
@@ -43,6 +43,7 @@ import {
     CacheLookupPolicy,
     DEFAULT_REQUEST,
     BrowserConstants,
+    iFrameRenewalPolicies,
 } from "../utils/BrowserConstants";
 import * as BrowserUtils from "../utils/BrowserUtils";
 import { RedirectRequest } from "../request/RedirectRequest";
@@ -2078,31 +2079,17 @@ export class StandardController implements IController {
                         silentRequest.correlationId
                     )(silentRequest, cacheLookupPolicy).catch(
                         (refreshTokenError: AuthError) => {
-                            const isSilentlyResolvable =
-                                (!(
-                                    refreshTokenError instanceof
-                                    InteractionRequiredAuthError
-                                ) &&
-                                    (refreshTokenError.errorCode ===
-                                        BrowserConstants.INVALID_GRANT_ERROR ||
-                                        refreshTokenError.errorCode ===
-                                            ClientAuthErrorCodes.tokenRefreshRequired)) ||
-                                refreshTokenError.errorCode ===
-                                    InteractionRequiredAuthErrorCodes.noTokensFound ||
-                                refreshTokenError.errorCode ===
-                                    InteractionRequiredAuthErrorCodes.refreshTokenExpired;
-
-                            const tryIframeRenewal =
-                                cacheLookupPolicy ===
-                                    CacheLookupPolicy.Default ||
-                                cacheLookupPolicy === CacheLookupPolicy.Skip ||
-                                cacheLookupPolicy ===
-                                    CacheLookupPolicy.RefreshTokenAndNetwork;
-
-                            if (isSilentlyResolvable && tryIframeRenewal) {
+                            const shouldTryToResolveSilently =
+                                checkIfRefreshTokenErrorCanBeResolvedSilently(
+                                    refreshTokenError,
+                                    silentRequest,
+                                    cacheLookupPolicy
+                                );
+
+                            if (shouldTryToResolveSilently) {
                                 this.logger.verbose(
                                     "Refresh token expired/invalid or CacheLookupPolicy is set to Skip, attempting acquire token by iframe.",
-                                    request.correlationId
+                                    silentRequest.correlationId
                                 );
                                 return invokeAsync(
                                     this.acquireTokenBySilentIframe.bind(this),
@@ -2112,6 +2099,7 @@ export class StandardController implements IController {
                                     silentRequest.correlationId
                                 )(silentRequest);
                             } else {
+                                // Error cannot be silently resolved or iframe renewal is not allowed, interaction required
                                 throw refreshTokenError;
                             }
                         }
@@ -2157,3 +2145,42 @@ export class StandardController implements IController {
             });
     }
 }
+
+/**
+ * Determines whether an error thrown by the refresh token endpoint can be resolved without interaction
+ * @param refreshTokenError
+ * @param silentRequest
+ * @param cacheLookupPolicy
+ * @returns
+ */
+function checkIfRefreshTokenErrorCanBeResolvedSilently(
+    refreshTokenError: AuthError,
+    silentRequest: CommonSilentFlowRequest,
+    cacheLookupPolicy: CacheLookupPolicy
+): boolean {
+    const noInteractionRequired = !(
+        refreshTokenError instanceof InteractionRequiredAuthError &&
+        // For refresh token errors, bad_token does not always require interaction (silently resolvable)
+        refreshTokenError.subError !==
+            InteractionRequiredAuthErrorCodes.badToken
+    );
+
+    // Errors that result when the refresh token needs to be replaced
+    const refreshTokenRefreshRequired =
+        refreshTokenError.errorCode === BrowserConstants.INVALID_GRANT_ERROR ||
+        refreshTokenError.errorCode ===
+            ClientAuthErrorCodes.tokenRefreshRequired;
+
+    // Errors that may be resolved before falling back to interaction (through iframe renewal)
+    const isSilentlyResolvable =
+        (noInteractionRequired && refreshTokenRefreshRequired) ||
+        refreshTokenError.errorCode ===
+            InteractionRequiredAuthErrorCodes.noTokensFound ||
+        refreshTokenError.errorCode ===
+            InteractionRequiredAuthErrorCodes.refreshTokenExpired;
+
+    // Only these policies allow for an iframe renewal attempt
+    const tryIframeRenewal = iFrameRenewalPolicies.includes(cacheLookupPolicy);
+
+    return isSilentlyResolvable && tryIframeRenewal;
+}
diff --git a/lib/msal-browser/src/utils/BrowserConstants.ts b/lib/msal-browser/src/utils/BrowserConstants.ts
@@ -240,5 +240,11 @@ export const CacheLookupPolicy = {
 export type CacheLookupPolicy =
     (typeof CacheLookupPolicy)[keyof typeof CacheLookupPolicy];
 
+export const iFrameRenewalPolicies: CacheLookupPolicy[] = [
+    CacheLookupPolicy.Default,
+    CacheLookupPolicy.Skip,
+    CacheLookupPolicy.RefreshTokenAndNetwork,
+];
+
 export const LOG_LEVEL_CACHE_KEY = "msal.browser.log.level";
 export const LOG_PII_CACHE_KEY = "msal.browser.log.pii";
diff --git a/lib/msal-common/src/client/RefreshTokenClient.ts b/lib/msal-common/src/client/RefreshTokenClient.ts
@@ -47,6 +47,7 @@ import {
 import { PerformanceEvents } from "../telemetry/performance/PerformanceEvent";
 import { IPerformanceClient } from "../telemetry/performance/IPerformanceClient";
 import { invoke, invokeAsync } from "../utils/FunctionWrappers";
+import { generateCredentialKey } from "../cache/utils/CacheHelpers";
 
 const DEFAULT_REFRESH_TOKEN_EXPIRATION_OFFSET_SECONDS = 300; // 5 Minutes
 
@@ -244,13 +245,29 @@ export class RefreshTokenClient extends BaseClient {
             },
         };
 
-        return invokeAsync(
-            this.acquireToken.bind(this),
-            PerformanceEvents.RefreshTokenClientAcquireToken,
-            this.logger,
-            this.performanceClient,
-            request.correlationId
-        )(refreshTokenRequest);
+        try {
+            return await invokeAsync(
+                this.acquireToken.bind(this),
+                PerformanceEvents.RefreshTokenClientAcquireToken,
+                this.logger,
+                this.performanceClient,
+                request.correlationId
+            )(refreshTokenRequest);
+        } catch (e) {
+            if (
+                e instanceof InteractionRequiredAuthError &&
+                e.subError === InteractionRequiredAuthErrorCodes.badToken
+            ) {
+                // Remove bad refresh token from cache
+                this.logger.verbose(
+                    "acquireTokenWithRefreshToken: bad refresh token, removing from cache"
+                );
+                const badRefreshTokenKey = generateCredentialKey(refreshToken);
+                this.cacheManager.removeRefreshToken(badRefreshTokenKey);
+            }
+
+            throw e;
+        }
     }
 
     /**

diff --git a/lib/msal-common/src/error/InteractionRequiredAuthError.ts b/lib/msal-common/src/error/InteractionRequiredAuthError.ts
@@ -15,6 +15,7 @@ export const InteractionRequiredServerErrorMessage = [
     InteractionRequiredAuthErrorCodes.interactionRequired,
     InteractionRequiredAuthErrorCodes.consentRequired,
     InteractionRequiredAuthErrorCodes.loginRequired,
+    InteractionRequiredAuthErrorCodes.badToken,
 ];
 
 export const InteractionRequiredAuthSubErrorMessage = [
@@ -23,6 +24,7 @@ export const InteractionRequiredAuthSubErrorMessage = [
     "basic_action",
     "user_password_expired",
     "consent_required",
+    "bad_token",
 ];
 
 const InteractionRequiredAuthErrorMessages = {
@@ -32,6 +34,8 @@ const InteractionRequiredAuthErrorMessages = {
         "The requested account is not available in the native broker. It may have been deleted or logged out. Please sign-in again using an interactive API.",
     [InteractionRequiredAuthErrorCodes.refreshTokenExpired]:
         "Refresh token has expired.",
+    [InteractionRequiredAuthErrorCodes.badToken]:
+        "Identity provider returned bad_token due to an expired or invalid refresh token. Please invoke an interactive API to resolve.",
 };
 
 /**
@@ -51,6 +55,12 @@ export const InteractionRequiredAuthErrorMessage = {
             InteractionRequiredAuthErrorCodes.nativeAccountUnavailable
         ],
     },
+    bad_token: {
+        code: InteractionRequiredAuthErrorCodes.badToken,
+        desc: InteractionRequiredAuthErrorMessages[
+            InteractionRequiredAuthErrorCodes.badToken
+        ],
+    },
 };
 
 /**

diff --git a/lib/msal-common/src/error/InteractionRequiredAuthErrorCodes.ts b/lib/msal-common/src/error/InteractionRequiredAuthErrorCodes.ts
@@ -12,3 +12,4 @@ export const refreshTokenExpired = "refresh_token_expired";
 export const interactionRequired = "interaction_required";
 export const consentRequired = "consent_required";
 export const loginRequired = "login_required";
+export const badToken = "bad_token";
diff --git a/lib/msal-common/test/client/RefreshTokenClient.spec.ts b/lib/msal-common/test/client/RefreshTokenClient.spec.ts
@@ -19,6 +19,7 @@ import {
     AUTHENTICATION_RESULT_WITH_HEADERS,
     CORS_RESPONSE_HEADERS,
     TEST_SSH_VALUES,
+    BAD_TOKEN_ERROR_RESPONSE,
 } from "../test_kit/StringConstants";
 import { BaseClient } from "../../src/client/BaseClient";
 import {
@@ -52,13 +53,15 @@ import { SilentFlowClient } from "../../src/client/SilentFlowClient";
 import { AppMetadataEntity } from "../../src/cache/entities/AppMetadataEntity";
 import { CcsCredentialType } from "../../src/account/CcsCredential";
 import {
+    InteractionRequiredAuthError,
     InteractionRequiredAuthErrorCodes,
     createInteractionRequiredAuthError,
 } from "../../src/error/InteractionRequiredAuthError";
 import { StubPerformanceClient } from "../../src/telemetry/performance/StubPerformanceClient";
 import { ProtocolMode } from "../../src/authority/ProtocolMode";
 import { TimeUtils } from "../../src/utils/TimeUtils";
 import { buildAccountFromIdTokenClaims } from "msal-test-utils";
+import { generateCredentialKey } from "../../src/cache/utils/CacheHelpers";
 
 const testAccountEntity: AccountEntity = new AccountEntity();
 testAccountEntity.homeAccountId = `${TEST_DATA_CLIENT_INFO.TEST_UID}.${TEST_DATA_CLIENT_INFO.TEST_UTID}`;
@@ -1369,6 +1372,69 @@ describe("RefreshTokenClient unit tests", () => {
                 )
             );
         });
+
+        it("Removes refresh token if server returns invalid_grant with bad_token suberror", async () => {
+            const config =
+                await ClientTestUtils.createTestClientConfiguration();
+            config.storageInterface!.setAccount(testAccountEntity);
+            config.storageInterface!.setRefreshTokenCredential(
+                testRefreshTokenEntity
+            );
+            config.storageInterface!.setAppMetadata(testAppMetadata);
+            const client = new RefreshTokenClient(
+                config,
+                stubPerformanceClient
+            );
+            const testAccount: AccountInfo =
+                buildAccountFromIdTokenClaims(ID_TOKEN_CLAIMS).getAccountInfo();
+            testAccount.idTokenClaims = ID_TOKEN_CLAIMS;
+            sinon
+                .stub(
+                    RefreshTokenClient.prototype,
+                    <any>"executePostToTokenEndpoint"
+                )
+                .resolves(BAD_TOKEN_ERROR_RESPONSE);
+
+            const serverResponse = BAD_TOKEN_ERROR_RESPONSE.body;
+            const invalidGrantAuthError = new InteractionRequiredAuthError(
+                serverResponse.error,
+                serverResponse.error_description,
+                serverResponse.suberror,
+                serverResponse.timestamp || Constants.EMPTY_STRING,
+                serverResponse.trace_id || Constants.EMPTY_STRING,
+                serverResponse.correlation_id || Constants.EMPTY_STRING,
+                // @ts-ignore
+                serverResponse.claims || Constants.EMPTY_STRING
+            );
+
+            const silentFlowRequest: CommonSilentFlowRequest = {
+                scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE,
+                account: testAccount,
+                authority: TEST_CONFIG.validAuthority,
+                correlationId: TEST_CONFIG.CORRELATION_ID,
+                forceRefresh: false,
+            };
+
+            const badRefreshTokenKey = generateCredentialKey(
+                testRefreshTokenEntity
+            );
+
+            expect(
+                config.storageInterface!.getRefreshTokenCredential(
+                    badRefreshTokenKey
+                )
+            ).toBe(testRefreshTokenEntity);
+
+            await expect(
+                client.acquireTokenByRefreshToken(silentFlowRequest)
+            ).rejects.toMatchObject(invalidGrantAuthError);
+
+            expect(
+                config.storageInterface!.getRefreshTokenCredential(
+                    badRefreshTokenKey
+                )
+            ).toBe(null);
+        });
     });
     describe("Telemetry protocol mode tests", () => {
         const refreshTokenRequest: CommonRefreshTokenRequest = {

diff --git a/lib/msal-common/test/error/InteractionRequiredAuthError.spec.ts b/lib/msal-common/test/error/InteractionRequiredAuthError.spec.ts
@@ -48,13 +48,6 @@ describe("InteractionRequiredAuthError.ts Class Unit Tests", () => {
             expect(isInteractionRequiredError("", "")).toBe(false);
         });
 
-        it("Returns expected value for given error code", () => {
-            InteractionRequiredServerErrorMessage.forEach(function (errorCode) {
-                expect(isInteractionRequiredError(errorCode, "")).toBe(true);
-            });
-            expect(isInteractionRequiredError("bad_token", "")).toBe(false);
-        });
-
         it("Returns expected value for given error string", () => {
             InteractionRequiredServerErrorMessage.forEach(function (errorCode) {
                 expect(
@@ -66,7 +59,7 @@ describe("InteractionRequiredAuthError.ts Class Unit Tests", () => {
             });
             expect(
                 isInteractionRequiredError(
-                    "",
+                    "not_interaction_required",
                     "This is not an interaction required error"
                 )
             ).toBe(false);
@@ -83,7 +76,7 @@ describe("InteractionRequiredAuthError.ts Class Unit Tests", () => {
             });
             expect(
                 isInteractionRequiredError(
-                    "bad_token",
+                    "",
                     "This is not an interaction required error"
                 )
             ).toBe(false);
@@ -97,7 +90,9 @@ describe("InteractionRequiredAuthError.ts Class Unit Tests", () => {
                     true
                 );
             });
-            expect(isInteractionRequiredError("", "", "bad_token")).toBe(false);
+            expect(
+                isInteractionRequiredError("", "", "not_interaction_required")
+            ).toBe(false);
         });
     });
 });
diff --git a/lib/msal-common/test/test_kit/StringConstants.ts b/lib/msal-common/test/test_kit/StringConstants.ts
@@ -676,6 +676,23 @@ export const AUTHORIZATION_PENDING_RESPONSE = {
     },
 };
 
+export const BAD_TOKEN_ERROR_RESPONSE = {
+    body: {
+        error: "invalid_grant",
+        suberror: "bad_token",
+        error_description:
+            "AADSTS9002313: Invalid request. Request is malformed or invalid." +
+            "Trace ID: 01707a0c-640b-4049-8cbb-ee2304dc0700" +
+            "Correlation ID: 78b0fdfc-dd0e-4dfb-b13a-d316333783f6" +
+            "Timestamp: 2020-03-26 22:54:14Z",
+        error_codes: [9002313],
+        timestamp: "2020-03-26 22:54:14Z",
+        trace_id: "01707a0c-640b-4049-8cbb-ee2304dc0700",
+        correlation_id: "78b0fdfc-dd0e-4dfb-b13a-d316333783f6",
+        error_uri: "https://login.microsoftonline.com/error?code=9002313",
+    },
+};
+
 export const SERVER_UNEXPECTED_ERROR = {
     status: 503,
     body: {