-
Notifications
You must be signed in to change notification settings - Fork 173
feat: enforce IPSIE session_expiry ceiling in credentials managers #983
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 all commits
0616347
36a60c8
bb44ffc
dd74bac
d0785cc
4f77b20
f48089d
6a90faf
5fa64c1
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 |
|---|---|---|
|
|
@@ -28,6 +28,19 @@ public abstract class BaseCredentialsManager internal constructor( | |
|
|
||
| @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) | ||
| internal const val KEY_TOKEN_TYPE = "com.auth0.token_type" | ||
|
|
||
| /** | ||
| * Storage key for the IPSIE `session_expiry` ceiling (Unix seconds), persisted at login so it | ||
| * survives a refresh whose ID token does not re-emit the claim. See [isSessionExpired]. | ||
| */ | ||
| @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) | ||
| internal const val KEY_SESSION_EXPIRY = "com.auth0.session_expiry" | ||
|
|
||
| /** | ||
| * Negative clock-skew leeway (seconds) applied when checking the `session_expiry` ceiling, so | ||
| * the session is treated as expired slightly *before* the wall-clock ceiling, never after. | ||
| */ | ||
| private const val SESSION_EXPIRY_LEEWAY_SECONDS = 30L | ||
|
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. Is 30s is the agreed leeway across ?
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 — 30s aligns implementation ,the ~30s negative leeway. The leeway is intentionally negative so we treat the session as expired slightly before the wall-clock ceiling, never after. |
||
| } | ||
|
|
||
| private var _clock: Clock = ClockImpl() | ||
|
|
@@ -302,6 +315,93 @@ public abstract class BaseCredentialsManager internal constructor( | |
| return expiresAt <= currentTimeInMillis | ||
| } | ||
|
|
||
| /** | ||
| * Reads the IPSIE `session_expiry` ceiling (Unix seconds) from the given ID token, or null when | ||
| * the token is absent/unparseable or does not carry the claim. | ||
| */ | ||
| private fun sessionExpiryFromIdToken(idToken: String?): Long? { | ||
| if (idToken.isNullOrBlank()) return null | ||
| return runCatching { jwtDecoder.decode(idToken).sessionExpiry }.getOrNull() | ||
| } | ||
|
|
||
| /** | ||
| * Checks whether the upstream-IdP session ceiling (`session_expiry`) has been reached. | ||
| * | ||
| * The ceiling is resolved in order: (1) the value pinned at login under [KEY_SESSION_EXPIRY]; | ||
| * (2) the live claim in [idToken], as a fallback only when nothing is pinned yet (migration of a | ||
| * session saved before this control existed); (3) if neither is present there is no ceiling and | ||
| * the session is NOT expired — a missing value must fall through to existing behavior, never be | ||
| * treated as already-expired. The pinned value is read first because the ceiling is fixed at the | ||
| * initial login: a refresh whose ID token re-emits a *later* `session_expiry` must never raise it. | ||
| * | ||
| * A small negative clock-skew leeway is applied so the session is treated as expired slightly | ||
| * before the wall-clock ceiling, never after. | ||
| */ | ||
| protected fun isSessionExpired(idToken: String?): Boolean { | ||
| // A non-positive value is not a valid Unix timestamp; treat it as "not pinned"/"no ceiling" | ||
| // (mirrors the unset/migration guard in [willExpire]) so a 0/negative stored sentinel falls | ||
| // through to the live claim rather than fail-open as "no ceiling". | ||
| val sessionExpiry = storage.retrieveLong(KEY_SESSION_EXPIRY)?.takeIf { it > 0 } | ||
| ?: sessionExpiryFromIdToken(idToken)?.takeIf { it > 0 } | ||
| ?: return false | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| val nowSeconds = currentTimeInMillis / 1000 | ||
| return nowSeconds + SESSION_EXPIRY_LEEWAY_SECONDS >= sessionExpiry | ||
| } | ||
|
|
||
| /** | ||
| * Stamps the pinned `session_expiry` ceiling (the value persisted at login under | ||
| * [KEY_SESSION_EXPIRY]) onto [credentials] so its public `sessionExpiresAt` reflects the value | ||
| * the SDK actually enforces, rather than re-decoding the live ID token — which would diverge | ||
| * after a refresh whose token omits or re-emits the claim. No-op when nothing is pinned, so the | ||
| * getter falls back to the token claim. Returns the same instance for call-site convenience. | ||
| */ | ||
| protected fun stampPinnedSessionExpiry(credentials: Credentials): Credentials { | ||
| storage.retrieveLong(KEY_SESSION_EXPIRY)?.takeIf { it > 0 }?.let { | ||
| credentials.pinnedSessionExpiresAt = it | ||
| } | ||
| return credentials | ||
| } | ||
|
|
||
| /** | ||
| * Pins the `session_expiry` ceiling from the initial login and preserves it across refreshes. | ||
| * | ||
| * The ceiling is read once and stamped onto the session at login: it is stored only when no value | ||
| * is already persisted. A `session_expiry` re-emitted on a later (refresh) grant is deliberately | ||
| * ignored, so the bound stays pinned to the initial-login value and a refresh can never extend the | ||
| * session past it. [clearCredentials] removes the stored value on logout, so the next login re-pins | ||
| * a fresh ceiling. Call from `saveCredentials`. | ||
| */ | ||
| protected fun persistSessionExpiry(idToken: String?) { | ||
| val incoming = sessionExpiryFromIdToken(idToken) ?: return | ||
| // A positive value is already pinned from the initial login -> keep it; ignore the claim | ||
| // re-emitted on this (refresh) grant. A null/non-positive stored value means nothing is pinned | ||
| // yet (mirrors the unset/migration guard in [isSessionExpired]), so stamp the ceiling now. | ||
| val pinned = storage.retrieveLong(KEY_SESSION_EXPIRY) | ||
| if (pinned != null && pinned > 0) return | ||
|
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. Shouldn't this check be before the |
||
| storage.store(KEY_SESSION_EXPIRY, incoming) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Validates, at session-creation time, that the given ID token is not already past its | ||
| * `session_expiry` ceiling. Throws [CredentialsManagerException] with code `SESSION_EXPIRED` | ||
| * when it is, so an already-expired session is never persisted. No-op when the token is absent | ||
| * or does not carry both `session_expiry` and `iat`. | ||
| * | ||
| * The same [SESSION_EXPIRY_LEEWAY_SECONDS] leeway used by [isSessionExpired] is applied here so | ||
| * the two checks agree: a ceiling within the leeway of `iat` is rejected up front rather than | ||
| * being persisted only to be treated as expired on the very next read. | ||
| */ | ||
| @Throws(CredentialsManagerException::class) | ||
| protected fun validateSessionExpiryAtCreation(idToken: String?) { | ||
| if (idToken.isNullOrBlank()) return | ||
| val jwt = runCatching { jwtDecoder.decode(idToken) }.getOrNull() ?: return | ||
| val sessionExpiry = jwt.sessionExpiry ?: return | ||
| val issuedAtSeconds = jwt.issuedAt?.time?.div(1000) ?: return | ||
| if (sessionExpiry <= issuedAtSeconds + SESSION_EXPIRY_LEEWAY_SECONDS) { | ||
| throw CredentialsManagerException.SESSION_EXPIRED | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns the key for storing the APICredentials in storage. Uses a combination of audience and scope. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,18 +63,27 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| * Stores the given credentials in the storage. Must have an access_token or id_token and a expires_in value. | ||
| * | ||
| * @param credentials the credentials to save in the storage. | ||
| * @throws CredentialsManagerException with code `SESSION_EXPIRED` if the credentials carry an | ||
| * IPSIE `session_expiry` claim that is already past its ceiling at creation time, or with code | ||
| * `INVALID_CREDENTIALS` if neither an access_token nor an id_token is present. | ||
| */ | ||
| @Throws(CredentialsManagerException::class) | ||
| override fun saveCredentials(credentials: Credentials) { | ||
| if (TextUtils.isEmpty(credentials.accessToken) && TextUtils.isEmpty(credentials.idToken)) { | ||
| throw CredentialsManagerException.INVALID_CREDENTIALS | ||
| } | ||
| // IPSIE session_expiry: reject a session already past its ceiling at creation time. | ||
| validateSessionExpiryAtCreation(credentials.idToken) | ||
|
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. validateSessionExpiryAtCreation now throws an exception . Update the comment and signature of saveCredentials for the same
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. Done. Added @throws(CredentialsManagerException::class) and documented that saveCredentials can now throw SESSION_EXPIRED (when the ID token is already past its session_expiry ceiling at creation) and INVALID_CREDENTIALS. |
||
| storage.store(KEY_ACCESS_TOKEN, credentials.accessToken) | ||
| storage.store(KEY_REFRESH_TOKEN, credentials.refreshToken) | ||
| storage.store(KEY_ID_TOKEN, credentials.idToken) | ||
| storage.store(KEY_TOKEN_TYPE, credentials.type) | ||
| storage.store(KEY_EXPIRES_AT, credentials.expiresAt.time) | ||
| storage.store(KEY_SCOPE, credentials.scope) | ||
| storage.store(LEGACY_KEY_CACHE_EXPIRES_AT, credentials.expiresAt.time) | ||
| // Preserve the session_expiry ceiling across refreshes: only ever written, never cleared, | ||
| // so a refresh whose ID token omits the claim does not silently remove the limit. | ||
| persistSessionExpiry(credentials.idToken) | ||
| saveDPoPThumbprint(credentials) | ||
| } | ||
|
|
||
|
|
@@ -129,6 +138,13 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| ) { | ||
| serialExecutor.execute { | ||
| runCatchingOnExecutor(callback) { | ||
| // IPSIE session_expiry: enforce the upstream-IdP session ceiling before exchanging the | ||
| // refresh token, so the SSO exchange is never used to outlive the session. | ||
| if (isSessionExpired(storage.retrieveString(KEY_ID_TOKEN))) { | ||
| clearCredentials() | ||
| callback.onFailure(CredentialsManagerException.SESSION_EXPIRED) | ||
| return@execute | ||
| } | ||
| val refreshToken = storage.retrieveString(KEY_REFRESH_TOKEN) | ||
| if (refreshToken.isNullOrEmpty()) { | ||
| callback.onFailure(CredentialsManagerException.NO_REFRESH_TOKEN) | ||
|
|
@@ -462,17 +478,27 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| callback.onFailure(CredentialsManagerException.NO_CREDENTIALS) | ||
| return@execute | ||
| } | ||
| // IPSIE session_expiry: enforce the upstream-IdP session ceiling before serving any | ||
| // cached token or attempting a refresh. Past the ceiling, clear and surface the error | ||
| // so the refresh-token grant is never used to outlive the session. | ||
| if (isSessionExpired(idToken)) { | ||
| clearCredentials() | ||
| callback.onFailure(CredentialsManagerException.SESSION_EXPIRED) | ||
| return@execute | ||
| } | ||
| val willAccessTokenExpire = willExpire(expiresAt!!, minTtl.toLong()) | ||
| val scopeChanged = hasScopeChanged(storedScope, scope) | ||
| if (!forceRefresh && !willAccessTokenExpire && !scopeChanged) { | ||
| callback.onSuccess( | ||
| recreateCredentials( | ||
| idToken.orEmpty(), | ||
| accessToken.orEmpty(), | ||
| tokenType.orEmpty(), | ||
| refreshToken, | ||
| Date(expiresAt), | ||
| storedScope | ||
| stampPinnedSessionExpiry( | ||
| recreateCredentials( | ||
| idToken.orEmpty(), | ||
| accessToken.orEmpty(), | ||
| tokenType.orEmpty(), | ||
| refreshToken, | ||
| Date(expiresAt), | ||
| storedScope | ||
| ) | ||
| ) | ||
| ) | ||
| return@execute | ||
|
|
@@ -525,7 +551,7 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| fresh.scope | ||
| ) | ||
| saveCredentials(credentials) | ||
| callback.onSuccess(credentials) | ||
| callback.onSuccess(stampPinnedSessionExpiry(credentials)) | ||
|
kishore7snehil marked this conversation as resolved.
|
||
| } catch (error: AuthenticationException) { | ||
| if (error.isMultifactorRequired) { | ||
| callback.onFailure( | ||
|
|
@@ -580,6 +606,13 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| ) { | ||
| serialExecutor.execute { | ||
| runCatchingOnExecutor(callback) { | ||
| // IPSIE session_expiry: enforce the upstream-IdP session ceiling before serving cached | ||
| // API credentials or exchanging the refresh token, so the session is never extended past it. | ||
| if (isSessionExpired(storage.retrieveString(KEY_ID_TOKEN))) { | ||
| clearCredentials() | ||
| callback.onFailure(CredentialsManagerException.SESSION_EXPIRED) | ||
| return@execute | ||
| } | ||
| val key = getAPICredentialsKey(audience, scope) | ||
| val apiCredentialsJson = storage.retrieveString(key) | ||
| var apiCredentialType: String? = null | ||
|
|
@@ -697,9 +730,15 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| val expiresAt = storage.retrieveLong(KEY_EXPIRES_AT) | ||
| val emptyCredentials = | ||
| TextUtils.isEmpty(accessToken) && TextUtils.isEmpty(idToken) || expiresAt == null | ||
| return !(emptyCredentials || willExpire( | ||
| expiresAt!!, minTtl | ||
| ) && refreshToken == null) | ||
| if (emptyCredentials) { | ||
| return false | ||
| } | ||
| // IPSIE session_expiry: once the upstream-IdP ceiling passes, no valid credentials remain and | ||
| // a refresh cannot extend the session past it, so report no valid credentials. | ||
| if (isSessionExpired(idToken)) { | ||
| return false | ||
| } | ||
| return !(willExpire(expiresAt!!, minTtl) && refreshToken == null) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -714,6 +753,7 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting | |
| storage.remove(KEY_SCOPE) | ||
| storage.remove(LEGACY_KEY_CACHE_EXPIRES_AT) | ||
| storage.remove(KEY_DPOP_THUMBPRINT) | ||
| storage.remove(KEY_SESSION_EXPIRY) | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.