-
Notifications
You must be signed in to change notification settings - Fork 53
Description
Background
XBuilder currently depends on Casdoor for authentication. During local debugging, I found a reproducible bug in the Casdoor OAuth refresh-token flow that can break session refresh after a user renames their own username.
This is not a builder-only logic bug, but it affects the authentication path that builder relies on, so it is worth tracking here.
Problem
Refreshing an OAuth access token can panic after the user changes their username.
The crash happens on the Casdoor endpoint used for token refresh:
POST /api/login/oauth/access_tokengrant_type=refresh_token
Reproduction
-
Sign in normally and obtain a refresh token.
-
Call the user update API to change the current user's username.
-
Call
POST /api/login/oauth/access_tokenwithgrant_type=refresh_token. -
The server crashes with a nil pointer dereference.
What I found
The refresh flow eventually reaches code equivalent to:
user, err := getUser(application.Organization, token.User)
if err != nil {
return nil, err
}
if user.IsForbidden {
...
}The key problem is:
- OAuth token records store
token.User = user.Namewhen the token is issued. - After the username is changed, the existing token record still keeps the old username.
- The refresh flow then looks up the user by that stale username.
getUser(...)returnsnil, nilwhen the user is not found.- The next line dereferences
user.IsForbidden, which causes the panic.
Why this happens reliably after rename
Casdoor already has rename-trigger logic to update some dependent records when a username changes, such as:
- role users
- permission users
- resource user
But token records are not updated there, so refresh tokens issued before the rename continue to reference the old username.
Likely root cause
Two problems stack together:
- Token data stores a mutable username as the refresh-time lookup key.
- The refresh flow assumes the user lookup result is always non-nil and does not guard against a missing user.
Suggested fix
- Add a nil check in the refresh-token flow and return a proper OAuth error instead of panicking.
- Fix the underlying consistency issue so refresh-token lookup does not depend on a stale mutable username.
Possible implementation directions:
- Update token records when username changes.
- Or preferably stop using mutable username as the refresh lookup key and use a stable user identifier.
Plan
- Confirm whether builder can trigger this path in its deployed auth flow as-is.
- Patch Casdoor-side logic to avoid panic.
- Decide whether the long-term fix should be data migration/update-on-rename or a stable-ID-based lookup.