Skip to content

Commit a7534cc

Browse files
addaleaxaddaleax
authored
fix: handle ID token expiry time and premature expiration MONGOSH-2145 MONGOSH-2147 (#211)
* fix: use ID token expiry if `passIdTokenAsAccessToken` is set MONGOSH-2145 * 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 4a79c3c commit a7534cc

File tree

7 files changed

+316
-40
lines changed

7 files changed

+316
-40
lines changed

‎src/integration.spec.ts

+13-5
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ async function fetchBrowser({ url }: OpenBrowserOptions): Promise<void> {
3030
(await fetch(url)).body?.resume();
3131
}
3232

33+
function filterConnectionStatus(
34+
status: Record<string, unknown>
35+
): Record<string, unknown> {
36+
// 8.1.0-rc0+ (SERVER-91936) adds and UUID to the response
37+
const { ok, authInfo } = status;
38+
return { ok, authInfo };
39+
}
40+
3341
describe('integration test with mongod', function () {
3442
this.timeout(90_000);
3543

@@ -109,7 +117,7 @@ describe('integration test with mongod', function () {
109117
const status = await client
110118
.db('admin')
111119
.command({ connectionStatus: 1 });
112-
expect(status).to.deep.equal({
120+
expect(filterConnectionStatus(status)).to.deep.equal({
113121
ok: 1,
114122
authInfo: {
115123
authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }],
@@ -184,7 +192,7 @@ describe('integration test with mongod', function () {
184192
const status = await client
185193
.db('admin')
186194
.command({ connectionStatus: 1 });
187-
expect(status).to.deep.equal({
195+
expect(filterConnectionStatus(status)).to.deep.equal({
188196
ok: 1,
189197
authInfo: {
190198
authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }],
@@ -210,7 +218,7 @@ describe('integration test with mongod', function () {
210218
const status = await client
211219
.db('admin')
212220
.command({ connectionStatus: 1 });
213-
expect(status).to.deep.equal({
221+
expect(filterConnectionStatus(status)).to.deep.equal({
214222
ok: 1,
215223
authInfo: {
216224
authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }],
@@ -234,7 +242,7 @@ describe('integration test with mongod', function () {
234242
const status = await client
235243
.db('admin')
236244
.command({ connectionStatus: 1 });
237-
expect(status).to.deep.equal({
245+
expect(filterConnectionStatus(status)).to.deep.equal({
238246
ok: 1,
239247
authInfo: {
240248
authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }],
@@ -270,7 +278,7 @@ describe('integration test with mongod', function () {
270278
const status = await client
271279
.db('admin')
272280
.command({ connectionStatus: 1 });
273-
expect(status).to.deep.equal({
281+
expect(filterConnectionStatus(status)).to.deep.equal({
274282
ok: 1,
275283
authInfo: {
276284
authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }],

‎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

+150-7
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
]);
@@ -905,24 +908,77 @@ describe('OIDC plugin (local OIDC provider)', function () {
905908

906909
describe('automaticRefreshTimeoutMS', function () {
907910
it('returns the correct automatic refresh timeout', function () {
911+
const nowS = Date.now() / 1000;
912+
const nowMS = nowS * 1000;
908913
expect(automaticRefreshTimeoutMS({})).to.equal(undefined);
909-
expect(automaticRefreshTimeoutMS({ expires_in: 10000 })).to.equal(
914+
expect(automaticRefreshTimeoutMS({ expires_at: nowS + 10000 })).to.equal(
910915
undefined
911916
);
912917
expect(
913-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 10000 })
918+
automaticRefreshTimeoutMS(
919+
{
920+
refresh_token: 'asdf',
921+
expires_at: nowS + 10000,
922+
},
923+
undefined,
924+
nowMS
925+
)
914926
).to.equal(9700000);
915927
expect(
916-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 100 })
928+
automaticRefreshTimeoutMS(
929+
{
930+
refresh_token: 'asdf',
931+
expires_at: nowS + 100,
932+
},
933+
undefined,
934+
nowMS
935+
)
917936
).to.equal(50000);
918937
expect(
919-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 10 })
938+
automaticRefreshTimeoutMS(
939+
{
940+
refresh_token: 'asdf',
941+
expires_at: nowS + 100,
942+
id_token: '...',
943+
claims() {
944+
return { exp: nowS + 500 };
945+
},
946+
},
947+
true,
948+
nowMS
949+
)
950+
).to.equal(250000);
951+
expect(
952+
automaticRefreshTimeoutMS(
953+
{
954+
refresh_token: 'asdf',
955+
expires_at: nowS + 100,
956+
id_token: '...',
957+
claims() {
958+
return { exp: nowS + 500 };
959+
},
960+
},
961+
false,
962+
nowMS
963+
)
964+
).to.equal(50000);
965+
expect(
966+
automaticRefreshTimeoutMS({
967+
refresh_token: 'asdf',
968+
expires_at: nowS + 10,
969+
})
920970
).to.equal(undefined);
921971
expect(
922-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 0 })
972+
automaticRefreshTimeoutMS({
973+
refresh_token: 'asdf',
974+
expires_at: nowS + 0,
975+
})
923976
).to.equal(undefined);
924977
expect(
925-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: -10 })
978+
automaticRefreshTimeoutMS({
979+
refresh_token: 'asdf',
980+
expires_at: nowS + -10,
981+
})
926982
).to.equal(undefined);
927983
});
928984
});
@@ -1235,6 +1291,93 @@ describe('OIDC plugin (mock OIDC provider)', function () {
12351291
});
12361292
});
12371293

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

0 commit comments

Comments
 (0)