Skip to content

Commit e7af23b

Browse files
authored
fix: support IdPs with 5 min token expiry and no refresh flow MONGOSH-2498 (#232)
Before this change, our OIDC plugin could exhibit undesirable behavior in a specific configuration edge case, which, unfortunately, matches the default behavior of at least one identity provider software. Specifically, Ping Identity software does not typically provide us with JWTs that are usable for authentication and authorization, so users may opt to use ID tokens instead. Ping, however, by default does not provide clients with new ID tokens on a refresh operations, and also only gives those tokens a default validity of 5 minutes. (We do recommend to Ping customers to enable passing new ID tokens on refresh if they have this setup.) However, 5 minutes also happens to be the time at which the plugin prefers either refreshing tokens or prompting the user for re-authentication instead of re-using tokens with a short leftover validity. This means that when the driver requests a new token multiple times, or multiple driver instances request tokens in quick succession (which can easily happen, especially if they used the same token beforehand, which obviously then expires at around the same time for each driver), the plugin would try to refresh the token, then when that failed, prompt the user for a new authentication attempt not just once every minutes, but multiple times in quick succession as well. To avoid this issue, we refactor the code that chooses a token acquisition mechanism so that even if the current token still has a leftover validity of less than 5 minutes and refreshing fails, we still keep using it until the driver signals to us that it has become fully unusable.
1 parent 24f095e commit e7af23b

File tree

3 files changed

+63
-9
lines changed

3 files changed

+63
-9
lines changed

src/plugin.spec.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,6 +1299,41 @@ describe('OIDC plugin (mock OIDC provider)', function () {
12991299
additionalIssuerMetadata = undefined;
13001300
});
13011301

1302+
context('with a broken token refresh flow', function () {
1303+
let plugin: MongoDBOIDCPlugin;
1304+
1305+
beforeEach(function () {
1306+
plugin = createMongoDBOIDCPlugin({
1307+
openBrowserTimeout: 60_000,
1308+
openBrowser: fetchBrowser,
1309+
allowedFlows: ['auth-code'],
1310+
redirectURI: 'http://localhost:0/callback',
1311+
});
1312+
1313+
overrideRequestHandler = (url, req, res) => {
1314+
if (new URL(url).searchParams.get('grant_type') === 'refresh_token') {
1315+
res.writeHead(500);
1316+
res.end('Internal Server Error');
1317+
}
1318+
};
1319+
});
1320+
1321+
it('will keep using an unexpired token if the refresh endpoint is broken even if refresh is overdue', async function () {
1322+
getTokenPayload = () => {
1323+
return { ...tokenPayload, expires_in: 60 /* one minute */ };
1324+
};
1325+
const req = async () =>
1326+
await requestToken(plugin, {
1327+
issuer: provider.issuer,
1328+
clientId: 'mockclientid',
1329+
requestScopes: [],
1330+
});
1331+
const result1 = await req();
1332+
const result2 = await req();
1333+
expect(result1).to.deep.equal(result2);
1334+
});
1335+
});
1336+
13021337
context('with different supported built-in scopes', function () {
13031338
let getScopes: () => Promise<string[]>;
13041339

src/plugin.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -964,27 +964,46 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
964964

965965
try {
966966
get_tokens: {
967-
if (
968-
!forceRefreshOrReauth &&
969-
tokenExpiryInSeconds(
970-
state.currentTokenSet?.set,
971-
passIdTokenAsAccessToken
972-
) >
973-
5 * 60
974-
) {
967+
const currentSetExpiresInSeconds = tokenExpiryInSeconds(
968+
state.currentTokenSet?.set,
969+
passIdTokenAsAccessToken
970+
);
971+
// If the current token set has a decent amount of validity, just keep using it.
972+
if (!forceRefreshOrReauth && currentSetExpiresInSeconds > 5 * 60) {
975973
this.logger.emit('mongodb-oidc-plugin:skip-auth-attempt', {
976974
authStateId: state.id,
977975
reason: 'not-expired',
978976
});
979977
break get_tokens;
980978
}
979+
// If the current token set is close to expiry, try to acquire a fresh token
980+
// if possible.
981981
if (await state.currentTokenSet?.tryRefresh?.()) {
982982
this.logger.emit('mongodb-oidc-plugin:skip-auth-attempt', {
983983
authStateId: state.id,
984984
reason: 'refresh-succeeded',
985985
});
986986
break get_tokens;
987987
}
988+
// If the current token set is close to expiry, and refreshing failed, that can
989+
// just mean that the token refresh mechanism are not available.
990+
// We want to avoid a situation in which two initiateAuthAttempt() calls are made
991+
// right after each other, the token returned from the first one has a short validity
992+
// (i.e. up to or less than 5 minutes), and the second one then ignores the token
993+
// from the first one, effectively keeping the user in a loop of 'overly' eager
994+
// re-authentication interactions.
995+
// This means we effectively have a minimum requirement of tokens being valid
996+
// for at least 10 seconds after being issued OR a working token refresh flow, which
997+
// seems like a reasonable expectation.
998+
if (!forceRefreshOrReauth && currentSetExpiresInSeconds > 10) {
999+
this.logger.emit('mongodb-oidc-plugin:skip-auth-attempt', {
1000+
authStateId: state.id,
1001+
reason: 'not-expired-refresh-failed',
1002+
});
1003+
break get_tokens;
1004+
}
1005+
// If no automatic mechanism for acquiring a token is available, we need to start
1006+
// an authentication flow that (typically) involves user interaction.
9881007
state.currentTokenSet = null;
9891008
let error;
9901009
const currentAllowedFlowSet = await this.getAllowedFlows({ signal });

src/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export interface MongoDBOIDCLogEventsMap {
8181
}) => void;
8282
'mongodb-oidc-plugin:skip-auth-attempt': (event: {
8383
authStateId: string;
84-
reason: string;
84+
reason: 'not-expired' | 'not-expired-refresh-failed' | 'refresh-succeeded';
8585
}) => void;
8686
'mongodb-oidc-plugin:auth-failed': (event: {
8787
authStateId: string;

0 commit comments

Comments
 (0)