-
Notifications
You must be signed in to change notification settings - Fork 157
feat: Respect IdP provided session expiry and cap the SDK session #828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
f0cd595
7671118
6e9a51a
aa0a68c
930c5c8
fdf2d3c
64c465f
6ada842
9e76dd6
20c24ae
aee902f
971ca7e
7b28673
09c750a
882b52f
3e2f615
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,9 +36,16 @@ interface AuthorizationParameters { | |
| } | ||
|
|
||
| /** | ||
| * ID Token claims | ||
| * ID Token claims, extended with the IPSIE SL1 session_expiry claim. | ||
| */ | ||
| type IdTokenClaims = IDToken; | ||
| type IdTokenClaims = IDToken & { | ||
| /** | ||
| * Unix timestamp (seconds since epoch) indicating the hard ceiling on the RP | ||
| * session lifetime, as asserted by the upstream enterprise IdP. | ||
| * Present only when the connection has `id_token_session_expiry_supported: true`. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This SDK is a generic OIDC library, not Auth0-specific, but the docs and types here read as Auth0-only. Could we frame this generically, something like "when the OpenID Provider emits a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, that's appropriate.
|
||
| */ | ||
| session_expiry?: number; | ||
| }; | ||
|
|
||
| /** | ||
| * Session object | ||
|
|
@@ -52,6 +59,13 @@ interface Session { | |
| refresh_token: string; | ||
| token_type: string; | ||
| expires_at: string; | ||
| /** | ||
| * Unix timestamp (seconds) of the IdP-asserted session ceiling from the | ||
| * IPSIE SL1 `session_expiry` claim. Present only when the upstream connection | ||
| * has `id_token_session_expiry_supported: true`. The SDK enforces this as a | ||
| * hard expiry on every session read and before any token refresh. | ||
| */ | ||
| sessionExpiresAt?: number; | ||
| [key: string]: any; | ||
| } | ||
|
|
||
|
|
@@ -1163,3 +1177,34 @@ export function claimCheck( | |
| * ``` | ||
| */ | ||
| export function attemptSilentLogin(): RequestHandler; | ||
|
|
||
| /** | ||
| * Error thrown by `accessToken.refresh()` when the IdP-asserted session ceiling | ||
| * (`session_expiry` claim) has passed. Catch this in API routes to redirect the | ||
| * user to re-authenticate, rather than receiving a confusing `invalid_grant` from | ||
| * the token endpoint. | ||
| * | ||
| * ```js | ||
| * const { SessionExpiredError } = require('express-openid-connect'); | ||
| * | ||
| * app.get('/api/data', async (req, res, next) => { | ||
| * try { | ||
| * if (req.oidc.accessToken.isExpired()) { | ||
| * await req.oidc.accessToken.refresh(); | ||
| * } | ||
| * } catch (err) { | ||
| * if (err instanceof SessionExpiredError) { | ||
| * return res.oidc.login(); | ||
| * } | ||
| * return next(err); | ||
| * } | ||
| * }); | ||
| * ``` | ||
| */ | ||
| export class SessionExpiredError extends Error { | ||
| readonly name: 'SessionExpiredError'; | ||
| readonly code: 'ERR_SESSION_EXPIRED'; | ||
| readonly status: 401; | ||
| readonly statusCode: 401; | ||
| constructor(message?: string); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| const auth = require('./middleware/auth'); | ||
| const requiresAuth = require('./middleware/requiresAuth'); | ||
| const attemptSilentLogin = require('./middleware/attemptSilentLogin'); | ||
| const { SessionExpiredError } = require('./lib/errors'); | ||
|
|
||
| module.exports = { | ||
| auth, | ||
| ...requiresAuth, | ||
| attemptSilentLogin, | ||
| SessionExpiredError, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,8 @@ const COOKIES = require('./cookies'); | |
| const { getKeyStore, verifyCookie, signCookie } = require('./crypto'); | ||
| const debug = require('./debug')('appSession'); | ||
|
|
||
| const epoch = () => (Date.now() / 1000) | 0; | ||
| const { epoch } = require('./utils/epoch'); | ||
| const { isSessionExpiryReached } = require('./utils/sessionExpiry'); | ||
| const MAX_COOKIE_SIZE = 4096; | ||
|
|
||
| const REASSIGN = Symbol('reassign'); | ||
|
|
@@ -101,22 +102,36 @@ module.exports = (config) => { | |
| throw lastError; | ||
| } | ||
|
|
||
| function calculateExp(iat, uat) { | ||
| // Calculates the session expiry Unix timestamp (seconds). | ||
| // iat: session issued-at time (Unix seconds) | ||
| // uat: session last-updated-at time (Unix seconds) | ||
| // sessionExpiresAt: optional IdP-asserted ceiling (Unix seconds) from IPSIE session_expiry claim; | ||
| // when present, caps the result so the cookie never outlives the IdP session. | ||
| function calculateExp(iat, uat, sessionExpiresAt) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cookie
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test was already there but only asserted const expSeconds = Math.floor(expDate.getTime() / 1000);
assert.approximately(expSeconds, sessionExpiresAt, 5);This confirms the cookie expiry is pinned to sessionExpiresAt |
||
| let exp; | ||
| if (!rollingEnabled) { | ||
| return iat + absoluteDuration; | ||
| exp = iat + absoluteDuration; | ||
| } else if (!absoluteDuration) { | ||
| exp = uat + rollingDuration; | ||
| } else { | ||
| exp = Math.min(uat + rollingDuration, iat + absoluteDuration); | ||
| } | ||
|
|
||
| if (!absoluteDuration) { | ||
| return uat + rollingDuration; | ||
| if (sessionExpiresAt) { | ||
| exp = Math.min(exp, sessionExpiresAt); | ||
| } | ||
|
|
||
| return Math.min(uat + rollingDuration, iat + absoluteDuration); | ||
| return exp; | ||
| } | ||
|
|
||
| async function setCookie( | ||
| req, | ||
| res, | ||
| { uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }, | ||
| { | ||
| uat = epoch(), | ||
| iat = uat, | ||
| exp = calculateExp(iat, uat, req[sessionName]?.sessionExpiresAt), | ||
| }, | ||
| ) { | ||
| if (res.headersSent) { | ||
| // If headers were already flushed before res.end() the | ||
|
|
@@ -229,7 +244,11 @@ module.exports = (config) => { | |
| id, | ||
| req, | ||
| res, | ||
| { uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }, | ||
| { | ||
| uat = epoch(), | ||
| iat = uat, | ||
| exp = calculateExp(iat, uat, req[sessionName]?.sessionExpiresAt), | ||
| }, | ||
| ) { | ||
| const hasPrevSession = !!req[COOKIES][sessionName]; | ||
| const replacingPrevSession = !!req[REGENERATED_SESSION_ID]; | ||
|
|
@@ -269,7 +288,11 @@ module.exports = (config) => { | |
| id, | ||
| req, | ||
| res, | ||
| { uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }, | ||
| { | ||
| uat = epoch(), | ||
| iat = uat, | ||
| exp = calculateExp(iat, uat, req[sessionName]?.sessionExpiresAt), | ||
| }, | ||
| ) { | ||
| if (res.headersSent) { | ||
| // If headers were already flushed before res.end() the | ||
|
|
@@ -395,6 +418,16 @@ module.exports = (config) => { | |
| ); | ||
| } | ||
|
|
||
| // check that the existing session hasn't passed the IdP-asserted session ceiling | ||
| // (IPSIE SL1 session_expiry claim). Only enforced when the claim was present at login. | ||
| // A 30s leeway guards against clock skew between the SDK and the authorization server. | ||
| if (data.sessionExpiresAt) { | ||
| assert( | ||
| !isSessionExpiryReached(data.sessionExpiresAt), | ||
| 'it is expired based on the session_expiry claim from the upstream IdP', | ||
| ); | ||
| } | ||
|
|
||
| attachSessionObject(req, sessionName, data); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,12 @@ const { | |
| regenerateSessionStoreId, | ||
| replaceSession, | ||
| } = require('../lib/appSession'); | ||
| const { SessionExpiredError } = require('./errors'); | ||
| const { epoch } = require('./utils/epoch'); | ||
| const { | ||
| extractSessionExpiry, | ||
| isSessionExpiryReached, | ||
| } = require('./utils/sessionExpiry'); | ||
|
|
||
| // Caches one RemoteJWKSet instance per auth() configuration for backchannel logout token verification. | ||
| const jwksClientCache = new Map(); | ||
|
|
@@ -143,6 +149,16 @@ function normalizeTokenType(tokenType) { | |
|
|
||
| async function refresh({ tokenEndpointParams } = {}) { | ||
| let { config, req } = weakRef(this); | ||
| const session = req[config.session.name]; | ||
|
|
||
| // Pre-check: do not call /oauth/token if the IdP session ceiling has passed. | ||
| // The authorization server has already revoked the session server-side at this point, so a | ||
| // refresh attempt would fail with invalid_grant anyway. Surfacing a clean | ||
| // SessionExpiredError is better than a confusing token-endpoint error. | ||
| if (isSessionExpiryReached(session?.sessionExpiresAt)) { | ||
| throw new SessionExpiredError(); | ||
| } | ||
|
|
||
| const { configuration } = await getClient(config); | ||
| const oldTokenSet = tokenSet.call(this); | ||
|
|
||
|
|
@@ -157,8 +173,9 @@ async function refresh({ tokenEndpointParams } = {}) { | |
| parameters, | ||
| ); | ||
|
|
||
| // Update the session | ||
| const session = req[config.session.name]; | ||
| // Update the session, preserving sessionExpiresAt — the refresh-token grant | ||
| // does not return a session_expiry claim, so we must not overwrite the ceiling | ||
| // that was set at login. | ||
| Object.assign(session, { | ||
| access_token: newTokenSet.access_token, | ||
| // If no new ID token assume the current ID token is valid. | ||
|
|
@@ -167,8 +184,13 @@ async function refresh({ tokenEndpointParams } = {}) { | |
| refresh_token: newTokenSet.refresh_token || oldTokenSet.refresh_token, | ||
| token_type: normalizeTokenType(newTokenSet.token_type), | ||
| expires_at: newTokenSet.expires_in | ||
| ? Math.floor(Date.now() / 1000) + newTokenSet.expires_in | ||
| ? epoch() + newTokenSet.expires_in | ||
| : undefined, | ||
| // Preserve the IdP session ceiling across refreshes — do not re-derive | ||
| // from the refresh response (it won't contain session_expiry). | ||
| ...(session.sessionExpiresAt !== undefined && { | ||
| sessionExpiresAt: session.sessionExpiresAt, | ||
| }), | ||
| }); | ||
|
|
||
| // Delete the old token set | ||
|
|
@@ -736,14 +758,31 @@ class ResponseContext { | |
| refresh_token: tokenResponse.refresh_token, | ||
| token_type: normalizeTokenType(tokenResponse.token_type), | ||
| expires_at: tokenResponse.expires_in | ||
| ? Math.floor(Date.now() / 1000) + tokenResponse.expires_in | ||
| ? epoch() + tokenResponse.expires_in | ||
| : undefined, | ||
| }; | ||
|
|
||
| // Must store the `sid` separately as the ID Token gets overridden by | ||
| // ID Token from the Refresh Grant which may not contain a sid (In Auth0 currently). | ||
| session.sid = claims?.sid; | ||
|
|
||
| // Persist the session_expiry claim from the ID token as sessionExpiresAt. | ||
| // This is the IdP-asserted ceiling on the RP session lifetime (IPSIE SL1). | ||
| // Only set when the claim is present — absence means the connection does not | ||
| // have id_token_session_expiry_supported enabled, and existing behavior applies. | ||
| const sessionExpiresAt = extractSessionExpiry(claims); | ||
| if (sessionExpiresAt) { | ||
| // Lockout guard: reject a session already past its ceiling at login. | ||
| if (isSessionExpiryReached(sessionExpiresAt)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This lockout compares The point of this guard is to reject a session that is born expired, where Related point:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, that makes sense.
|
||
| throw createError( | ||
| 400, | ||
| 'The session_expiry claim is in the past at login time.', | ||
| { error: 'invalid_session_expiry' }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The login lockout throws a generic A consumer catching
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was an intentional decision, discussion_r3451378840.
|
||
| ); | ||
| } | ||
| session.sessionExpiresAt = sessionExpiresAt; | ||
| } | ||
|
|
||
| // Check if user was previously authenticated BEFORE we modify the session | ||
| const wasAuthenticated = req.oidc.isAuthenticated(); | ||
| const previousSub = wasAuthenticated ? req.oidc.user.sub : null; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| class SessionExpiredError extends Error { | ||
|
kishore7snehil marked this conversation as resolved.
|
||
| constructor(message = 'The upstream IdP session has expired.') { | ||
| super(message); | ||
| this.name = 'SessionExpiredError'; | ||
| this.code = 'ERR_SESSION_EXPIRED'; | ||
|
kishore7snehil marked this conversation as resolved.
|
||
| this.status = 401; | ||
| this.statusCode = 401; | ||
| } | ||
| } | ||
|
|
||
| module.exports = { SessionExpiredError }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| const epoch = () => (Date.now() / 1000) | 0; | ||
|
|
||
| module.exports = { epoch }; |
Uh oh!
There was an error while loading. Please reload this page.