Bubble back err if role fetch fails

Author: torcolvin

rest/handler.go

@@ -908,8 +908,8 @@ func getSGUserRolesForAudit(db *db.DatabaseContext, user auth.User) ([]string, e
 	return roleNames, nil
 }
 
-// checkPublicAuth verifies that the current request is authenticated for the given database.
-//
+// checkPublicAuth verifies that the current request is authenticated for the given database. Returns an HTTPError if
+// authentication fails.
 // NOTE: checkPublicAuth is not used for the admin interface.
 func (h *handler) checkPublicAuth(dbCtx *db.DatabaseContext) (err error) {
 
@@ -918,59 +918,69 @@ func (h *handler) checkPublicAuth(dbCtx *db.DatabaseContext) (err error) {
 		return nil
 	}
 
-	var auditFields base.AuditFields
-
-	// Record Auth stats
-	defer func(t time.Time) {
-		delta := time.Since(t).Nanoseconds()
-		dbCtx.DbStats.Security().TotalAuthTime.Add(delta)
-		if err != nil {
-			dbCtx.DbStats.Security().AuthFailedCount.Add(1)
-			if errors.Is(err, ErrInvalidLogin) {
-				base.Audit(h.ctx(), base.AuditIDPublicUserAuthenticationFailed, auditFields)
-			}
-		} else {
-			dbCtx.DbStats.Security().AuthSuccessCount.Add(1)
-
-			username := ""
-			if h.isGuest() {
-				username = base.GuestUsername
-			} else if h.user != nil {
-				username = h.user.Name()
-			}
-			roleNames, err := getSGUserRolesForAudit(dbCtx, h.user)
-			if err != nil {
-				base.InfofCtx(h.ctx(), base.KeyHTTP, "Error getting user roles for audit logging: %v", err)
-			}
-			h.rqCtx = base.UserLogCtx(h.ctx(), username, base.UserDomainSyncGateway, roleNames)
-			base.Audit(h.ctx(), base.AuditIDPublicUserAuthenticated, auditFields)
+	start := time.Now()
+	auditFields, err := h.setUserForPublicAuth(dbCtx)
+	dbCtx.DbStats.Security().TotalAuthTime.Add(time.Since(start).Nanoseconds())
+	if err != nil {
+		dbCtx.DbStats.Security().AuthFailedCount.Add(1)
+		if errors.Is(err, ErrInvalidLogin) {
+			base.Audit(h.ctx(), base.AuditIDPublicUserAuthenticationFailed, auditFields)
 		}
-	}(time.Now())
+		return
+	}
+	dbCtx.DbStats.Security().AuthSuccessCount.Add(1)
+
+	username := ""
+	if h.isGuest() {
+		username = base.GuestUsername
+	} else if h.user != nil {
+		username = h.user.Name()
+	}
+	roleNames, err := getSGUserRolesForAudit(dbCtx, h.user)
+	if err != nil {
+		base.InfofCtx(h.ctx(), base.KeyHTTP, "Error getting user roles for audit logging: %v", err)
+	}
+	h.rqCtx = base.UserLogCtx(h.ctx(), username, base.UserDomainSyncGateway, roleNames)
+	base.Audit(h.ctx(), base.AuditIDPublicUserAuthenticated, auditFields)
+	return err
+}
 
+// setUserForPublicAuth sets h.user based on the authentication information in the request. Returns an error if the user
+// can not authenticate successfully, and returns AuditFields even in the case that there is an error in the request.
+//
+// Uses:
+//
+//  1. Bearer token (OIDC JWT) if present and OIDC is enabled
+//  2. Basic auth if present and password authentication is not disabled
+//  3. Cookie auth if present
+//  4. Guest access if enabled
+func (h *handler) setUserForPublicAuth(dbCtx *db.DatabaseContext) (base.AuditFields, error) {
+	var auditFields base.AuditFields
 	// If oidc enabled, check for bearer ID token
 	if dbCtx.Options.OIDCOptions != nil || len(dbCtx.LocalJWTProviders) > 0 {
 		if token := h.getBearerToken(); token != "" {
 			auditFields = base.AuditFields{base.AuditFieldAuthMethod: "bearer"}
 			var updates auth.PrincipalConfig
+			var err error
 			h.user, updates, err = dbCtx.Authenticator(h.ctx()).AuthenticateUntrustedJWT(token, dbCtx.OIDCProviders, dbCtx.LocalJWTProviders, h.getOIDCCallbackURL)
 			if h.user == nil || err != nil {
-				return ErrInvalidLogin
+				return auditFields, ErrInvalidLogin
 			}
 			if issuer := h.user.JWTIssuer(); issuer != "" {
 				auditFields["oidc_issuer"] = issuer
 			}
 			if changes := checkJWTIssuerStillValid(h.ctx(), dbCtx, h.user); changes != nil {
 				updates = updates.Merge(*changes)
 			}
-			_, _, err := dbCtx.UpdatePrincipal(h.ctx(), &updates, true, true)
+			_, _, err = dbCtx.UpdatePrincipal(h.ctx(), &updates, true, true)
 			if err != nil {
-				return fmt.Errorf("failed to update OIDC user after sign-in: %w", err)
+				return auditFields, fmt.Errorf("failed to update OIDC user after sign-in: %w", err)
 			}
 			// 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)
 			}
-			return err
+			return auditFields, err
 		}
 
 		/*
@@ -985,8 +995,7 @@ func (h *handler) checkPublicAuth(dbCtx *db.DatabaseContext) (err error) {
 				provider := dbCtx.Options.OIDCOptions.Providers.GetProviderForIssuer(h.ctx(), issuerUrlForDB(h, dbCtx.Name), testProviderAudiences)
 				if provider != nil && provider.ValidationKey != nil {
 					if base.ValDefault(provider.ClientID, "") == username && *provider.ValidationKey == password {
-						auditFields = base.AuditFields{base.AuditFieldAuthMethod: "basic"}
-						return nil
+						return base.AuditFields{base.AuditFieldAuthMethod: "basic"}, nil
 					}
 				}
 			}
@@ -996,47 +1005,49 @@ func (h *handler) checkPublicAuth(dbCtx *db.DatabaseContext) (err error) {
 	// Check basic auth first
 	if !dbCtx.Options.DisablePasswordAuthentication {
 		if userName, password := h.getBasicAuth(); userName != "" {
-			auditFields = base.AuditFields{base.AuditFieldAuthMethod: "basic"}
+			auditFields := base.AuditFields{base.AuditFieldAuthMethod: "basic"}
+			var err error
 			h.user, err = dbCtx.Authenticator(h.ctx()).AuthenticateUser(userName, password)
 			if err != nil {
-				return err
+				return auditFields, err
 			}
 			if h.user == nil {
 				auditFields["username"] = userName
 				if dbCtx.Options.SendWWWAuthenticateHeader == nil || *dbCtx.Options.SendWWWAuthenticateHeader {
 					h.response.Header().Set("WWW-Authenticate", wwwAuthenticateHeader)
 				}
-				return ErrInvalidLogin
+				return auditFields, ErrInvalidLogin
 			}
-			return nil
+			return auditFields, nil
 		}
 	}
 
 	// Check cookie
 	auditFields = base.AuditFields{base.AuditFieldAuthMethod: "cookie"}
+	var err error
 	h.user, err = dbCtx.Authenticator(h.ctx()).AuthenticateCookie(h.rq, h.response)
 	if err != nil && h.privs != publicPrivs {
-		return err
+		return auditFields, err
 	} else if h.user != nil {
-		return nil
+		return auditFields, nil
 	}
 
 	// No auth given -- check guest access
 	auditFields = base.AuditFields{base.AuditFieldAuthMethod: "guest"}
 	if h.user, err = dbCtx.Authenticator(h.ctx()).GetUser(""); err != nil {
-		return err
+		return auditFields, err
 	}
 	if h.privs == regularPrivs && h.user.Disabled() {
 		if dbCtx.Options.SendWWWAuthenticateHeader == nil || *dbCtx.Options.SendWWWAuthenticateHeader {
 			h.response.Header().Set("WWW-Authenticate", wwwAuthenticateHeader)
 		}
 		if h.providedAuthCredentials() {
-			return ErrInvalidLogin
+			return auditFields, ErrInvalidLogin
 		}
-		return ErrLoginRequired
+		return auditFields, ErrLoginRequired
 	}
 
-	return nil
+	return auditFields, nil
 }
 
 func checkJWTIssuerStillValid(ctx context.Context, dbCtx *db.DatabaseContext, user auth.User) *auth.PrincipalConfig {