-
Notifications
You must be signed in to change notification settings - Fork 10
Add Session Minter support to the token fetch path #835
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,6 +31,7 @@ import com.clerk.api.network.serialization.onFailure | |||||||||||||
| import com.clerk.api.passkeys.PasskeyService | ||||||||||||||
| import com.clerk.api.session.GetTokenOptions | ||||||||||||||
| import com.clerk.api.session.Session | ||||||||||||||
| import com.clerk.api.session.SessionTokensCache | ||||||||||||||
| import com.clerk.api.session.fetchToken | ||||||||||||||
| import com.clerk.api.session.revoke | ||||||||||||||
| import com.clerk.api.signin.SignIn | ||||||||||||||
|
|
@@ -639,6 +640,7 @@ class Auth internal constructor() { | |||||||||||||
| if (sessionId != null) { | ||||||||||||||
| when (val result = ClerkApi.session.removeSession(sessionId)) { | ||||||||||||||
| is ClerkResult.Success -> { | ||||||||||||||
| SessionTokensCache.removeTokensForSession(sessionId) | ||||||||||||||
| removeSessionLocally(sessionId) | ||||||||||||||
| refreshClientAfterSessionMutation() | ||||||||||||||
|
Comment on lines
+643
to
645
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. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Clear the removed session’s persisted token during the refresh.
Proposed fix- refreshClientAfterSessionMutation()
+ refreshClientAfterSessionMutation(droppedTokenSessionId = sessionId)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| ClerkResult.success(Unit) | ||||||||||||||
|
|
@@ -673,20 +675,38 @@ class Auth internal constructor() { | |||||||||||||
| private suspend fun refreshClientAfterSessionMutation( | ||||||||||||||
| activeSessionFallbackId: String? = null, | ||||||||||||||
| fallbackClient: Client? = null, | ||||||||||||||
| droppedTokenSessionId: String? = null, | ||||||||||||||
| ) { | ||||||||||||||
| when (val clientResult = Client.get()) { | ||||||||||||||
| is ClerkResult.Success -> | ||||||||||||||
| Clerk.updateClient( | ||||||||||||||
| clientResult.value.withActiveSessionFallback( | ||||||||||||||
| activeSessionFallbackId = activeSessionFallbackId, | ||||||||||||||
| fallbackClient = fallbackClient, | ||||||||||||||
| ) | ||||||||||||||
| clientResult.value | ||||||||||||||
| .withActiveSessionFallback( | ||||||||||||||
| activeSessionFallbackId = activeSessionFallbackId, | ||||||||||||||
| fallbackClient = fallbackClient, | ||||||||||||||
| ) | ||||||||||||||
| .withoutLastActiveToken(droppedTokenSessionId) | ||||||||||||||
| ) | ||||||||||||||
| is ClerkResult.Failure -> | ||||||||||||||
| ClerkLog.w("Client refresh after session mutation failed: ${clientResult.errorMessage}") | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Drops the persisted token of one session. | ||||||||||||||
| * | ||||||||||||||
| * Both merge paths above carry `lastActiveToken` forward from the pre-mutation session, and the | ||||||||||||||
| * token fetcher falls back to it when the cache is empty. Left in place after an org switch it | ||||||||||||||
| * would seed the next mint with the old organization scope, undoing the cache invalidation. | ||||||||||||||
| */ | ||||||||||||||
| private fun Client.withoutLastActiveToken(sessionId: String?): Client { | ||||||||||||||
| if (sessionId == null) return this | ||||||||||||||
|
|
||||||||||||||
| val updatedSessions = | ||||||||||||||
| sessions.map { if (it.id == sessionId) it.copy(lastActiveToken = null) else it } | ||||||||||||||
| return if (updatedSessions == sessions) this else copy(sessions = updatedSessions) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private fun Client.withActiveSessionFallback( | ||||||||||||||
| activeSessionFallbackId: String?, | ||||||||||||||
| fallbackClient: Client?, | ||||||||||||||
|
|
@@ -773,6 +793,9 @@ class Auth internal constructor() { | |||||||||||||
| ) | ||||||||||||||
| when (result) { | ||||||||||||||
| is ClerkResult.Success -> { | ||||||||||||||
| // The cached token still carries the previous organization scope, and it seeds the next | ||||||||||||||
| // mint, so a stale entry would outlive the switch. | ||||||||||||||
| SessionTokensCache.removeTokensForSession(sessionId) | ||||||||||||||
| setActiveSessionLocally( | ||||||||||||||
| sessionId, | ||||||||||||||
| activeSession = result.value, | ||||||||||||||
|
|
@@ -782,6 +805,7 @@ class Auth internal constructor() { | |||||||||||||
| refreshClientAfterSessionMutation( | ||||||||||||||
| activeSessionFallbackId = sessionId, | ||||||||||||||
| fallbackClient = if (Clerk.clientInitialized) Clerk.client else previousClient, | ||||||||||||||
| droppedTokenSessionId = sessionId, | ||||||||||||||
| ) | ||||||||||||||
| } | ||||||||||||||
| is ClerkResult.Failure -> emitAuthError(result) | ||||||||||||||
|
|
@@ -799,7 +823,11 @@ class Auth internal constructor() { | |||||||||||||
| val sessions = client.sessions.withUpdatedSession(activeSession, activeOrganizationId) | ||||||||||||||
| if (sessions.none { it.id == sessionId }) return | ||||||||||||||
|
|
||||||||||||||
| Clerk.updateClient(client.copy(sessions = sessions, lastActiveSessionId = sessionId)) | ||||||||||||||
| Clerk.updateClient( | ||||||||||||||
| client | ||||||||||||||
| .copy(sessions = sessions, lastActiveSessionId = sessionId) | ||||||||||||||
| .withoutLastActiveToken(sessionId) | ||||||||||||||
| ) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private fun List<Session>.withUpdatedSession( | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.clerk.api.session | ||
|
|
||
| import com.auth0.android.jwt.JWT | ||
| import java.util.concurrent.TimeUnit | ||
|
|
||
| /** | ||
| * Orders two session JWTs by how recently their claims were assembled. | ||
| * | ||
| * Minted tokens carry `oiat` (original issued at) in the protected header. It survives re-minting, | ||
| * so it is the primary ordering claim; the payload's `iat` only breaks a tie between two tokens | ||
| * assembled at the same instant. A token without `oiat` comes from a pre-minter code path and is | ||
| * staler than any token that has one. | ||
| * | ||
| * This mirrors clerk-js `pickFreshestJwt`, so web and Android agree on which of two tokens wins. | ||
| */ | ||
| internal object JwtFreshness { | ||
|
|
||
| private const val ORIGINAL_ISSUED_AT_HEADER = "oiat" | ||
|
|
||
| /** Whether the JWT can be decoded at all. An undecodable JWT is never sent or ranked. */ | ||
| internal fun isReadable(jwt: String): Boolean = claimsOf(jwt) != null | ||
|
|
||
| /** | ||
| * Whether [incoming] should replace [existing]. | ||
| * | ||
| * A full tie accepts [incoming]: two tokens assembled at the same instant can still differ in | ||
| * other claims, so only a strictly fresher [existing] suppresses the write. An unreadable | ||
| * [incoming] never wins over a readable [existing]. | ||
| */ | ||
| internal fun incomingWins(existing: String, incoming: String): Boolean { | ||
| val existingClaims = claimsOf(existing) | ||
| val incomingClaims = claimsOf(incoming) | ||
| return when { | ||
| existingClaims == null -> true | ||
| incomingClaims == null -> false | ||
| else -> incomingIsAtLeastAsFresh(existing = existingClaims, incoming = incomingClaims) | ||
| } | ||
| } | ||
|
|
||
| private fun incomingIsAtLeastAsFresh(existing: Claims, incoming: Claims): Boolean { | ||
| val existingOriginalIssuedAt = existing.originalIssuedAt | ||
| val incomingOriginalIssuedAt = incoming.originalIssuedAt | ||
| return when { | ||
| existingOriginalIssuedAt == null && incomingOriginalIssuedAt == null -> true | ||
| incomingOriginalIssuedAt == null -> false | ||
| existingOriginalIssuedAt == null -> true | ||
| existingOriginalIssuedAt != incomingOriginalIssuedAt -> | ||
| existingOriginalIssuedAt < incomingOriginalIssuedAt | ||
| else -> (existing.issuedAt ?: 0L) <= (incoming.issuedAt ?: 0L) | ||
| } | ||
| } | ||
|
|
||
| private fun claimsOf(jwt: String): Claims? = | ||
| runCatching { | ||
| val decoded = JWT(jwt) | ||
| Claims( | ||
| originalIssuedAt = decoded.header[ORIGINAL_ISSUED_AT_HEADER]?.toLongOrNull(), | ||
| issuedAt = decoded.issuedAt?.let { TimeUnit.MILLISECONDS.toSeconds(it.time) }, | ||
| ) | ||
| } | ||
| .getOrNull() | ||
|
|
||
| private data class Claims(val originalIssuedAt: Long?, val issuedAt: Long?) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| package com.clerk.api.session | ||
|
|
||
| import com.clerk.api.Constants.Config.TOKEN_FETCH_BACKOFF_BASE_MS | ||
| import com.clerk.api.Constants.Config.TOKEN_FETCH_BACKOFF_MAX_MS | ||
| import java.util.concurrent.ConcurrentHashMap | ||
|
|
||
| /** | ||
| * Per-cache-key failure backoff for session token requests. | ||
| * | ||
| * The SDK wakes every few seconds to refresh the active session token. Without a backoff a failing | ||
| * endpoint is re-POSTed on every wake for as long as the app stays in the foreground, so each | ||
| * consecutive failure doubles the window during which the fetch layer refuses to send another | ||
| * request. | ||
| * | ||
| * The failure count outlives the window it opened: an expired window means another request may go | ||
| * out, not that the endpoint recovered. Only a success or a deliberate state change (sign-out, org | ||
| * switch, SDK reset) resets the schedule. Dropping the count at the deadline instead would restart | ||
| * every key at five seconds and leave the wake loop hammering forever. | ||
| * | ||
| * @param clock Source of the current time in milliseconds. Injectable for tests. | ||
| */ | ||
| internal class SessionTokenBackoff(private val clock: () -> Long = System::currentTimeMillis) { | ||
|
|
||
| private val windows = ConcurrentHashMap<String, Window>() | ||
|
|
||
| /** Whether a request for [cacheKey] is still inside the window opened by its last failure. */ | ||
| internal fun isBackingOff(cacheKey: String): Boolean { | ||
| val window = windows[cacheKey] ?: return false | ||
| return clock() < window.retryAtMillis | ||
| } | ||
|
|
||
| /** Opens the next, wider window for [cacheKey]. */ | ||
| internal fun recordFailure(cacheKey: String) { | ||
| windows.compute(cacheKey) { _, existing -> | ||
| val failures = (existing?.failures ?: 0) + 1 | ||
| Window(failures = failures, retryAtMillis = clock() + delayMillis(failures)) | ||
| } | ||
| } | ||
|
|
||
| /** Closes the window for [cacheKey] and resets its schedule. */ | ||
| internal fun recordSuccess(cacheKey: String) { | ||
| windows.remove(cacheKey) | ||
| } | ||
|
|
||
| /** Resets the schedule for a session's default and templated keys. */ | ||
| internal fun clearForSession(sessionId: String) { | ||
| windows.keys.removeAll { it == sessionId || it.startsWith("$sessionId-") } | ||
| } | ||
|
|
||
| /** Resets every schedule. */ | ||
| internal fun clear() { | ||
| windows.clear() | ||
| } | ||
|
|
||
| private fun delayMillis(failures: Int): Long { | ||
| val exponent = (failures - 1).coerceAtMost(EXPONENT_CEILING) | ||
| return (TOKEN_FETCH_BACKOFF_BASE_MS shl exponent).coerceAtMost(TOKEN_FETCH_BACKOFF_MAX_MS) | ||
| } | ||
|
|
||
| private data class Window(val failures: Int, val retryAtMillis: Long) | ||
|
|
||
| internal companion object { | ||
| /** Keeps the shift below Long overflow; the delay is capped well before this bites. */ | ||
| private const val EXPONENT_CEILING = 16 | ||
|
|
||
| /** The schedule the SDK uses. Tests build their own with a fake clock. */ | ||
| internal val shared = SessionTokenBackoff() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 High
clerk-android/source/api/src/main/kotlin/com/clerk/api/Clerk.kt
Line 763 in b863b20
A stale token from the old configuration can survive
reset()and remain usable after re-initialization.SessionTokenFetcher.resetForNewConfiguration()bumps the configuration epoch only afterSessionTokensCache.clear()andClerkApi.reset()have already run, so a concurrentgetTokenadmitted after the cache clears but before the epoch bump captures the new cache generation with the old epoch. If that request reaches the old Retrofit service and completes before the bump,setTokenIfCurrentaccepts its token (the generation still matches), and the later epoch bump never clears it. The stale token then persists into the next configuration. Move theresetForNewConfiguration()call beforeSessionTokensCache.clear()andClerkApi.reset(), so any request that could still reach the old service captured the old epoch and is discarded.🚀 Reply "fix it for me" or copy this AI Prompt for your agent: