Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uncaught exceptions in acquireTokenSilent #7073

Merged
merged 4 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix uncaught exceptions in acquireTokenSilent #7073",
"packageName": "@azure/msal-browser",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch"
}
37 changes: 16 additions & 21 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export class StandardController implements IController {
>;

// Active Iframe request
private activeIframeRequest: [Promise<void>, string] | undefined;
private activeIframeRequest: [Promise<boolean>, string] | undefined;

private ssoSilentMeasurement?: InProgressPerformanceEvent;
private acquireTokenByCodeAsyncMeasurement?: InProgressPerformanceEvent;
Expand Down Expand Up @@ -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,
];
Expand All @@ -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(() => {
Expand All @@ -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
Expand All @@ -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(
Expand Down
37 changes: 37 additions & 0 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading