diff --git a/package-lock.json b/package-lock.json index c27be8a..8f4a7bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@mongodb-js/oidc-plugin", - "version": "1.1.6", + "version": "1.1.7", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@mongodb-js/oidc-plugin", - "version": "1.1.6", + "version": "1.1.7", "license": "Apache-2.0", "dependencies": { "express": "^4.18.2", diff --git a/package.json b/package.json index 5014058..e099743 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "email": "compass@mongodb.com" }, "homepage": "https://github.com/mongodb-js/oidc-plugin", - "version": "1.1.6", + "version": "1.1.7", "repository": { "type": "git", "url": "https://github.com/mongodb-js/oidc-plugin.git" diff --git a/src/integration.spec.ts b/src/integration.spec.ts index 1165fed..8c53531 100644 --- a/src/integration.spec.ts +++ b/src/integration.spec.ts @@ -30,6 +30,14 @@ async function fetchBrowser({ url }: OpenBrowserOptions): Promise { (await fetch(url)).body?.resume(); } +function filterConnectionStatus( + status: Record +): Record { + // 8.1.0-rc0+ (SERVER-91936) adds and UUID to the response + const { ok, authInfo } = status; + return { ok, authInfo }; +} + describe('integration test with mongod', function () { this.timeout(90_000); @@ -109,7 +117,7 @@ describe('integration test with mongod', function () { const status = await client .db('admin') .command({ connectionStatus: 1 }); - expect(status).to.deep.equal({ + expect(filterConnectionStatus(status)).to.deep.equal({ ok: 1, authInfo: { authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], @@ -184,7 +192,7 @@ describe('integration test with mongod', function () { const status = await client .db('admin') .command({ connectionStatus: 1 }); - expect(status).to.deep.equal({ + expect(filterConnectionStatus(status)).to.deep.equal({ ok: 1, authInfo: { authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], @@ -210,7 +218,7 @@ describe('integration test with mongod', function () { const status = await client .db('admin') .command({ connectionStatus: 1 }); - expect(status).to.deep.equal({ + expect(filterConnectionStatus(status)).to.deep.equal({ ok: 1, authInfo: { authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], @@ -234,7 +242,7 @@ describe('integration test with mongod', function () { const status = await client .db('admin') .command({ connectionStatus: 1 }); - expect(status).to.deep.equal({ + expect(filterConnectionStatus(status)).to.deep.equal({ ok: 1, authInfo: { authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], @@ -270,7 +278,7 @@ describe('integration test with mongod', function () { const status = await client .db('admin') .command({ connectionStatus: 1 }); - expect(status).to.deep.equal({ + expect(filterConnectionStatus(status)).to.deep.equal({ ok: 1, authInfo: { authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], diff --git a/src/log-hook.ts b/src/log-hook.ts index 1d000c7..ec53c78 100644 --- a/src/log-hook.ts +++ b/src/log-hook.ts @@ -241,7 +241,15 @@ export function hookLoggerToMongoLogWriter( emitter.on( 'mongodb-oidc-plugin:auth-succeeded', - ({ tokenType, refreshToken, expiresAt, passIdTokenAsAccessToken }) => { + ({ + tokenType, + refreshToken, + expiresAt, + passIdTokenAsAccessToken, + forceRefreshOrReauth, + willRetryWithForceRefreshOrReauth, + tokenSetId, + }) => { log.info( 'OIDC-PLUGIN', mongoLogId(1_002_000_017), @@ -252,6 +260,9 @@ export function hookLoggerToMongoLogWriter( refreshToken, expiresAt, passIdTokenAsAccessToken, + forceRefreshOrReauth, + willRetryWithForceRefreshOrReauth, + tokenSetId, } ); } diff --git a/src/plugin.spec.ts b/src/plugin.spec.ts index 5b28358..9b6faf8 100644 --- a/src/plugin.spec.ts +++ b/src/plugin.spec.ts @@ -56,13 +56,15 @@ function requestToken( plugin: MongoDBOIDCPlugin, oidcParams: IdPServerInfo, abortSignal?: OIDCAbortSignal, - username?: string + username?: string, + refreshToken?: string ): ReturnType { return plugin.mongoClientOptions.authMechanismProperties.OIDC_HUMAN_CALLBACK({ timeoutContext: abortSignal, version: 1, idpInfo: oidcParams, username, + refreshToken, }); } @@ -390,6 +392,7 @@ describe('OIDC plugin (local OIDC provider)', function () { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument expect(Object.keys(serializedData.state[0][1]).sort()).to.deep.equal([ 'currentTokenSet', + 'discardingTokenSets', 'lastIdTokenClaims', 'serverOIDCMetadata', ]); @@ -905,24 +908,77 @@ describe('OIDC plugin (local OIDC provider)', function () { describe('automaticRefreshTimeoutMS', function () { it('returns the correct automatic refresh timeout', function () { + const nowS = Date.now() / 1000; + const nowMS = nowS * 1000; expect(automaticRefreshTimeoutMS({})).to.equal(undefined); - expect(automaticRefreshTimeoutMS({ expires_in: 10000 })).to.equal( + expect(automaticRefreshTimeoutMS({ expires_at: nowS + 10000 })).to.equal( undefined ); expect( - automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 10000 }) + automaticRefreshTimeoutMS( + { + refresh_token: 'asdf', + expires_at: nowS + 10000, + }, + undefined, + nowMS + ) ).to.equal(9700000); expect( - automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 100 }) + automaticRefreshTimeoutMS( + { + refresh_token: 'asdf', + expires_at: nowS + 100, + }, + undefined, + nowMS + ) ).to.equal(50000); expect( - automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 10 }) + automaticRefreshTimeoutMS( + { + refresh_token: 'asdf', + expires_at: nowS + 100, + id_token: '...', + claims() { + return { exp: nowS + 500 }; + }, + }, + true, + nowMS + ) + ).to.equal(250000); + expect( + automaticRefreshTimeoutMS( + { + refresh_token: 'asdf', + expires_at: nowS + 100, + id_token: '...', + claims() { + return { exp: nowS + 500 }; + }, + }, + false, + nowMS + ) + ).to.equal(50000); + expect( + automaticRefreshTimeoutMS({ + refresh_token: 'asdf', + expires_at: nowS + 10, + }) ).to.equal(undefined); expect( - automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 0 }) + automaticRefreshTimeoutMS({ + refresh_token: 'asdf', + expires_at: nowS + 0, + }) ).to.equal(undefined); expect( - automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: -10 }) + automaticRefreshTimeoutMS({ + refresh_token: 'asdf', + expires_at: nowS + -10, + }) ).to.equal(undefined); }); }); @@ -1235,6 +1291,93 @@ describe('OIDC plugin (mock OIDC provider)', function () { }); }); + context('when drivers re-request tokens early', function () { + let plugin: MongoDBOIDCPlugin; + + beforeEach(function () { + plugin = createMongoDBOIDCPlugin({ + openBrowserTimeout: 60_000, + openBrowser: fetchBrowser, + allowedFlows: ['auth-code'], + redirectURI: 'http://localhost:0/callback', + }); + }); + + it('will return a different token even if the existing one is not yet expired', async function () { + const result1 = await requestToken(plugin, { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }); + const result2 = await requestToken(plugin, { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }); + const result3 = await requestToken( + plugin, + { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }, + undefined, + undefined, + result2.refreshToken + ); + const result4 = await requestToken( + plugin, + { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }, + undefined, + undefined, + result2.refreshToken + ); + expect(result1).to.deep.equal(result2); + expect(result2.accessToken).not.to.equal(result3.accessToken); + expect(result2.refreshToken).not.to.equal(result3.refreshToken); + expect(result3).to.deep.equal(result4); + }); + + it('will return only one new token per expired token even when called in parallel', async function () { + const result1 = await requestToken(plugin, { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }); + const [result2, result3] = await Promise.all([ + requestToken( + plugin, + { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }, + undefined, + undefined, + result1.refreshToken + ), + requestToken( + plugin, + { + issuer: provider.issuer, + clientId: 'mockclientid', + requestScopes: [], + }, + undefined, + undefined, + result1.refreshToken + ), + ]); + expect(result1.accessToken).not.to.equal(result2.accessToken); + expect(result1.refreshToken).not.to.equal(result2.refreshToken); + expect(result2).to.deep.equal(result3); + }); + }); + context('HTTP request tracking', function () { it('will log all outgoing HTTP requests', async function () { const pluginHttpRequests: string[] = []; diff --git a/src/plugin.ts b/src/plugin.ts index 279f713..5877543 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -10,6 +10,7 @@ import { MongoDBOIDCError } from './types'; import { errorString, getRefreshTokenId, + getStableTokenSetId, messageFromError, normalizeObject, throwIfAborted, @@ -69,6 +70,10 @@ interface UserOIDCAuthState { // A cached Client instance that uses the issuer metadata as discovered // through serverOIDCMetadata. client?: Client; + // A set of refresh token IDs which are currently being rejected, i.e. + // where the driver has called our callback indicating that the corresponding + // access token has become invalid. + discardingTokenSets?: string[]; } // eslint-disable-next-line @typescript-eslint/consistent-type-imports @@ -123,27 +128,53 @@ async function getDefaultOpenBrowser(): Promise< }; } +// Trimmed-down type for easier testing +type TokenSetExpiryInfo = Pick< + TokenSet, + 'refresh_token' | 'id_token' | 'expires_at' +> & { + claims?: () => { exp: number }; +}; + +function tokenExpiryInSeconds( + tokenSet: TokenSetExpiryInfo = {}, + passIdTokenAsAccessToken = false, + now = Date.now() +): number { + // If we have an ID token and are supposed to use it, its `exp` claim + // specifies the token expiry. Otherwise, we assume that the `expires_at` + // value presented by the OIDC provider is correct, since OIDC clients are + // not supposed to inspect access tokens and treat them as opaque. + const expiresAt = + (tokenSet.id_token && + passIdTokenAsAccessToken && + tokenSet.claims?.().exp) || + tokenSet.expires_at || + 0; + return Math.max(0, (expiresAt ?? 0) - now / 1000); +} + /** @internal Exported for testing only */ export function automaticRefreshTimeoutMS( - tokenSet: Pick + tokenSet: TokenSetExpiryInfo, + passIdTokenAsAccessToken = false, + now = Date.now() ): number | undefined { + const expiresIn = tokenExpiryInSeconds( + tokenSet, + passIdTokenAsAccessToken, + now + ); + if (!tokenSet.refresh_token || !expiresIn) return; + // If the tokens expire in more than 1 minute, automatically register // a refresh handler. (They should not expire in less; however, // if we didn't handle that case, we'd run the risk of refreshing very // frequently.) Refresh the token 5 minutes before expiration or // halfway between now and the expiration time, whichever comes later // (expires in 1 hour -> refresh in 55 min, expires in 5 min -> refresh in 2.5 min). - if ( - tokenSet.refresh_token && - tokenSet.expires_in && - tokenSet.expires_in >= 60 /* 1 minute */ - ) { - return ( - Math.max( - tokenSet.expires_in - 300 /* 5 minutes */, - tokenSet.expires_in / 2 - ) * 1000 - ); + if (expiresIn >= 60 /* 1 minute */) { + return Math.max(expiresIn - 300 /* 5 minutes */, expiresIn / 2) * 1000; } } @@ -219,6 +250,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { lastIdTokenClaims: serializedState.lastIdTokenClaims ? { ...serializedState.lastIdTokenClaims } : undefined, + discardingTokenSets: serializedState.discardingTokenSets, }; this.updateStateWithTokenSet( state, @@ -254,6 +286,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { lastIdTokenClaims: state.lastIdTokenClaims ? { ...state.lastIdTokenClaims } : undefined, + discardingTokenSets: state.discardingTokenSets ?? [], }, ] as const; }), @@ -565,7 +598,10 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { const refreshTokenId = getRefreshTokenId(tokenSet.refresh_token); const updateId = updateIdCounter++; - const timerDuration = automaticRefreshTimeoutMS(tokenSet); + const timerDuration = automaticRefreshTimeoutMS( + tokenSet, + this.options.passIdTokenAsAccessToken + ); // Use `.call()` because in browsers, `setTimeout()` requires that it is called // without a `this` value. `.unref()` is not available in browsers either. if (state.timer) this.timers.clearTimeout.call(null, state.timer); @@ -629,6 +665,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { this.logger.emit('mongodb-oidc-plugin:state-updated', { updateId, timerDuration, + tokenSetId: getStableTokenSetId(tokenSet), }); } @@ -798,7 +835,8 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { private async initiateAuthAttempt( state: UserOIDCAuthState, - driverAbortSignal?: OIDCAbortSignal + driverAbortSignal?: OIDCAbortSignal, + { forceRefreshOrReauth = false } = {} ): Promise { throwIfAborted(this.options.signal); throwIfAborted(driverAbortSignal); @@ -819,7 +857,14 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { try { get_tokens: { - if ((state.currentTokenSet?.set?.expires_in ?? 0) > 5 * 60) { + if ( + !forceRefreshOrReauth && + tokenExpiryInSeconds( + state.currentTokenSet?.set, + passIdTokenAsAccessToken + ) > + 5 * 60 + ) { this.logger.emit('mongodb-oidc-plugin:skip-auth-attempt', { reason: 'not-expired', }); @@ -886,6 +931,13 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { const { token_type, expires_at, access_token, id_token, refresh_token } = state.currentTokenSet.set; + const tokenSetId = getStableTokenSetId(state.currentTokenSet.set); + + // We would not want to return the access token or ID token of a token set whose + // accompanying refresh token was passed to us by the driver + const willRetryWithForceRefreshOrReauth = + !forceRefreshOrReauth && + !!state.discardingTokenSets?.includes(tokenSetId); this.logger.emit('mongodb-oidc-plugin:auth-succeeded', { tokenType: token_type ?? null, // DPoP or Bearer @@ -897,11 +949,20 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { idToken: id_token, refreshToken: refresh_token, }, + tokenSetId, + forceRefreshOrReauth, + willRetryWithForceRefreshOrReauth, }); + if (willRetryWithForceRefreshOrReauth) { + return await this.initiateAuthAttempt(state, driverAbortSignal, { + forceRefreshOrReauth: true, + }); + } + return { accessToken: passIdTokenAsAccessToken ? id_token || '' : access_token, - refreshToken: refresh_token, + refreshToken: tokenSetId, // Passing `expiresInSeconds: 0` results in the driver not caching the token. // We perform our own caching here inside the plugin, so interactions with the // cache of the driver are not really required or necessarily helpful. @@ -940,20 +1001,39 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin { username: params.username, }); - if (state.currentAuthAttempt) { - return await state.currentAuthAttempt; + // If the driver called us with a refresh token, that means that its corresponding + // access token has become invalid and we should always return a new one. + if (params.refreshToken) { + (state.discardingTokenSets ??= []).push(params.refreshToken); + this.logger.emit('mongodb-oidc-plugin:discarding-token-set', { + tokenSetId: params.refreshToken, + }); } - const newAuthAttempt = this.initiateAuthAttempt( - state, - params.timeoutContext - ); try { - state.currentAuthAttempt = newAuthAttempt; - return await newAuthAttempt; + if (state.currentAuthAttempt) { + return await state.currentAuthAttempt; + } + + const newAuthAttempt = this.initiateAuthAttempt( + state, + params.timeoutContext + ); + try { + state.currentAuthAttempt = newAuthAttempt; + return await newAuthAttempt; + } finally { + if (state.currentAuthAttempt === newAuthAttempt) + state.currentAuthAttempt = null; + } } finally { - if (state.currentAuthAttempt === newAuthAttempt) - state.currentAuthAttempt = null; + if (params.refreshToken) { + const index = + state.discardingTokenSets?.indexOf(params.refreshToken) ?? -1; + if (index > 0) { + state.discardingTokenSets?.splice(index, 1); + } + } } } diff --git a/src/types.ts b/src/types.ts index af20bd7..7ad6715 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,6 +5,7 @@ export interface MongoDBOIDCLogEventsMap { }) => void; 'mongodb-oidc-plugin:state-updated': (event: { updateId: number; + tokenSetId: string; timerDuration: number | undefined; }) => void; 'mongodb-oidc-plugin:local-redirect-accessed': (event: { @@ -82,6 +83,12 @@ export interface MongoDBOIDCLogEventsMap { idToken: string | undefined; refreshToken: string | undefined; }; + forceRefreshOrReauth: boolean; + willRetryWithForceRefreshOrReauth: boolean; + tokenSetId: string; + }) => void; + 'mongodb-oidc-plugin:discarding-token-set': (event: { + tokenSetId: string; }) => void; 'mongodb-oidc-plugin:destroyed': () => void; 'mongodb-oidc-plugin:missing-id-token': () => void; diff --git a/src/util.ts b/src/util.ts index 427204d..c468309 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,3 +1,4 @@ +import type { TokenSet } from 'openid-client'; import type { OIDCAbortSignal } from './types'; import { createHash, randomBytes } from 'crypto'; @@ -135,3 +136,20 @@ export function getRefreshTokenId( 'debugid:' + createHash('sha256').update(salt).update(token).digest('hex') ); } + +// Identify a token set based on a hash of its contents +export function getStableTokenSetId(tokenSet: TokenSet): string { + const { access_token, id_token, refresh_token, token_type, expires_at } = + tokenSet; + return createHash('sha256') + .update( + JSON.stringify({ + access_token, + id_token, + refresh_token, + token_type, + expires_at, + }) + ) + .digest('hex'); +} diff --git a/test/oidc-test-provider.ts b/test/oidc-test-provider.ts index 115eefd..293f312 100644 --- a/test/oidc-test-provider.ts +++ b/test/oidc-test-provider.ts @@ -335,10 +335,18 @@ export async function functioningAuthCodeBrowserFlow({ await ensureValue(browser, 'input[name="login"]', 'testuser'); await ensureValue(browser, 'input[name="password"]', 'testpassword'); await browser.$('button[type="submit"]').click(); + const idpUrl = await browser.getUrl(); await waitForTitle(browser, 'Authorize'); await browser.$('button[type="submit"][autofocus]').click(); - await waitForLocalhostRedirect(browser); + + // Cannot use `waitForLocalhostRedirect` because we already started on localhost + await browser.waitUntil(async () => { + return ( + new URL((await browser?.getUrl()) ?? 'http://nonexistent').host !== + new URL(idpUrl).host + ); + }); } catch (err: unknown) { await dumpHtml(browser); throw err; @@ -385,6 +393,7 @@ export async function functioningDeviceAuthBrowserFlow({ await waitForTitle(browser, 'Authorize'); await browser.$('button[type="submit"][autofocus]').click(); + await waitForTitle(browser, 'Sign-in Success'); } catch (err: unknown) { await dumpHtml(browser); throw err;