fix(auth): skip Nais login when already authenticated - #782
Conversation
📝 Changelog previewBelow is a preview of the Changelog that will be added to the next release. Only commit messages that follow the Conventional Commits specification will be included in the Changelog. v5.50.2 - 2026-09-30Full Changelog: v5.50.1...v5.50.2 🐛 Bug Fixes
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Authentication-expiry errors from access-token retrieval should trigger login instead of being returned.
Review effort: Balanced
Findings: 1
What changed in this PR
Skips the Nais OIDC login flow when a valid session already exists.
Changes:
- Validates the stored user and access token.
- Reports the authenticated email when login is skipped.
| File | Description |
|---|---|
internal/naisapi/auth/oidc.go |
Adds existing-session detection to OIDC login. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a975aed to
d990c0e
Compare
d990c0e to
da47267
Compare
| if err == nil { | ||
| // The access token may be expired, in which case it is refreshed here. | ||
| // A failed refresh means the session is gone and we must log in again. | ||
| _, err = user.AccessToken() | ||
| } |
There was a problem hiding this comment.
I'm nitpicking a little here, but this is already covered through OIDC() -> getOIDCUser() -> user.Valid() which attempts a refresh if the token is expired or within 10 seconds of expiry. A failed refresh also returns ErrNeedsOIDCLogin.
I also think the check to skip login should live in naisapi.Login().
OIDCLogin() should instead always run the login flow, e.g. in case we want to force a new login or switch accounts.
There was a problem hiding this comment.
Agree, fixing in new PR (#783). Nitpicking is the way tbh
No description provided.