Skip to content

Commit 74fbc57

Browse files
committed
fix: use ID token expiry if passIdTokenAsAccessToken is set MONGOSH-2145
1 parent 4a79c3c commit 74fbc57

File tree

2 files changed

+91
-20
lines changed

2 files changed

+91
-20
lines changed

src/plugin.spec.ts

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -905,24 +905,66 @@ describe('OIDC plugin (local OIDC provider)', function () {
905905

906906
describe('automaticRefreshTimeoutMS', function () {
907907
it('returns the correct automatic refresh timeout', function () {
908+
const now = () => Date.now() / 1000;
908909
expect(automaticRefreshTimeoutMS({})).to.equal(undefined);
909-
expect(automaticRefreshTimeoutMS({ expires_in: 10000 })).to.equal(
910+
expect(automaticRefreshTimeoutMS({ expires_at: now() + 10000 })).to.equal(
910911
undefined
911912
);
912913
expect(
913-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 10000 })
914+
automaticRefreshTimeoutMS({
915+
refresh_token: 'asdf',
916+
expires_at: now() + 10000,
917+
})
914918
).to.equal(9700000);
915919
expect(
916-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 100 })
920+
automaticRefreshTimeoutMS({
921+
refresh_token: 'asdf',
922+
expires_at: now() + 100,
923+
})
917924
).to.equal(50000);
918925
expect(
919-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 10 })
926+
automaticRefreshTimeoutMS(
927+
{
928+
refresh_token: 'asdf',
929+
expires_at: now() + 100,
930+
id_token: '...',
931+
claims() {
932+
return { exp: now() + 500 };
933+
},
934+
},
935+
true
936+
)
937+
).to.equal(250000);
938+
expect(
939+
automaticRefreshTimeoutMS(
940+
{
941+
refresh_token: 'asdf',
942+
expires_at: now() + 100,
943+
id_token: '...',
944+
claims() {
945+
return { exp: now() + 500 };
946+
},
947+
},
948+
false
949+
)
950+
).to.equal(50000);
951+
expect(
952+
automaticRefreshTimeoutMS({
953+
refresh_token: 'asdf',
954+
expires_at: now() + 10,
955+
})
920956
).to.equal(undefined);
921957
expect(
922-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: 0 })
958+
automaticRefreshTimeoutMS({
959+
refresh_token: 'asdf',
960+
expires_at: now() + 0,
961+
})
923962
).to.equal(undefined);
924963
expect(
925-
automaticRefreshTimeoutMS({ refresh_token: 'asdf', expires_in: -10 })
964+
automaticRefreshTimeoutMS({
965+
refresh_token: 'asdf',
966+
expires_at: now() + -10,
967+
})
926968
).to.equal(undefined);
927969
});
928970
});

src/plugin.ts

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,27 +123,47 @@ async function getDefaultOpenBrowser(): Promise<
123123
};
124124
}
125125

126+
// Trimmed-down type for easier testing
127+
type TokenSetExpiryInfo = Pick<
128+
TokenSet,
129+
'refresh_token' | 'id_token' | 'expires_at'
130+
> & {
131+
claims?: () => { exp: number };
132+
};
133+
134+
function tokenExpiryInSeconds(
135+
tokenSet: TokenSetExpiryInfo = {},
136+
passIdTokenAsAccessToken = false
137+
): number {
138+
// If we have an ID token and are supposed to use it, its `exp` claim
139+
// specifies the token expiry. Otherwise, we assume that the `expires_at`
140+
// value presented by the OIDC provider is correct, since OIDC clients are
141+
// not supposed to inspect access tokens and treat them as opaque.
142+
const expiresAt =
143+
(tokenSet.id_token &&
144+
passIdTokenAsAccessToken &&
145+
tokenSet.claims?.().exp) ||
146+
tokenSet.expires_at ||
147+
0;
148+
return Math.max(0, (expiresAt ?? 0) - Date.now() / 1000);
149+
}
150+
126151
/** @internal Exported for testing only */
127152
export function automaticRefreshTimeoutMS(
128-
tokenSet: Pick<TokenSet, 'refresh_token' | 'expires_in'>
153+
tokenSet: TokenSetExpiryInfo,
154+
passIdTokenAsAccessToken = false
129155
): number | undefined {
156+
const expiresIn = tokenExpiryInSeconds(tokenSet, passIdTokenAsAccessToken);
157+
if (!tokenSet.refresh_token || !expiresIn) return;
158+
130159
// If the tokens expire in more than 1 minute, automatically register
131160
// a refresh handler. (They should not expire in less; however,
132161
// if we didn't handle that case, we'd run the risk of refreshing very
133162
// frequently.) Refresh the token 5 minutes before expiration or
134163
// halfway between now and the expiration time, whichever comes later
135164
// (expires in 1 hour -> refresh in 55 min, expires in 5 min -> refresh in 2.5 min).
136-
if (
137-
tokenSet.refresh_token &&
138-
tokenSet.expires_in &&
139-
tokenSet.expires_in >= 60 /* 1 minute */
140-
) {
141-
return (
142-
Math.max(
143-
tokenSet.expires_in - 300 /* 5 minutes */,
144-
tokenSet.expires_in / 2
145-
) * 1000
146-
);
165+
if (expiresIn >= 60 /* 1 minute */) {
166+
return Math.max(expiresIn - 300 /* 5 minutes */, expiresIn / 2) * 1000;
147167
}
148168
}
149169

@@ -565,7 +585,10 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
565585
const refreshTokenId = getRefreshTokenId(tokenSet.refresh_token);
566586
const updateId = updateIdCounter++;
567587

568-
const timerDuration = automaticRefreshTimeoutMS(tokenSet);
588+
const timerDuration = automaticRefreshTimeoutMS(
589+
tokenSet,
590+
this.options.passIdTokenAsAccessToken
591+
);
569592
// Use `.call()` because in browsers, `setTimeout()` requires that it is called
570593
// without a `this` value. `.unref()` is not available in browsers either.
571594
if (state.timer) this.timers.clearTimeout.call(null, state.timer);
@@ -819,7 +842,13 @@ export class MongoDBOIDCPluginImpl implements MongoDBOIDCPlugin {
819842

820843
try {
821844
get_tokens: {
822-
if ((state.currentTokenSet?.set?.expires_in ?? 0) > 5 * 60) {
845+
if (
846+
tokenExpiryInSeconds(
847+
state.currentTokenSet?.set,
848+
passIdTokenAsAccessToken
849+
) >
850+
5 * 60
851+
) {
823852
this.logger.emit('mongodb-oidc-plugin:skip-auth-attempt', {
824853
reason: 'not-expired',
825854
});

0 commit comments

Comments
 (0)