diff --git a/extensions/microsoft-authentication/src/common/cachePlugin.ts b/extensions/microsoft-authentication/src/common/cachePlugin.ts index b87fdb78d9b553..bb1ef7b3866c05 100644 --- a/extensions/microsoft-authentication/src/common/cachePlugin.ts +++ b/extensions/microsoft-authentication/src/common/cachePlugin.ts @@ -49,6 +49,19 @@ export class SecretStorageCachePlugin implements ICachePlugin, Disposable { } } + async afterCacheFailure(tokenCacheContext: TokenCacheContext): Promise { + // Unlike a successful token response, a failed request does not reload the + // cache before changing it. Keep credentials written by another window + // while the request was in flight instead of overwriting them. + const data = await this._secretStorage.get(this._key); + if (data !== this._value) { + this._value = data; + tokenCacheContext.tokenCache.deserialize(data ?? '{}'); + return; + } + await this.afterCacheAccess(tokenCacheContext); + } + dispose() { this._disposable.dispose(); } diff --git a/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts b/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts index 4ab1dd3c3d0003..a6e0f34d3fcd3f 100644 --- a/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts +++ b/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts @@ -11,6 +11,7 @@ import { SecretStorageCachePlugin } from '../common/cachePlugin'; import { MsalLoggerOptions } from '../common/loggerOptions'; import { ICachedPublicClientApplication } from '../common/publicClientCache'; import { IAccountAccess } from '../common/accountAccess'; +import { acquireTokenSilentWithCache } from './tokenCache'; import { MicrosoftAuthenticationTelemetryReporter } from '../common/telemetryReporter'; export class CachedPublicClientApplication implements ICachedPublicClientApplication { @@ -110,7 +111,7 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica async acquireTokenSilent(request: SilentFlowRequest): Promise { this._logger.debug(`[acquireTokenSilent] [${this._clientId}] [${request.authority}] [${request.scopes.join(' ')}] [${request.account.username}] starting...`); - let result = await this._sequencer.queue(() => this._pca.acquireTokenSilent(request)); + let result = await this._acquireTokenSilent(request); this._logger.debug(`[acquireTokenSilent] [${this._clientId}] [${request.authority}] [${request.scopes.join(' ')}] [${request.account.username}] got result`); // Check expiration of id token and if it's 5min before expiration, force a refresh. // this is what MSAL does for access tokens already so we're just adding it for id tokens since we care about those. @@ -127,7 +128,7 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica // HACK: Broker doesn't support forceRefresh so we need to pass in claims which will force a refresh ? { ...request, claims: request.claims ?? '{ "id_token": {}}' } : { ...request, forceRefresh: true }; - result = await this._sequencer.queue(() => this._pca.acquireTokenSilent(newRequest)); + result = await this._acquireTokenSilent(newRequest); this._logger.debug(`[acquireTokenSilent] [${this._clientId}] [${request.authority}] [${request.scopes.join(' ')}] [${request.account.username}] got forced result`); } const newIdTokenExpirationInSecs = (result.idTokenClaims as { exp?: number }).exp; @@ -170,6 +171,12 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica return result; } + private _acquireTokenSilent(request: SilentFlowRequest): Promise { + return this._sequencer.queue(() => this.isBrokerAvailable + ? this._pca.acquireTokenSilent(request) + : acquireTokenSilentWithCache(this._pca, this._secretStorageCachePlugin, request)); + } + async acquireTokenInteractive(request: InteractiveRequest): Promise { this._logger.debug(`[acquireTokenInteractive] [${this._clientId}] [${request.authority}] [${request.scopes?.join(' ')}] loopbackClientOverride: ${request.loopbackClient ? 'true' : 'false'}`); return await window.withProgress( diff --git a/extensions/microsoft-authentication/src/node/test/tokenCache.test.ts b/extensions/microsoft-authentication/src/node/test/tokenCache.test.ts new file mode 100644 index 00000000000000..ab9fb667fc0895 --- /dev/null +++ b/extensions/microsoft-authentication/src/node/test/tokenCache.test.ts @@ -0,0 +1,411 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { + AccountInfo, + INetworkModule, + NetworkRequestOptions, + NetworkResponse, + PublicClientApplication, + SilentFlowRequest, +} from '@azure/msal-node'; +import { + Disposable, + LogOutputChannel, + EventEmitter, + SecretStorage, + SecretStorageChangeEvent, + window, + workspace, + ConfigurationTarget, +} from 'vscode'; +import { SecretStorageCachePlugin } from '../../common/cachePlugin'; +import { CachedPublicClientApplication } from '../cachedPublicClientApplication'; +import { MicrosoftAuthenticationTelemetryReporter } from '../../common/telemetryReporter'; + +const clientId = '00000000-0000-4000-8000-000000000001'; +const tenantId = '00000000-0000-4000-8000-000000000002'; +const userId = '00000000-0000-4000-8000-000000000003'; +const environment = 'login.microsoftonline.com'; +const authority = `https://${environment}/${tenantId}`; +const cacheKey = `pca:${clientId}`; + +class TestSecretStorage implements SecretStorage, Disposable { + private readonly _onDidChange = new EventEmitter(); + readonly onDidChange = this._onDidChange.event; + private readonly _values = new Map(); + writes = 0; + failWrites = false; + + async keys(): Promise { + return [...this._values.keys()]; + } + async get(key: string): Promise { + return this._values.get(key); + } + async store(key: string, value: string): Promise { + if (this.failWrites) { + throw new Error('SecretStorage write failed'); + } + this._values.set(key, value); + this.writes++; + this._onDidChange.fire({ key }); + } + async delete(key: string): Promise { + this._values.delete(key); + this._onDidChange.fire({ key }); + } + dispose(): void { + this._onDidChange.dispose(); + } +} + +function encode(value: object): string { + return Buffer.from(JSON.stringify(value)).toString('base64url'); +} + +/** Uses real MSAL code and cache handling; only the identity server is simulated. */ +class TestIdentityServer implements INetworkModule { + issueFamilyToken = true; + idTokenExpiresIn = 3600; + rejectApplicationToken = false; + refreshError = 'invalid_grant'; + refreshSubError = 'bad_token'; + readonly requests: URLSearchParams[] = []; + beforeReject: (() => Promise) | undefined; + + async sendGetRequestAsync(): Promise> { + throw new Error( + 'Unexpected network GET: authority metadata is supplied by the test', + ); + } + + async sendPostRequestAsync( + _url: string, + options?: NetworkRequestOptions, + ): Promise> { + const request = new URLSearchParams(options?.body); + this.requests.push(request); + const refreshToken = request.get('refresh_token'); + if ( + refreshToken === 'expired-family-token' || + (refreshToken && this.rejectApplicationToken) + ) { + await this.beforeReject?.(); + return { + headers: {}, + status: 400, + body: { + error: this.refreshError, + error_description: + 'AADSTS700082: The refresh token has expired due to inactivity.', + suberror: this.refreshSubError, + error_codes: [700082], + } as T, + }; + } + const now = Math.floor(Date.now() / 1000); + const idToken = `${encode({ alg: 'none' })}.${encode({ + aud: clientId, + iss: `${authority}/v2.0`, + iat: now, + exp: now + this.idTokenExpiresIn, + tid: tenantId, + oid: userId, + sub: userId, + preferred_username: 'test@example.com', + })}.synthetic-signature`; + return { + headers: {}, + status: 200, + body: { + token_type: 'Bearer', + scope: request.get('scope'), + expires_in: 3600, + access_token: 'fresh-access-token', + id_token: idToken, + refresh_token: this.issueFamilyToken + ? 'expired-family-token' + : 'fresh-application-token', + foci: this.issueFamilyToken ? '1' : undefined, + client_info: encode({ uid: userId, utid: tenantId }), + } as T, + }; + } +} + +suite('Microsoft authentication token cache', () => { + let secrets: TestSecretStorage; + let cachePlugin: SecretStorageCachePlugin; + let server: TestIdentityServer; + let pca: PublicClientApplication; + let account: AccountInfo; + let app: CachedPublicClientApplication | undefined; + let disposables: Disposable[]; + let previousImplementation: string | undefined; + let logger: LogOutputChannel; + + suiteSetup(() => { + logger = window.createOutputChannel('Microsoft authentication cache test', { log: true }); + }); + + suiteTeardown(() => { + logger.dispose(); + }); + + async function acquireTokenSilent(request: SilentFlowRequest) { + if (!app) { + const accessChanged = new EventEmitter(); + disposables.push(accessChanged); + app = await CachedPublicClientApplication.create(clientId, secrets, { + onDidAccountAccessChange: accessChanged.event, + isAllowedAccess: () => true, + setAllowedAccess: async () => { } + }, logger, sinon.createStubInstance(MicrosoftAuthenticationTelemetryReporter) as unknown as MicrosoftAuthenticationTelemetryReporter); + disposables.push(app); + assert.strictEqual(app.isBrokerAvailable, false); + } + // Install real MSAL with simulated transport on this instance, without replacing acquisition or persistence methods. + Object.assign(app, { _pca: pca, _secretStorageCachePlugin: cachePlugin }); + return app.acquireTokenSilent(request); + } + + function createPca(plugin = cachePlugin): PublicClientApplication { + return new PublicClientApplication({ + auth: { + clientId, + authority, + cloudDiscoveryMetadata: JSON.stringify({ + metadata: [ + { + preferred_network: environment, + preferred_cache: environment, + aliases: [environment], + }, + ], + }), + authorityMetadata: JSON.stringify({ + authorization_endpoint: `${authority}/oauth2/v2.0/authorize`, + token_endpoint: `${authority}/oauth2/v2.0/token`, + issuer: `${authority}/v2.0`, + jwks_uri: `${authority}/discovery/v2.0/keys`, + }), + }, + system: { networkClient: server }, + cache: { cachePlugin: plugin }, + }); + } + + async function signIn(): Promise { + // This is the code exchange used by acquireTokenInteractive after browser authorization. + const result = await pca.acquireTokenByCode({ + code: 'synthetic-code', + redirectUri: 'http://localhost', + scopes: ['User.Read'], + }); + assert.ok(result.account); + account = result.account; + } + + function request(): SilentFlowRequest { + return { account, authority, scopes: ['User.Read'], forceRefresh: true }; + } + + function refreshTokensSent(): (string | null)[] { + return server.requests + .filter((r) => r.get('grant_type') === 'refresh_token') + .map((r) => r.get('refresh_token')); + } + + async function storedRefreshTokens(): Promise { + const cache = JSON.parse((await secrets.get(cacheKey))!) as { + RefreshToken: Record; + }; + return Object.values(cache.RefreshToken).map((token) => token.secret); + } + + setup(async () => { + disposables = []; + app = undefined; + const configuration = workspace.getConfiguration('microsoft-authentication'); + previousImplementation = configuration.inspect('implementation')?.globalValue; + await configuration.update('implementation', 'msal-no-broker', ConfigurationTarget.Global); + secrets = new TestSecretStorage(); + cachePlugin = new SecretStorageCachePlugin(secrets, cacheKey); + server = new TestIdentityServer(); + pca = createPca(); + await signIn(); + server.issueFamilyToken = false; + }); + + teardown(async () => { + for (const disposable of disposables.reverse()) { + disposable.dispose(); + } + cachePlugin.dispose(); + secrets.dispose(); + sinon.restore(); + await workspace.getConfiguration('microsoft-authentication').update('implementation', previousImplementation, ConfigurationTarget.Global); + }); + + test('uses fresh credentials after signing in over an expired family refresh token', async () => { + await signIn(); + assert.strictEqual((await pca.getAllAccounts()).length, 1); + const result = await acquireTokenSilent(request()); + assert.deepStrictEqual( + { + token: result.accessToken, + sent: refreshTokensSent(), + stored: await storedRefreshTokens(), + }, + { + token: 'fresh-access-token', + sent: ['expired-family-token', 'fresh-application-token'], + stored: ['fresh-application-token'], + }, + ); + }); + + test('recovers when a cached access token requires an ID-token refresh', async () => { + server.idTokenExpiresIn = 120; + await signIn(); + server.idTokenExpiresIn = 3600; + const result = await acquireTokenSilent({ ...request(), forceRefresh: false }); + assert.deepStrictEqual({ token: result.accessToken, sent: refreshTokensSent(), stored: await storedRefreshTokens() }, { + token: 'fresh-access-token', + sent: ['expired-family-token', 'fresh-application-token'], + stored: ['fresh-application-token'] + }); + }); + + test('does not reload the rejected token after restarting', async () => { + await signIn(); + await acquireTokenSilent(request()); + app?.dispose(); + app = undefined; + pca = createPca(); + await acquireTokenSilent(request()); + assert.deepStrictEqual(refreshTokensSent(), [ + 'expired-family-token', + 'fresh-application-token', + 'fresh-application-token', + ]); + }); + + test('persists invalidation when no usable refresh token remains', async () => { + await assert.rejects( + acquireTokenSilent(request()), + { errorCode: 'no_tokens_found' }, + ); + app?.dispose(); + app = undefined; + pca = createPca(); + await assert.rejects( + acquireTokenSilent(request()), + { errorCode: 'no_tokens_found' }, + ); + assert.deepStrictEqual( + { sent: refreshTokensSent(), stored: await storedRefreshTokens() }, + { sent: ['expired-family-token'], stored: [] }, + ); + }); + + test('bounds retries and persists invalidation when both refresh tokens are rejected', async () => { + await signIn(); + server.rejectApplicationToken = true; + await assert.rejects( + acquireTokenSilent(request()), + { errorCode: 'invalid_grant', subError: 'bad_token' }, + ); + assert.deepStrictEqual( + { sent: refreshTokensSent(), stored: await storedRefreshTokens() }, + { + sent: ['expired-family-token', 'fresh-application-token'], + stored: [], + }, + ); + }); + + test('does not retry or change credentials for unrelated server errors', async () => { + await signIn(); + server.refreshError = 'temporarily_unavailable'; + server.refreshSubError = ''; + const before = await secrets.get(cacheKey); + await assert.rejects( + acquireTokenSilent(request()), + { errorCode: 'temporarily_unavailable' }, + ); + assert.deepStrictEqual( + { sent: refreshTokensSent(), cache: await secrets.get(cacheKey) }, + { sent: ['expired-family-token'], cache: before }, + ); + }); + + test('does not write or retry when the cached access token is valid', async () => { + await signIn(); + const writes = secrets.writes; + const result = await acquireTokenSilent({ + ...request(), + forceRefresh: false, + }); + assert.deepStrictEqual( + { + cached: result.fromCache, + sent: refreshTokensSent(), + writes: secrets.writes, + }, + { cached: true, sent: [], writes }, + ); + }); + + test('does not retry if persisting the invalidation fails', async () => { + await signIn(); + secrets.failWrites = true; + await assert.rejects( + acquireTokenSilent(request()), + /SecretStorage write failed/, + ); + assert.deepStrictEqual(refreshTokensSent(), ['expired-family-token']); + }); + + test('preserves a concurrent sign-in while a refresh request is in flight', async () => { + const otherPlugin = new SecretStorageCachePlugin(secrets, cacheKey); + try { + const otherPca = createPca(otherPlugin); + server.beforeReject = async () => { + server.beforeReject = undefined; + await otherPca.acquireTokenByCode({ + code: 'other-window-code', + redirectUri: 'http://localhost', + scopes: ['Other.Read'], + }); + }; + // The bounded retry can encounter the old family token again, but must + // preserve the application token that was written by the other window. + await assert.rejects( + acquireTokenSilent(request()), + { subError: 'bad_token' }, + ); + const result = await acquireTokenSilent(request()); + assert.deepStrictEqual( + { token: result.accessToken, stored: await storedRefreshTokens() }, + { token: 'fresh-access-token', stored: ['fresh-application-token'] }, + ); + } finally { + otherPlugin.dispose(); + } + }); + + test('does not restore credentials removed by another window', async () => { + server.beforeReject = () => secrets.delete(cacheKey); + await assert.rejects( + acquireTokenSilent(request()), + { errorCode: 'no_tokens_found' }, + ); + assert.strictEqual(await secrets.get(cacheKey), undefined); + }); +}); diff --git a/extensions/microsoft-authentication/src/node/tokenCache.ts b/extensions/microsoft-authentication/src/node/tokenCache.ts new file mode 100644 index 00000000000000..6b872e11c2d002 --- /dev/null +++ b/extensions/microsoft-authentication/src/node/tokenCache.ts @@ -0,0 +1,31 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { AuthenticationResult, InteractionRequiredAuthError, InteractionRequiredAuthErrorCodes, PublicClientApplication, SilentFlowRequest, TokenCacheContext } from '@azure/msal-node'; +import { SecretStorageCachePlugin } from '../common/cachePlugin'; + +/** Must run inside the PCA's sequencer so cache invalidation completes before the next cache read. */ +export async function acquireTokenSilentWithCache(pca: PublicClientApplication, cachePlugin: SecretStorageCachePlugin, request: SilentFlowRequest): Promise { + for (let attempt = 0; ; attempt++) { + try { + return await pca.acquireTokenSilent(request); + } catch (error) { + if (!(error instanceof InteractionRequiredAuthError) || error.subError !== InteractionRequiredAuthErrorCodes.badToken) { + throw error; + } + + // MSAL removes a rejected refresh token from memory, but does not invoke + // afterCacheAccess on this error path. Persist the removal before another + // silent request reloads the rejected token from SecretStorage. + const tokenCache = pca.getTokenCache(); + await cachePlugin.afterCacheFailure(new TokenCacheContext(tokenCache, tokenCache.hasChanged())); + if (attempt > 0) { + throw error; + } + // A fresh application refresh token can coexist with a rejected family + // refresh token. Give MSAL one chance to use the remaining credential. + } + } +}