Keycloak - offline_access & auto login#5400
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5400 +/- ##
============================================
+ Coverage 67.04% 67.52% +0.47%
- Complexity 0 46 +46
============================================
Files 381 481 +100
Lines 22619 25053 +2434
Branches 3095 3481 +386
============================================
+ Hits 15166 16918 +1752
- Misses 6291 6881 +590
- Partials 1162 1254 +92
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
There was a problem hiding this comment.
Maybe we can rename this to ContinueAsGuestButton for consistency.
| } | ||
| } | ||
|
|
||
| private func refreshExistingAuthState( |
There was a problem hiding this comment.
Some duplication with refreshAuthState above, do we need both or can we deduplicate some code?
| setupAutomaticRefresh(); | ||
| } | ||
|
|
||
| private AuthState restoreAuthState() { |
There was a problem hiding this comment.
All of this runs on the Android main thread (Capacitor calls KeycloakPlugin.load() → new Keycloak(context) → restoreAuthState() → authStateStore.read() → DataStore.data().firstOrError().blockingGet(). ). On cold start with slow flash I/O, this could exceed Android's 5-second ANR threshold and force-closes the app. write() (called from persistAuthState() inside performActionWithFreshTokens callbacks) and clear() carry the same risk if AppAuth delivers those callbacks on the main thread.
We can use a shared IO executor thread pool to keep this off the main thread like: task/keycloak-asago-tweaks...task/executor-non-blocking-fix
| expiresIn?: number | ||
| scope?: string | ||
| }) => { | ||
| const handleTokenRefresh = (tokenResponse: KeycloakTokenResponse) => { |
There was a problem hiding this comment.
Less likely to happen now but cheap to fix:
Every time authenticate() is dispatched a second handleTokenRefresh closure is registered. After N login sessions, N closures fire per native tokenRefresh event: N dispatches of authenticateFinished, N Sentry.setUser calls. The PluginListenerHandle returned by addListener is discarded.
We can have a module level handle to register the tokenRefresh handle once to avoid, like: task/keycloak-asago-tweaks...task/single-login-listener
| self.notifyListeners("tokenRefresh", data: tokenResponse) | ||
| } | ||
| } else { | ||
| self.notifyListeners( |
There was a problem hiding this comment.
We should clearStoredAuthState() here to avoid looping retries, see: task/keycloak-asago-tweaks...task/failed-foreground-fix-swift
2 participants
-Android was not requesting offline_access, so its refresh token payload had
"typ": "Refresh"instead of the expected"typ": "Offline". iOS was receiving offline tokens, but itsAppAuthstate was not persisted somewhere the app could reuse after restart. Android also persisted AppAuth state but did not check the stored state before starting a new browser login flow.Login was relying mostly on browser session cookies stored on the device. On iOS, clearing recent Safari cache/cookies could cause ASA Go to prompt for login again
This PR
offline_accessscope to Android authentication requests.OIDAuthStatein Keychain.Test Links:
Landing Page
MoreCast
Percentile Calculator
C-Haines
FireCalc
FireCalc bookmark
Auto Spatial Advisory (ASA)
HFI Calculator
SFMS Insights
Fire Watch
Weather Toolkit