Skip to content

Commit 8a4aa37

Browse files
tnorlingtnorling
authored
Fix for concurrent iframe calls (#6962)
When multiple acquireTokenSilent requests are made in parallel there is a chance that they will all need to fallback to the iframe flow. It is both unnecessary for more than 1 iframe call to be made and results in a perf and reliability degradation for all calls. This PR introduces a mechanism to track in progress iframe calls and cause subsequent requests to wait before retrying the cache and/or RT redemption. Note: Telemetry dashboards will need to be updated after this change is released to avoid counting awaited iframe calls against our perf metrics more than once. Note: This PR does not make any changes to ssoSilent - follow up work should add ssoSilent calls to the active request tracking variable and log warnings when more than 1 ssoSilent requests are made but should **not** block the calls.
1 parent 1a0414e commit 8a4aa37

File tree

5 files changed

+393
-97
lines changed

5 files changed

+393
-97
lines changed

‎change/@azure-msal-browser-1c51e91a-9f80-485d-a826-aa20c210cc26.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Improve perf and reliability of parallel acquireTokenSilent calls that must fallback to iframes #6962",
4+
"packageName": "@azure/msal-browser",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@azure-msal-common-dbac308f-ff86-497a-b282-8b0e5cbf64ee.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "New performance event for awaiting parallel iframe calls #6962",
4+
"packageName": "@azure/msal-common",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎lib/msal-browser/src/controllers/StandardController.ts

+172-97
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ export class StandardController implements IController {
141141
Promise<AuthenticationResult>
142142
>;
143143

144+
// Active Iframe request
145+
private activeIframeRequest: [Promise<void>, string] | undefined;
146+
144147
private ssoSilentMeasurement?: InProgressPerformanceEvent;
145148
private acquireTokenByCodeAsyncMeasurement?: InProgressPerformanceEvent;
146149
/**
@@ -1966,24 +1969,180 @@ export class StandardController implements IController {
19661969
document.addEventListener("visibilitychange", () =>
19671970
this.trackPageVisibility(request.correlationId)
19681971
);
1969-
let result: Promise<AuthenticationResult>;
1972+
1973+
const silentRequest = await invokeAsync(
1974+
initializeSilentRequest,
1975+
PerformanceEvents.InitializeSilentRequest,
1976+
this.logger,
1977+
this.performanceClient,
1978+
request.correlationId
1979+
)(request, account, this.config, this.performanceClient, this.logger);
1980+
const cacheLookupPolicy =
1981+
request.cacheLookupPolicy || CacheLookupPolicy.Default;
1982+
1983+
const result = this.acquireTokenSilentNoIframe(
1984+
silentRequest,
1985+
cacheLookupPolicy
1986+
).catch(async (refreshTokenError: AuthError) => {
1987+
const shouldTryToResolveSilently =
1988+
checkIfRefreshTokenErrorCanBeResolvedSilently(
1989+
refreshTokenError,
1990+
cacheLookupPolicy
1991+
);
1992+
1993+
if (shouldTryToResolveSilently) {
1994+
if (!this.activeIframeRequest) {
1995+
let _resolve: () => void,
1996+
_reject: (reason?: AuthError | Error) => void;
1997+
// Always set the active request tracker immediately after checking it to prevent races
1998+
this.activeIframeRequest = [
1999+
new Promise((resolve, reject) => {
2000+
_resolve = resolve;
2001+
_reject = reject;
2002+
}),
2003+
silentRequest.correlationId,
2004+
];
2005+
this.logger.verbose(
2006+
"Refresh token expired/invalid or CacheLookupPolicy is set to Skip, attempting acquire token by iframe.",
2007+
silentRequest.correlationId
2008+
);
2009+
return invokeAsync(
2010+
this.acquireTokenBySilentIframe.bind(this),
2011+
PerformanceEvents.AcquireTokenBySilentIframe,
2012+
this.logger,
2013+
this.performanceClient,
2014+
silentRequest.correlationId
2015+
)(silentRequest)
2016+
.then((iframeResult) => {
2017+
_resolve();
2018+
return iframeResult;
2019+
})
2020+
.catch((e) => {
2021+
_reject(e);
2022+
throw e;
2023+
})
2024+
.finally(() => {
2025+
this.activeIframeRequest = undefined;
2026+
});
2027+
} else if (cacheLookupPolicy !== CacheLookupPolicy.Skip) {
2028+
const [activePromise, activeCorrelationId] =
2029+
this.activeIframeRequest;
2030+
this.logger.verbose(
2031+
`Iframe request is already in progress, awaiting resolution for request with correlationId: ${activeCorrelationId}`,
2032+
silentRequest.correlationId
2033+
);
2034+
const awaitConcurrentIframeMeasure =
2035+
this.performanceClient.startMeasurement(
2036+
PerformanceEvents.AwaitConcurrentIframe,
2037+
silentRequest.correlationId
2038+
);
2039+
awaitConcurrentIframeMeasure.add({
2040+
awaitIframeCorrelationId: activeCorrelationId,
2041+
});
2042+
2043+
// Await for errors first so we can distinguish errors thrown by activePromise versus errors thrown by .then below
2044+
await activePromise.catch(() => {
2045+
awaitConcurrentIframeMeasure.end({
2046+
success: false,
2047+
});
2048+
this.logger.info(
2049+
`Iframe request with correlationId: ${activeCorrelationId} failed. Interaction is required.`
2050+
);
2051+
// If previous iframe request failed, it's unlikely to succeed this time. Throw original error.
2052+
throw refreshTokenError;
2053+
});
2054+
2055+
return activePromise.then(() => {
2056+
awaitConcurrentIframeMeasure.end({ success: true });
2057+
this.logger.verbose(
2058+
`Parallel iframe request with correlationId: ${activeCorrelationId} succeeded. Retrying cache and/or RT redemption`,
2059+
silentRequest.correlationId
2060+
);
2061+
// Retry cache lookup and/or RT exchange after iframe completes
2062+
return this.acquireTokenSilentNoIframe(
2063+
silentRequest,
2064+
cacheLookupPolicy
2065+
);
2066+
});
2067+
} else {
2068+
// Cache policy set to skip and another iframe request is already in progress
2069+
this.logger.warning(
2070+
"Another iframe request is currently in progress and CacheLookupPolicy is set to Skip. This may result in degraded performance and/or reliability for both calls. Please consider changing the CacheLookupPolicy to take advantage of request queuing and token cache.",
2071+
silentRequest.correlationId
2072+
);
2073+
return invokeAsync(
2074+
this.acquireTokenBySilentIframe.bind(this),
2075+
PerformanceEvents.AcquireTokenBySilentIframe,
2076+
this.logger,
2077+
this.performanceClient,
2078+
silentRequest.correlationId
2079+
)(silentRequest);
2080+
}
2081+
} else {
2082+
// Error cannot be silently resolved or iframe renewal is not allowed, interaction required
2083+
throw refreshTokenError;
2084+
}
2085+
});
2086+
2087+
return result
2088+
.then((response) => {
2089+
this.eventHandler.emitEvent(
2090+
EventType.ACQUIRE_TOKEN_SUCCESS,
2091+
InteractionType.Silent,
2092+
response
2093+
);
2094+
if (request.correlationId) {
2095+
this.performanceClient.addFields(
2096+
{
2097+
fromCache: response.fromCache,
2098+
isNativeBroker: response.fromNativeBroker,
2099+
requestId: response.requestId,
2100+
},
2101+
request.correlationId
2102+
);
2103+
}
2104+
2105+
return response;
2106+
})
2107+
.catch((tokenRenewalError: Error) => {
2108+
this.eventHandler.emitEvent(
2109+
EventType.ACQUIRE_TOKEN_FAILURE,
2110+
InteractionType.Silent,
2111+
null,
2112+
tokenRenewalError
2113+
);
2114+
throw tokenRenewalError;
2115+
})
2116+
.finally(() => {
2117+
document.removeEventListener("visibilitychange", () =>
2118+
this.trackPageVisibility(request.correlationId)
2119+
);
2120+
});
2121+
}
2122+
2123+
/**
2124+
* AcquireTokenSilent without the iframe fallback. This is used to enable the correct fallbacks in cases where there's a potential for multiple silent requests to be made in parallel and prevent those requests from making concurrent iframe requests.
2125+
* @param silentRequest
2126+
* @param cacheLookupPolicy
2127+
* @returns
2128+
*/
2129+
private async acquireTokenSilentNoIframe(
2130+
silentRequest: CommonSilentFlowRequest,
2131+
cacheLookupPolicy: CacheLookupPolicy
2132+
): Promise<AuthenticationResult> {
19702133
if (
19712134
NativeMessageHandler.isNativeAvailable(
19722135
this.config,
19732136
this.logger,
19742137
this.nativeExtensionProvider,
1975-
request.authenticationScheme
2138+
silentRequest.authenticationScheme
19762139
) &&
1977-
account.nativeAccountId
2140+
silentRequest.account.nativeAccountId
19782141
) {
19792142
this.logger.verbose(
19802143
"acquireTokenSilent - attempting to acquire token from native platform"
19812144
);
1982-
const silentRequest: SilentRequest = {
1983-
...request,
1984-
account,
1985-
};
1986-
result = this.acquireTokenNative(
2145+
return this.acquireTokenNative(
19872146
silentRequest,
19882147
ApiId.acquireTokenSilent_silentFlow
19892148
).catch(async (e: AuthError) => {
@@ -1995,47 +2154,25 @@ export class StandardController implements IController {
19952154
this.nativeExtensionProvider = undefined; // Prevent future requests from continuing to attempt
19962155

19972156
// Cache will not contain tokens, given that previous WAM requests succeeded. Skip cache and RT renewal and go straight to iframe renewal
1998-
const silentIframeClient = this.createSilentIframeClient(
1999-
request.correlationId
2157+
throw createClientAuthError(
2158+
ClientAuthErrorCodes.tokenRefreshRequired
20002159
);
2001-
return silentIframeClient.acquireToken(request);
20022160
}
20032161
throw e;
20042162
});
20052163
} else {
20062164
this.logger.verbose(
20072165
"acquireTokenSilent - attempting to acquire token from web flow"
20082166
);
2009-
2010-
const silentRequest = await invokeAsync(
2011-
initializeSilentRequest,
2012-
PerformanceEvents.InitializeSilentRequest,
2013-
this.logger,
2014-
this.performanceClient,
2015-
request.correlationId
2016-
)(
2017-
request,
2018-
account,
2019-
this.config,
2020-
this.performanceClient,
2021-
this.logger
2022-
);
2023-
2024-
const cacheLookupPolicy =
2025-
request.cacheLookupPolicy || CacheLookupPolicy.Default;
2026-
2027-
result = invokeAsync(
2167+
return invokeAsync(
20282168
this.acquireTokenFromCache.bind(this),
20292169
PerformanceEvents.AcquireTokenFromCache,
20302170
this.logger,
20312171
this.performanceClient,
20322172
silentRequest.correlationId
20332173
)(silentRequest, cacheLookupPolicy).catch(
20342174
(cacheError: AuthError) => {
2035-
if (
2036-
request.cacheLookupPolicy ===
2037-
CacheLookupPolicy.AccessToken
2038-
) {
2175+
if (cacheLookupPolicy === CacheLookupPolicy.AccessToken) {
20392176
throw cacheError;
20402177
}
20412178

@@ -2051,71 +2188,10 @@ export class StandardController implements IController {
20512188
this.logger,
20522189
this.performanceClient,
20532190
silentRequest.correlationId
2054-
)(silentRequest, cacheLookupPolicy).catch(
2055-
(refreshTokenError: AuthError) => {
2056-
const shouldTryToResolveSilently =
2057-
checkIfRefreshTokenErrorCanBeResolvedSilently(
2058-
refreshTokenError,
2059-
silentRequest,
2060-
cacheLookupPolicy
2061-
);
2062-
2063-
if (shouldTryToResolveSilently) {
2064-
this.logger.verbose(
2065-
"Refresh token expired/invalid or CacheLookupPolicy is set to Skip, attempting acquire token by iframe.",
2066-
silentRequest.correlationId
2067-
);
2068-
return invokeAsync(
2069-
this.acquireTokenBySilentIframe.bind(this),
2070-
PerformanceEvents.AcquireTokenBySilentIframe,
2071-
this.logger,
2072-
this.performanceClient,
2073-
silentRequest.correlationId
2074-
)(silentRequest);
2075-
} else {
2076-
// Error cannot be silently resolved or iframe renewal is not allowed, interaction required
2077-
throw refreshTokenError;
2078-
}
2079-
}
2080-
);
2191+
)(silentRequest, cacheLookupPolicy);
20812192
}
20822193
);
20832194
}
2084-
2085-
return result
2086-
.then((response) => {
2087-
this.eventHandler.emitEvent(
2088-
EventType.ACQUIRE_TOKEN_SUCCESS,
2089-
InteractionType.Silent,
2090-
response
2091-
);
2092-
if (request.correlationId) {
2093-
this.performanceClient.addFields(
2094-
{
2095-
fromCache: response.fromCache,
2096-
isNativeBroker: response.fromNativeBroker,
2097-
requestId: response.requestId,
2098-
},
2099-
request.correlationId
2100-
);
2101-
}
2102-
2103-
return response;
2104-
})
2105-
.catch((tokenRenewalError: Error) => {
2106-
this.eventHandler.emitEvent(
2107-
EventType.ACQUIRE_TOKEN_FAILURE,
2108-
InteractionType.Silent,
2109-
null,
2110-
tokenRenewalError
2111-
);
2112-
throw tokenRenewalError;
2113-
})
2114-
.finally(() => {
2115-
document.removeEventListener("visibilitychange", () =>
2116-
this.trackPageVisibility(request.correlationId)
2117-
);
2118-
});
21192195
}
21202196
}
21212197

@@ -2128,7 +2204,6 @@ export class StandardController implements IController {
21282204
*/
21292205
function checkIfRefreshTokenErrorCanBeResolvedSilently(
21302206
refreshTokenError: AuthError,
2131-
silentRequest: CommonSilentFlowRequest,
21322207
cacheLookupPolicy: CacheLookupPolicy
21332208
): boolean {
21342209
const noInteractionRequired = !(

0 commit comments

Comments
 (0)