Skip to content

CBG-5311: [4.0.5 backport] Cleanup for GetUser#8261

Merged
gregns1 merged 1 commit into
release/4.0.5from
CBG-5311
May 12, 2026
Merged

CBG-5311: [4.0.5 backport] Cleanup for GetUser#8261
gregns1 merged 1 commit into
release/4.0.5from
CBG-5311

Conversation

@gregns1

@gregns1 gregns1 commented May 12, 2026

Copy link
Copy Markdown
Contributor

CBG-5311

Backports #8259

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

  • Link upstream PRs
  • Update Go module dependencies when merged

Integration Tests

Copilot AI review requested due to automatic review settings May 12, 2026 14:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Backport of upstream “Cleanup for GetUser” changes to make auth/session and REST auth flows safer when GetUser/GetRole can return (nil, nil), and to normalize concurrent registration behavior.

Changes:

  • Hardened multiple call sites against GetUser/GetRole returning nil (avoid nil dereferences; return appropriate auth errors).
  • Updated RegisterNewUser to treat CAS-race “user already created” as success (return existing user with nil error) and simplified callers accordingly.
  • Improved error handling/logging paths where user data was previously referenced via potentially-nil principals.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
rest/session_api.go Simplifies registration error handling to align with updated RegisterNewUser CAS-race semantics.
rest/handler.go Ensures GetUser returning nil after principal updates results in ErrInvalidLogin instead of leaving h.user nil.
rest/diagnostic_api.go Avoids dereferencing user.Name() when GetUser returns an error by using the requested username string.
db/database.go Skips missing roles/users during sequence regeneration to avoid nil principal usage.
db/crud.go Handles GetUser returning nil when reloading the active user, with a warning.
auth/session.go Treats missing user as invalid session (prevents nil deref on user.GetSessionUUID()).
auth/auth.go casUpdatePrincipal returns not-found when a principal disappears during CAS retries; RegisterNewUser returns existing user with nil error on CAS-race.

Comment thread auth/auth.go
Comment on lines +974 to +977
if existing == nil {
return nil, err
}
return existing, nil
Comment thread auth/session.go
Comment on lines +75 to 77
if user == nil || session.SessionUUID != user.GetSessionUUID() {
base.InfofCtx(auth.LogCtx, base.KeyAuth, "Session no longer valid for user %s", base.UD(session.Username))
return nil, base.HTTPErrorf(http.StatusUnauthorized, "Session no longer valid for user")
@gregns1
gregns1 merged commit 57f7e12 into release/4.0.5 May 12, 2026
46 checks passed
@gregns1
gregns1 deleted the CBG-5311 branch May 12, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants