-
Notifications
You must be signed in to change notification settings - Fork 3
fix(auth): token refresh handling issues #39
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
Conversation
Reviewer's GuideRefines Rownd server state synchronization behavior for sign-in/sign-out vs token refresh and introduces an explicit, defaulted Hub API version that propagates from the Rownd provider into the Hub script injector config. Sequence diagram for RowndServerStateSync initial load behaviorsequenceDiagram
participant RowndServerStateSync
participant useRownd
participant cookieSignIn
RowndServerStateSync->>useRownd: read access_token, is_initializing
useRownd-->>RowndServerStateSync: access_token, is_initializing
alt is_initializing is true
RowndServerStateSync->>RowndServerStateSync: return early (no sync)
else is_initializing is false and hasInitialized is false
RowndServerStateSync->>RowndServerStateSync: set hasInitialized to true
RowndServerStateSync->>RowndServerStateSync: set prevAccessToken to access_token
alt access_token exists on initial load
RowndServerStateSync->>cookieSignIn: cookieSignIn()
else no access_token on initial load
RowndServerStateSync->>RowndServerStateSync: do nothing
end
end
Sequence diagram for RowndServerStateSync sign-in, refresh, and sign-outsequenceDiagram
participant RowndServerStateSync
participant useRownd
participant cookieSignIn
participant cookieSignOut
participant Window
RowndServerStateSync->>useRownd: read access_token, is_initializing
useRownd-->>RowndServerStateSync: access_token, is_initializing
RowndServerStateSync->>RowndServerStateSync: compute wasSignedOut, isSigningIn, isSigningOut, isTokenRefresh
RowndServerStateSync->>RowndServerStateSync: prevAccessToken = access_token
alt isSigningIn (null -> token)
RowndServerStateSync->>cookieSignIn: cookieSignIn(reloadCallback)
activate cookieSignIn
cookieSignIn-->>RowndServerStateSync: invokes reloadCallback
RowndServerStateSync->>Window: window.location.reload()
else isTokenRefresh (token -> different token)
RowndServerStateSync->>cookieSignIn: cookieSignIn()
else isSigningOut (token -> null)
RowndServerStateSync->>cookieSignOut: cookieSignOut(reloadCallback)
activate cookieSignOut
cookieSignOut-->>RowndServerStateSync: invokes reloadCallback
RowndServerStateSync->>Window: window.location.reload()
else no change (same token)
RowndServerStateSync->>RowndServerStateSync: no cookie actions
end
Class diagram for RowndProvider and HubScriptInjector API versioningclassDiagram
class RowndProviderProps {
string appKey
string~optional~ hubUrlOverride
string~optional~ postRegistrationUrl
string~optional~ postSignOutRedirect
string~optional~ apiVersion
ReactNode children
}
class HubScriptInjectorProps {
string appKey
function stateListener
string~optional~ hubUrlOverride
string~optional~ locationHash
string~optional~ apiVersion
}
class HubScriptInjector {
- string DEFAULT_API_VERSION
+ HubScriptInjector(props)
+ setConfigValue(key, value)
}
class WindowConfig {
any[] _rphConfig
}
RowndProviderProps <.. HubScriptInjectorProps : passes props
HubScriptInjectorProps --> HubScriptInjector : used_by
HubScriptInjector --> WindowConfig : pushes setApiVersion
HubScriptInjector : useEffect(appKey, stateListener, locationHash, hubUrlOverride, apiVersion, rest)
HubScriptInjector : setConfigValue(setAppKey, appKey)
HubScriptInjector : setConfigValue(setStateListener, stateListener)
HubScriptInjector : setConfigValue(setLocationHash, locationHash)
HubScriptInjector : setConfigValue(setApiVersion, apiVersion)
HubScriptInjector : injects hubUrlOverride or default URL
WindowConfig : _rphConfig.push([setApiVersion, apiVersion])
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
size-limit report 📦
|
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.
Hey - I've left some high level feedback:
- The
hasInitializedref inRowndServerStateSyncnever resets, so if the app key or auth context changes while the component remains mounted, the "initial load" branch will never run again; consider tying that initialization state to a stable key (e.g., app key or user/session id) so it can re-run when the underlying auth context changes. - The hard-coded
DEFAULT_API_VERSIONstring inHubScriptInjectormay drift from the actual SDK version over time; consider sourcing this from a single version/config constant (or package metadata) so future updates do not require manual date changes in multiple places.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `hasInitialized` ref in `RowndServerStateSync` never resets, so if the app key or auth context changes while the component remains mounted, the "initial load" branch will never run again; consider tying that initialization state to a stable key (e.g., app key or user/session id) so it can re-run when the underlying auth context changes.
- The hard-coded `DEFAULT_API_VERSION` string in `HubScriptInjector` may drift from the actual SDK version over time; consider sourcing this from a single version/config constant (or package metadata) so future updates do not require manual date changes in multiple places.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Refine authentication state synchronization between client and server cookies and introduce API versioning configuration for the Hub script injector.
Bug Fixes:
Enhancements:
Tests: