Add support to configure token caching in BearerAuthProvider - #5111
ruben-caceiro-digiwest wants to merge 2 commits into
Conversation
WalkthroughAdds an optional caching flag to token handling. AuthTokenHolder gets a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{kt,kts}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.kt📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BearerAuthProvider.kt (1)
105-111: Document the new constructor parameter.The
BearerAuthProvideris a public class, and per coding guidelines, public APIs should be fully documented. The newcacheparameter should be documented in the class KDoc.Apply this diff to add parameter documentation:
/** * An authentication provider for the Bearer HTTP authentication scheme. * Bearer authentication involves security tokens called bearer tokens. * As an example, these tokens can be used as a part of OAuth flow to authorize users of your application * by using external providers, such as Google, Facebook, Twitter, and so on. * * You can learn more from [Bearer authentication](https://ktor.io/docs/bearer-client.html). * + * @param refreshTokens A callback to refresh tokens when a 401 response is received. + * @param cache Whether to cache loaded and refreshed tokens. When `false`, tokens are loaded fresh on every request. + * @param loadTokens A callback to load tokens from local storage. + * @param sendWithoutRequestCallback A predicate to determine if credentials should be sent preemptively. + * @param realm The authentication realm to match, or null to match any realm. + * * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.client.plugins.auth.providers.BearerAuthProvider) */ public class BearerAuthProvider(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/AuthTokenHolder.kt(2 hunks)ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BearerAuthProvider.kt(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BearerAuthProvider.ktktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/AuthTokenHolder.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BearerAuthProvider.ktktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/AuthTokenHolder.kt
🔇 Additional comments (6)
ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/AuthTokenHolder.kt (3)
14-17: LGTM! Clean parameter addition.The new
cacheparameter is well-positioned and follows Kotlin conventions. Making itprivate valensures proper encapsulation.
18-22: LGTM! Improved volatile field formatting.Moving
@Volatileto its own line improves readability and aligns with common Kotlin practices for visibility of important annotations.
36-36: Thread-safety behavior remains unchanged
Whencache=false,loadTokenalways acquires themutex(bypassing the hot-path) and reloads under lock, so there’s no unsynchronized TOCTOU window. Whencache=true, the optimisticif (cache && value != null)read is identical to the original implementation (using a@Volatilefield), so this same race window still exists by design for performance. No changes required.ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BearerAuthProvider.kt (3)
22-22: LGTM! Correct integration.The
bearerextension properly passes the newcacheparameter from config to provider.
107-107: LGTM! Constructor parameter added with sensible default.The
cacheparameter is well-positioned (afterrefreshTokens, beforeloadTokens) and has a sensible default value oftrueto maintain backward compatibility. SinceBearerAuthProvideris a public class, users may instantiate it directly, and the default ensures existing code continues to work.
118-118: LGTM! Correct initialization.The
tokensHolderinitialization properly propagates thecacheparameter toAuthTokenHolder.
…/ktor/client/plugins/auth/providers/BearerAuthProvider.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Hey ruben-caceiro-digiwest, sorry for the delay. The issue is fixed in |
2 participants
Subsystem
Client, related module
Motivation
Allow non cache Bearer token
Solution
I simply add a cache flag when the flag is false it ignores the cached token and get always a new one.