Skip to content

Commit b4e59a2

Browse files
committed
fix: do not return token to driver which is being rejected MONGOSH-2147
Ensure that we do not return tokens from a token set that the driver is currently rejecting (in the sense of calling the OIDC callback again and referring to it via the driver `refreshToken` property).
1 parent 74fbc57 commit b4e59a2

File tree

5 files changed

+185
-15
lines changed

5 files changed

+185
-15
lines changed

src/log-hook.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,15 @@ export function hookLoggerToMongoLogWriter(
241241

242242
emitter.on(
243243
'mongodb-oidc-plugin:auth-succeeded',
244-
({ tokenType, refreshToken, expiresAt, passIdTokenAsAccessToken }) => {
244+
({
245+
tokenType,
246+
refreshToken,
247+
expiresAt,
248+
passIdTokenAsAccessToken,
249+
forceRefreshOrReauth,
250+
willRetryWithForceRefreshOrReauth,
251+
tokenSetId,
252+
}) => {
245253
log.info(
246254
'OIDC-PLUGIN',
247255
mongoLogId(1_002_000_017),
@@ -252,6 +260,9 @@ export function hookLoggerToMongoLogWriter(
252260
refreshToken,
253261
expiresAt,
254262
passIdTokenAsAccessToken,
263+
forceRefreshOrReauth,
264+
willRetryWithForceRefreshOrReauth,
265+
tokenSetId,
255266
}
256267
);
257268
}

src/plugin.spec.ts

+91-1
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,15 @@ function requestToken(
5656
plugin: MongoDBOIDCPlugin,
5757
oidcParams: IdPServerInfo,
5858
abortSignal?: OIDCAbortSignal,
59-
username?: string
59+
username?: string,
60+
refreshToken?: string
6061
): ReturnType<OIDCCallbackFunction> {
6162
return plugin.mongoClientOptions.authMechanismProperties.OIDC_HUMAN_CALLBACK({
6263
timeoutContext: abortSignal,
6364
version: 1,
6465
idpInfo: oidcParams,
6566
username,
67+
refreshToken,
6668
});
6769
}
6870

@@ -390,6 +392,7 @@ describe('OIDC plugin (local OIDC provider)', function () {
390392
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
391393
expect(Object.keys(serializedData.state[0][1]).sort()).to.deep.equal([
392394
'currentTokenSet',
395+
'discardingTokenSets',
393396
'lastIdTokenClaims',
394397
'serverOIDCMetadata',
395398
]);
@@ -1277,6 +1280,93 @@ describe('OIDC plugin (mock OIDC provider)', function () {
12771280
});
12781281
});
12791282

1283+
context('when drivers re-request tokens early', function () {
1284+
let plugin: MongoDBOIDCPlugin;
1285+
1286+
beforeEach(function () {
1287+
plugin = createMongoDBOIDCPlugin({
1288+
openBrowserTimeout: 60_000,
1289+
openBrowser: fetchBrowser,
1290+
allowedFlows: ['auth-code'],
1291+
redirectURI: 'http://localhost:0/callback',
1292+
});
1293+
});
1294+
1295+
it('will return a different token even if the existing one is not yet expired', async function () {
1296+
const result1 = await requestToken(plugin, {
1297+
issuer: provider.issuer,
1298+
clientId: 'mockclientid',
1299+
requestScopes: [],
1300+
});
1301+
const result2 = await requestToken(plugin, {
1302+
issuer: provider.issuer,
1303+
clientId: 'mockclientid',
1304+
requestScopes: [],
1305+
});
1306+
const result3 = await requestToken(
1307+
plugin,
1308+
{
1309+
issuer: provider.issuer,
1310+
clientId: 'mockclientid',
1311+
requestScopes: [],
1312+
},
1313+
undefined,
1314+
undefined,
1315+
result2.refreshToken
1316+
);
1317+
const result4 = await requestToken(
1318+
plugin,
1319+
{
1320+
issuer: provider.issuer,
1321+
clientId: 'mockclientid',
1322+
requestScopes: [],
1323+
},
1324+
undefined,
1325+
undefined,
1326+
result2.refreshToken
1327+
);
1328+
expect(result1).to.deep.equal(result2);
1329+
expect(result2.accessToken).not.to.equal(result3.accessToken);
1330+
expect(result2.refreshToken).not.to.equal(result3.refreshToken);
1331+
expect(result3).to.deep.equal(result4);
1332+
});
1333+
1334+
it('will return only one new token per expired token even when called in parallel', async function () {
1335+
const result1 = await requestToken(plugin, {
1336+
issuer: provider.issuer,
1337+
clientId: 'mockclientid',
1338+
requestScopes: [],
1339+
});
1340+
const [result2, result3] = await Promise.all([
1341+
requestToken(
1342+
plugin,
1343+
{
1344+
issuer: provider.issuer,
1345+
clientId: 'mockclientid',
1346+
requestScopes: [],
1347+
},
1348+
undefined,
1349+
undefined,
1350+
result1.refreshToken
1351+
),
1352+
requestToken(
1353+
plugin,
1354+
{
1355+
issuer: provider.issuer,
1356+
clientId: 'mockclientid',
1357+
requestScopes: [],
1358+
},
1359+
undefined,
1360+
undefined,
1361+
result1.refreshToken
1362+
),
1363+
]);
1364+
expect(result1.accessToken).not.to.equal(result2.accessToken);
1365+
expect(result1.refreshToken).not.to.equal(result2.refreshToken);
1366+
expect(result2).to.deep.equal(result3);
1367+
});
1368+
});
1369+
12801370
context('HTTP request tracking', function () {
12811371
it('will log all outgoing HTTP requests', async function () {
12821372
const pluginHttpRequests: string[] = [];

src/plugin.ts

+58-13
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { MongoDBOIDCError } from './types';
1010
import {
1111
errorString,
1212
getRefreshTokenId,
13+
getStableTokenSetId,
1314
messageFromError,
1415
normalizeObject,
1516
throwIfAborted,
@@ -69,6 +70,10 @@ interface UserOIDCAuthState {
6970
// A cached Client instance that uses the issuer metadata as discovered
7071
// through serverOIDCMetadata.
7172
client?: Client;
73+
// A set of refresh token IDs which are currently being rejected, i.e.
74+
// where the driver has called our callback indicating that the corresponding
75+
// access token has become invalid.
76+
discardingTokenSets?: string[];
7277
}
7378

7479
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
@@ -239,6 +244,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
239244
lastIdTokenClaims: serializedState.lastIdTokenClaims
240245
? { ...serializedState.lastIdTokenClaims }
241246
: undefined,
247+
discardingTokenSets: serializedState.discardingTokenSets,
242248
};
243249
this.updateStateWithTokenSet(
244250
state,
@@ -274,6 +280,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
274280
lastIdTokenClaims: state.lastIdTokenClaims
275281
? { ...state.lastIdTokenClaims }
276282
: undefined,
283+
discardingTokenSets: state.discardingTokenSets ?? [],
277284
},
278285
] as const;
279286
}),
@@ -652,6 +659,7 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
652659
this.logger.emit('mongodb-oidc-plugin:state-updated', {
653660
updateId,
654661
timerDuration,
662+
tokenSetId: getStableTokenSetId(tokenSet),
655663
});
656664
}
657665

@@ -821,7 +829,8 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
821829

822830
private async initiateAuthAttempt(
823831
state: UserOIDCAuthState,
824-
driverAbortSignal?: OIDCAbortSignal
832+
driverAbortSignal?: OIDCAbortSignal,
833+
{ forceRefreshOrReauth = false } = {}
825834
): Promise<IdPServerResponse> {
826835
throwIfAborted(this.options.signal);
827836
throwIfAborted(driverAbortSignal);
@@ -843,11 +852,12 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
843852
try {
844853
get_tokens: {
845854
if (
855+
!forceRefreshOrReauth &&
846856
tokenExpiryInSeconds(
847857
state.currentTokenSet?.set,
848858
passIdTokenAsAccessToken
849859
) >
850-
5 * 60
860+
5 * 60
851861
) {
852862
this.logger.emit('mongodb-oidc-plugin:skip-auth-attempt', {
853863
reason: 'not-expired',
@@ -915,6 +925,13 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
915925

916926
const { token_type, expires_at, access_token, id_token, refresh_token } =
917927
state.currentTokenSet.set;
928+
const tokenSetId = getStableTokenSetId(state.currentTokenSet.set);
929+
930+
// We would not want to return the access token or ID token of a token set whose
931+
// accompanying refresh token was passed to us by
932+
const willRetryWithForceRefreshOrReauth =
933+
!forceRefreshOrReauth &&
934+
!!state.discardingTokenSets?.includes(tokenSetId);
918935

919936
this.logger.emit('mongodb-oidc-plugin:auth-succeeded', {
920937
tokenType: token_type ?? null, // DPoP or Bearer
@@ -926,11 +943,20 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
926943
idToken: id_token,
927944
refreshToken: refresh_token,
928945
},
946+
tokenSetId,
947+
forceRefreshOrReauth,
948+
willRetryWithForceRefreshOrReauth,
929949
});
930950

951+
if (willRetryWithForceRefreshOrReauth) {
952+
return await this.initiateAuthAttempt(state, driverAbortSignal, {
953+
forceRefreshOrReauth: true,
954+
});
955+
}
956+
931957
return {
932958
accessToken: passIdTokenAsAccessToken ? id_token || '' : access_token,
933-
refreshToken: refresh_token,
959+
refreshToken: tokenSetId,
934960
// Passing `expiresInSeconds: 0` results in the driver not caching the token.
935961
// We perform our own caching here inside the plugin, so interactions with the
936962
// cache of the driver are not really required or necessarily helpful.
@@ -969,20 +995,39 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
969995
username: params.username,
970996
});
971997

972-
if (state.currentAuthAttempt) {
973-
return await state.currentAuthAttempt;
998+
// If the driver called us with a refresh token, that means that its corresponding
999+
// access token has become invalid and we should always return a new one.
1000+
if (params.refreshToken) {
1001+
(state.discardingTokenSets ??= []).push(params.refreshToken);
1002+
this.logger.emit('mongodb-oidc-plugin:discarding-token-set', {
1003+
tokenSetId: params.refreshToken,
1004+
});
9741005
}
9751006

976-
const newAuthAttempt = this.initiateAuthAttempt(
977-
state,
978-
params.timeoutContext
979-
);
9801007
try {
981-
state.currentAuthAttempt = newAuthAttempt;
982-
return await newAuthAttempt;
1008+
if (state.currentAuthAttempt) {
1009+
return await state.currentAuthAttempt;
1010+
}
1011+
1012+
const newAuthAttempt = this.initiateAuthAttempt(
1013+
state,
1014+
params.timeoutContext
1015+
);
1016+
try {
1017+
state.currentAuthAttempt = newAuthAttempt;
1018+
return await newAuthAttempt;
1019+
} finally {
1020+
if (state.currentAuthAttempt === newAuthAttempt)
1021+
state.currentAuthAttempt = null;
1022+
}
9831023
} finally {
984-
if (state.currentAuthAttempt === newAuthAttempt)
985-
state.currentAuthAttempt = null;
1024+
if (params.refreshToken) {
1025+
const index =
1026+
state.discardingTokenSets?.indexOf(params.refreshToken) ?? -1;
1027+
if (index > 0) {
1028+
state.discardingTokenSets?.splice(index, 1);
1029+
}
1030+
}
9861031
}
9871032
}
9881033

src/types.ts

+7
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export interface MongoDBOIDCLogEventsMap {
55
}) => void;
66
'mongodb-oidc-plugin:state-updated': (event: {
77
updateId: number;
8+
tokenSetId: string;
89
timerDuration: number | undefined;
910
}) => void;
1011
'mongodb-oidc-plugin:local-redirect-accessed': (event: {
@@ -82,6 +83,12 @@ export interface MongoDBOIDCLogEventsMap {
8283
idToken: string | undefined;
8384
refreshToken: string | undefined;
8485
};
86+
forceRefreshOrReauth: boolean;
87+
willRetryWithForceRefreshOrReauth: boolean;
88+
tokenSetId: string;
89+
}) => void;
90+
'mongodb-oidc-plugin:discarding-token-set': (event: {
91+
tokenSetId: string;
8592
}) => void;
8693
'mongodb-oidc-plugin:destroyed': () => void;
8794
'mongodb-oidc-plugin:missing-id-token': () => void;

src/util.ts

+17
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { TokenSet } from 'openid-client';
12
import type { OIDCAbortSignal } from './types';
23
import { createHash, randomBytes } from 'crypto';
34

@@ -135,3 +136,19 @@ export function getRefreshTokenId(
135136
'debugid:' + createHash('sha256').update(salt).update(token).digest('hex')
136137
);
137138
}
139+
140+
export function getStableTokenSetId(tokenSet: TokenSet): string {
141+
const { access_token, id_token, refresh_token, token_type, expires_at } =
142+
tokenSet;
143+
return createHash('sha256')
144+
.update(
145+
JSON.stringify({
146+
access_token,
147+
id_token,
148+
refresh_token,
149+
token_type,
150+
expires_at,
151+
})
152+
)
153+
.digest('hex');
154+
}

0 commit comments

Comments
 (0)