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

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>

Author: gregns1

auth/auth.go

@@ -719,6 +719,9 @@ func (auth *Authenticator) casUpdatePrincipal(p Principal, callback casUpdatePri
 		if err != nil {
 			return fmt.Errorf("Error reloading principal after CAS failure: %w", err)
 		}
+		if p == nil {
+			return base.ErrNotFound
+		}
 	}
 	base.InfofCtx(auth.LogCtx, base.KeyAuth, "Unable to update principal after %d attempts.  Principal:%s Error:%v", PrincipalUpdateMaxCasRetries, base.UD(p.Name()), err)
 	return err
@@ -929,7 +932,7 @@ func (auth *Authenticator) authenticateJWTIdentity(identity *Identity, provider
 	if common.Register {
 		base.DebugfCtx(auth.LogCtx, base.KeyAuth, "Registering new user: %v with email: %v", base.UD(username), base.UD(identity.Email))
 		user, err := auth.RegisterNewUser(username, identity.Email)
-		if err != nil && !base.IsCasMismatch(err) {
+		if err != nil {
 			base.InfofCtx(auth.LogCtx, base.KeyAuth, "Error registering new user: %v", err)
 			return nil, PrincipalConfig{}, time.Time{}, err
 		}
@@ -941,8 +944,8 @@ func (auth *Authenticator) authenticateJWTIdentity(identity *Identity, provider
 }
 
 // Registers a new user account based on the given verified username and optional email address.
-// Password will be random. The user will have access to no channels.  If the user already exists,
-// returns the existing user along with the cas failure error
+// Password will be random. The user will have access to no channels.  If the user already exists
+// (race with a concurrent registration), returns the existing user with a nil error.
 func (auth *Authenticator) RegisterNewUser(username, email string) (User, error) {
 	secret, err := base.GenerateRandomSecret()
 	if err != nil {
@@ -964,7 +967,14 @@ func (auth *Authenticator) RegisterNewUser(username, email string) (User, error)
 
 	err = auth.Save(user)
 	if base.IsCasMismatch(err) {
-		return auth.GetUser(username)
+		existing, getErr := auth.GetUser(username)
+		if getErr != nil {
+			return nil, getErr
+		}
+		if existing == nil {
+			return nil, err
+		}
+		return existing, nil
 	} else if err != nil {
 		return nil, err
 	}

auth/session.go

@@ -72,8 +72,7 @@ func (auth *Authenticator) AuthenticateCookie(rq *http.Request, response http.Re
 	if err != nil {
 		return nil, err
 	}
-
-	if session.SessionUUID != user.GetSessionUUID() {
+	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")
 	}

db/crud.go

@@ -3179,6 +3179,8 @@ func (db *DatabaseCollectionWithUser) MarkPrincipalsChanged(ctx context.Context,
 		user, err := db.Authenticator(ctx).GetUser(db.user.Name())
 		if err != nil {
 			base.WarnfCtx(ctx, "Error reloading active db.user[%s], security information will not be recalculated until next authentication --> %+v", base.UD(db.user.Name()), err)
+		} else if user == nil {
+			base.WarnfCtx(ctx, "Active db.user[%s] no longer exists; security information will not be recalculated until next authentication", base.UD(db.user.Name()))
 		} else {
 			db.user = user
 		}

db/database.go

@@ -1717,22 +1717,28 @@ func (c *DatabaseCollection) updateAllPrincipalsSequences(ctx context.Context) e
 
 	authr := c.Authenticator(ctx)
 
-	for _, role := range roles {
-		role, err := authr.GetRole(role)
+	for _, roleName := range roles {
+		role, err := authr.GetRole(roleName)
 		if err != nil {
 			return err
 		}
+		if role == nil {
+			continue
+		}
 		err = c.regeneratePrincipalSequences(ctx, authr, role)
 		if err != nil {
 			return err
 		}
 	}
 
-	for _, user := range users {
-		user, err := authr.GetUser(user)
+	for _, userName := range users {
+		user, err := authr.GetUser(userName)
 		if err != nil {
 			return err
 		}
+		if user == nil {
+			continue
+		}
 		err = c.regeneratePrincipalSequences(ctx, authr, user)
 		if err != nil {
 			return err

rest/diagnostic_api.go

@@ -197,9 +197,10 @@ func getHistoricRoleChanEntryOverlap(roleEntries []auth.GrantHistorySequencePair
 
 func (h *handler) handleGetAllChannels() error {
 	h.assertAdminOnly()
-	user, err := h.db.Authenticator(h.ctx()).GetUser(internalUserName(mux.Vars(h.rq)["name"]))
+	name := internalUserName(mux.Vars(h.rq)["name"])
+	user, err := h.db.Authenticator(h.ctx()).GetUser(name)
 	if err != nil {
-		return fmt.Errorf("could not get user %s: %w", user.Name(), err)
+		return fmt.Errorf("could not get user %s: %w", name, err)
 	}
 	if user == nil {
 		return kNotFoundError
@@ -221,9 +222,10 @@ func (h *handler) handleGetAllChannels() error {
 
 func (h *handler) handleGetUserDocAccessSpan() error {
 	h.assertAdminOnly()
-	user, err := h.db.Authenticator(h.ctx()).GetUser(internalUserName(mux.Vars(h.rq)["name"]))
+	name := internalUserName(mux.Vars(h.rq)["name"])
+	user, err := h.db.Authenticator(h.ctx()).GetUser(name)
 	if err != nil {
-		return fmt.Errorf("could not get user %s: %w", user.Name(), err)
+		return fmt.Errorf("could not get user %s: %w", name, err)
 	}
 	if user == nil {
 		return kNotFoundError

rest/handler.go

@@ -521,10 +521,13 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
 				return fmt.Errorf("failed to revoke stale OIDC roles/channels: %w", err)
 			}
 			// TODO: could avoid this extra fetch if UpdatePrincipal returned the new principal
-			h.user, err = dbContext.Authenticator(h.ctx()).GetUser(*updates.Name)
+			user, err := dbContext.Authenticator(h.ctx()).GetUser(*updates.Name)
 			if err != nil {
 				return err
+			} else if user == nil {
+				return ErrInvalidLogin
 			}
+			h.user = user
 		}
 	}
 
@@ -981,7 +984,13 @@ func (h *handler) setUserForPublicAuth(dbCtx *db.DatabaseContext) (base.AuditFie
 			}
 			// TODO: could avoid this extra fetch if UpdatePrincipal returned the newly updated principal
 			if updates.Name != nil {
-				h.user, err = dbCtx.Authenticator(h.ctx()).GetUser(*updates.Name)
+				user, err := dbCtx.Authenticator(h.ctx()).GetUser(*updates.Name)
+				if err != nil {
+					return auditFields, err
+				} else if user == nil {
+					return auditFields, ErrInvalidLogin
+				}
+				h.user = user
 			}
 			return auditFields, err
 		}

rest/session_api.go

@@ -194,9 +194,8 @@ func (h *handler) makeSessionFromNameAndEmail(username, email string, createUser
 		}
 
 		// Create a User with the given username, email address, and a random password.
-		// CAS mismatch indicates the user has been created by another request underneath us, can continue with session creation
 		user, err = h.db.Authenticator(h.ctx()).RegisterNewUser(username, email)
-		if err != nil && !base.IsCasMismatch(err) {
+		if err != nil {
 			return err
 		}
 	}