-
Notifications
You must be signed in to change notification settings - Fork 10
Align Android session token fetching with edge minter behavior #827
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 | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,8 +26,10 @@ import kotlinx.coroutines.Deferred | |||||||
| * @param jwtManager The JWT manager used for token parsing and validation | ||||||||
| */ | ||||||||
| internal class SessionTokenFetcher(private val jwtManager: JWTManager = JWTManagerImpl()) { | ||||||||
| private companion object { | ||||||||
| val sessionInvalidationErrorCodes = | ||||||||
| internal companion object { | ||||||||
| internal val shared: SessionTokenFetcher by lazy { SessionTokenFetcher() } | ||||||||
|
|
||||||||
| private val sessionInvalidationErrorCodes = | ||||||||
| setOf( | ||||||||
| "session_revoked", | ||||||||
| "session_expired", | ||||||||
|
|
@@ -40,9 +42,23 @@ internal class SessionTokenFetcher(private val jwtManager: JWTManager = JWTManag | |||||||
| ) | ||||||||
| } | ||||||||
|
|
||||||||
| private data class FetchContext( | ||||||||
| val session: Session, | ||||||||
| val cacheKey: String, | ||||||||
| val sessionMinterEnabled: Boolean, | ||||||||
| ) | ||||||||
|
|
||||||||
| /** Map of cache keys to deferred token fetch tasks for request deduplication */ | ||||||||
| private val tokenTasks = ConcurrentHashMap<String, Deferred<TokenResource?>>() | ||||||||
|
|
||||||||
| /** | ||||||||
| * Releases deduplicated waiters and removes requests registered by the previous Clerk runtime. | ||||||||
| */ | ||||||||
| internal fun reset() { | ||||||||
| tokenTasks.values.forEach { it.cancel() } | ||||||||
| tokenTasks.clear() | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * Retrieves a token for the specified session with the given options. | ||||||||
| * | ||||||||
|
|
@@ -76,31 +92,46 @@ internal class SessionTokenFetcher(private val jwtManager: JWTManager = JWTManag | |||||||
| session: Session, | ||||||||
| options: GetTokenOptions, | ||||||||
| ): TokenResource? { | ||||||||
| val cacheKey = session.tokenCacheKey(options.template) | ||||||||
| val context = makeFetchContext(session, options.template) | ||||||||
| ClerkLog.d( | ||||||||
| "Fetching token for session ${session.id} with options: $options and cache key: $cacheKey" | ||||||||
| "Fetching token for session ${context.session.id} with options: $options and cache key: " + | ||||||||
| context.cacheKey | ||||||||
| ) | ||||||||
|
|
||||||||
| return tokenTasks[cacheKey]?.await() | ||||||||
| if (options.skipCache) { | ||||||||
| return fetchToken(context, options) | ||||||||
| } | ||||||||
|
|
||||||||
| return tokenTasks[context.cacheKey]?.await() | ||||||||
| ?: run { | ||||||||
| val deferred = CompletableDeferred<TokenResource?>() | ||||||||
| val existingTask = tokenTasks.putIfAbsent(cacheKey, deferred) | ||||||||
| val existingTask = tokenTasks.putIfAbsent(context.cacheKey, deferred) | ||||||||
|
|
||||||||
| existingTask?.await() | ||||||||
| ?: try { | ||||||||
| fetchToken(session, options).also { deferred.complete(it) } | ||||||||
| fetchToken(context, options).also { deferred.complete(it) } | ||||||||
| } catch (e: CancellationException) { | ||||||||
| deferred.cancel(e) | ||||||||
| throw e | ||||||||
| } catch (t: Throwable) { | ||||||||
| deferred.completeExceptionally(t) | ||||||||
| throw t | ||||||||
| } finally { | ||||||||
| tokenTasks.remove(cacheKey, deferred) | ||||||||
| tokenTasks.remove(context.cacheKey, deferred) | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| private fun makeFetchContext(session: Session, template: String?): FetchContext { | ||||||||
| val currentSession = | ||||||||
| Clerk.clientFlow.value?.sessions?.firstOrNull { it.id == session.id } ?: session | ||||||||
| return FetchContext( | ||||||||
| session = currentSession, | ||||||||
| cacheKey = currentSession.tokenCacheKey(template), | ||||||||
| sessionMinterEnabled = Clerk.environment?.authConfig?.sessionMinter == true, | ||||||||
| ) | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * Internal method to fetch a token from cache or network. | ||||||||
| * | ||||||||
|
|
@@ -112,8 +143,17 @@ internal class SessionTokenFetcher(private val jwtManager: JWTManager = JWTManag | |||||||
| * @param options Options controlling the fetch behavior | ||||||||
| * @return The token resource, or null if the fetch failed | ||||||||
| */ | ||||||||
| private suspend fun fetchToken(session: Session, options: GetTokenOptions): TokenResource? { | ||||||||
| val cacheKey = session.tokenCacheKey(options.template) | ||||||||
| private suspend fun fetchToken(context: FetchContext, options: GetTokenOptions): TokenResource? { | ||||||||
| val session = context.session | ||||||||
| val cacheKey = context.cacheKey | ||||||||
|
|
||||||||
| if (options.template == null) { | ||||||||
| session.lastActiveToken | ||||||||
| ?.takeIf { | ||||||||
| TokenFreshness.matches(it, session.id, session.lastActiveOrganizationId) | ||||||||
| } | ||||||||
| ?.let { SessionTokensCache.hydrate(cacheKey, it) } | ||||||||
| } | ||||||||
|
|
||||||||
| // Check cache first (unless skipped) | ||||||||
| if (!options.skipCache) { | ||||||||
|
|
@@ -134,12 +174,25 @@ internal class SessionTokenFetcher(private val jwtManager: JWTManager = JWTManag | |||||||
| if (options.template != null) { | ||||||||
| ClerkApi.session.tokens(session.id, options.template) | ||||||||
| } else { | ||||||||
| ClerkApi.session.tokens(session.id) | ||||||||
| val cachedToken = SessionTokensCache.getToken(cacheKey) | ||||||||
| val previousToken = | ||||||||
| cachedToken?.let { | ||||||||
| TokenFreshness.pickFreshest( | ||||||||
| existing = session.lastActiveToken, | ||||||||
| incoming = it, | ||||||||
| ) | ||||||||
| } ?: session.lastActiveToken | ||||||||
| ClerkApi.session.tokens( | ||||||||
| sessionId = session.id, | ||||||||
| organizationId = session.lastActiveOrganizationId.orEmpty(), | ||||||||
| token = previousToken?.jwt.takeIf { context.sessionMinterEnabled }, | ||||||||
| forceOrigin = "true".takeIf { context.sessionMinterEnabled && options.skipCache }, | ||||||||
| ) | ||||||||
| } | ||||||||
|
|
||||||||
| when (tokensRequest) { | ||||||||
| is ClerkResult.Success -> { | ||||||||
| SessionTokensCache.setToken(cacheKey, tokensRequest.value) | ||||||||
| SessionTokensCache.storeIfFresher(cacheKey, tokensRequest.value) | ||||||||
| tokensRequest.value | ||||||||
|
Comment on lines
+195
to
196
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. 🟠 High
Suggested change
🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||||||||
| } | ||||||||
| is ClerkResult.Failure -> { | ||||||||
|
|
@@ -228,10 +281,11 @@ data class GetTokenOptions( | |||||||
| /** | ||||||||
| * Extension function to generate a cache key for session tokens. | ||||||||
| * | ||||||||
| * This function creates a unique cache key based on the session ID and optional template name. This | ||||||||
| * ensures that tokens for different templates are cached separately. | ||||||||
| * This function creates a unique cache key based on the session ID and either its active | ||||||||
| * organization or the optional template name. | ||||||||
| * | ||||||||
| * @param template Optional template name to include in the cache key | ||||||||
| * @return A unique cache key string for the session and template combination | ||||||||
| */ | ||||||||
| internal fun Session.tokenCacheKey(template: String?): String = template?.let { "$id-$it" } ?: id | ||||||||
| internal fun Session.tokenCacheKey(template: String?): String = | ||||||||
| template?.let { "$id-template-$it" } ?: "$id-organization-${lastActiveOrganizationId.orEmpty()}" | ||||||||
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
session/SessionTokenFetcher.kt:57reset()cancels the waiterDeferreds but not the in-flightfetchTokencoroutine, so an ongoing token request can keep running after reset, write its JWT into the sharedSessionTokensCache, and return a token from the old configuration to the caller. This stale credential can contaminate the new runtime. Consider cancelling or fencing the actual fetch work so an in-flight request cannot store results after reset.🚀 Reply "fix it for me" or copy this AI Prompt for your agent: