diff --git a/apps/api/internal/auth/oidc.go b/apps/api/internal/auth/oidc.go index eaca8c2..fa8c539 100644 --- a/apps/api/internal/auth/oidc.go +++ b/apps/api/internal/auth/oidc.go @@ -132,15 +132,28 @@ func (v *OIDCVerifier) Verify(ctx context.Context, raw string) (*User, error) { parser := jwt.NewParser(jwt.WithValidMethods([]string{"RS256", "ES256"})) unverified, _, err := parser.ParseUnverified(raw, jwt.MapClaims{}) if err != nil || unverified == nil { - return nil, oidcUnauthorized("token envelope is invalid", err) + return nil, oidcUnauthorized("TOKEN_ENVELOPE_INVALID", "token envelope is invalid", err) + } + unverifiedClaims, ok := unverified.Claims.(jwt.MapClaims) + if !ok { + return nil, oidcUnauthorized("TOKEN_ENVELOPE_INVALID", "token envelope is invalid", nil) + } + if stringClaim(unverifiedClaims, "iss") == "" { + return nil, oidcUnauthorized("ISSUER_MISSING", "issuer is missing", nil) + } + if len(stringSliceClaim(unverifiedClaims, "aud")) == 0 { + return nil, oidcUnauthorized("AUDIENCE_MISSING", "audience is missing", nil) + } + if _, exists := unverifiedClaims["exp"]; !exists { + return nil, oidcUnauthorized("EXP_MISSING", "exp is missing", nil) } kid, _ := unverified.Header["kid"].(string) if kid == "" { - return nil, oidcUnauthorized("kid is missing", nil) + return nil, oidcUnauthorized("KID_MISSING", "kid is missing", nil) } key, err := v.key(ctx, kid) if err != nil { - return nil, oidcUnauthorized("signing key lookup failed", err) + return nil, oidcUnauthorized("SIGNING_KEY_LOOKUP_FAILED", "signing key lookup failed", err) } token, err := jwt.Parse(raw, func(token *jwt.Token) (any, error) { if token.Header["kid"] != kid { @@ -150,30 +163,30 @@ func (v *OIDCVerifier) Verify(ctx context.Context, raw string) (*User, error) { }, jwt.WithValidMethods([]string{"RS256", "ES256"}), jwt.WithIssuer(v.config.Issuer), jwt.WithAudience(v.config.Audience), jwt.WithExpirationRequired(), jwt.WithLeeway(30*time.Second)) if err != nil || !token.Valid { - return nil, oidcUnauthorized("signature or registered claims are invalid", err) + return nil, oidcUnauthorized(registeredClaimsValidationCategory(err), "signature or registered claims are invalid", err) } claims, ok := token.Claims.(jwt.MapClaims) if !ok || stringClaim(claims, "sub") == "" || stringClaim(claims, "tid") != v.config.TenantID { - return nil, oidcUnauthorized("stable identity claims are invalid", nil) + return nil, oidcUnauthorized("STABLE_IDENTITY_CLAIMS_INVALID", "stable identity claims are invalid", nil) } expiresAt, ok := numericDateClaim(claims["exp"]) if !ok { - return nil, oidcUnauthorized("exp is invalid", nil) + return nil, oidcUnauthorized("EXP_INVALID", "exp is invalid", nil) } if _, ok := numericDateClaim(claims["nbf"]); !ok { - return nil, oidcUnauthorized("nbf is missing", nil) + return nil, oidcUnauthorized("NBF_MISSING", "nbf is missing", nil) } issuedAt, hasIssuedAt := numericDateClaim(claims["iat"]) scopes := scopeClaims(claims) if !containsAll(scopes, v.config.RequiredScopes) { - return nil, oidcUnauthorized("required scope is missing", nil) + return nil, oidcUnauthorized("REQUIRED_SCOPE_MISSING", "required scope is missing", nil) } roles := gatewayRoles(stringSliceClaim(claims, "roles"), v.config.RolePrefix) if len(roles) == 0 { roles = gatewayRoles(stringSliceClaim(claims, "role"), v.config.RolePrefix) } if len(roles) == 0 { - return nil, oidcUnauthorized("mapped role is missing", nil) + return nil, oidcUnauthorized("MAPPED_ROLE_MISSING", "mapped role is missing", nil) } if v.config.SecurityEventEvaluator != nil { evaluation, evaluateErr := v.config.SecurityEventEvaluator(ctx, OIDCSecurityEventIdentity{ @@ -183,10 +196,10 @@ func (v *OIDCVerifier) Verify(ctx context.Context, raw string) (*User, error) { return nil, NewRequestAuthError(http.StatusServiceUnavailable, "OIDC_SECURITY_EVENT_STATE_UNAVAILABLE", "认证撤销状态暂时不可用") } if evaluation.Enabled && !hasIssuedAt { - return nil, oidcUnauthorized("iat is required when security events are enabled", nil) + return nil, oidcUnauthorized("SECURITY_EVENT_IAT_MISSING", "iat is required when security events are enabled", nil) } if evaluation.Revoked { - return nil, oidcUnauthorized("token was issued before the revocation watermark", nil) + return nil, oidcUnauthorized("TOKEN_REVOKED", "token was issued before the revocation watermark", nil) } if evaluation.RequireIntrospection { active, introspectionErr := v.introspect(ctx, raw) @@ -194,18 +207,18 @@ func (v *OIDCVerifier) Verify(ctx context.Context, raw string) (*User, error) { return nil, NewRequestAuthError(http.StatusServiceUnavailable, "OIDC_INTROSPECTION_UNAVAILABLE", "认证中心内省暂时不可用") } if !active { - return nil, oidcUnauthorized("token is inactive", nil) + return nil, oidcUnauthorized("TOKEN_INACTIVE", "token is inactive", nil) } } else if !evaluation.Enabled && v.config.IntrospectionEnabled { active, introspectionErr := v.introspect(ctx, raw) if introspectionErr != nil || !active { - return nil, oidcUnauthorized("token is inactive", introspectionErr) + return nil, oidcUnauthorized("TOKEN_INACTIVE", "token is inactive", introspectionErr) } } } else if v.config.IntrospectionEnabled { active, err := v.introspect(ctx, raw) if err != nil || !active { - return nil, oidcUnauthorized("token is inactive", err) + return nil, oidcUnauthorized("TOKEN_INACTIVE", "token is inactive", err) } } username := stringClaim(claims, "preferred_username") @@ -218,11 +231,67 @@ func (v *OIDCVerifier) Verify(ctx context.Context, raw string) (*User, error) { }, nil } -func oidcUnauthorized(reason string, cause error) error { - if cause != nil { - return fmt.Errorf("%w: %s: %v", ErrUnauthorized, reason, cause) +type oidcValidationError struct { + category string + reason string + cause error +} + +func (err *oidcValidationError) Error() string { + if err.cause != nil { + return fmt.Sprintf("%v: %s: %v", ErrUnauthorized, err.reason, err.cause) + } + return fmt.Sprintf("%v: %s", ErrUnauthorized, err.reason) +} + +func (err *oidcValidationError) Unwrap() []error { + if err.cause == nil { + return []error{ErrUnauthorized} + } + return []error{ErrUnauthorized, err.cause} +} + +func OIDCValidationCategory(err error) string { + var validationError *oidcValidationError + if errors.As(err, &validationError) { + return validationError.category + } + return "UNCLASSIFIED" +} + +func registeredClaimsValidationCategory(err error) string { + switch { + case errors.Is(err, jwt.ErrTokenRequiredClaimMissing): + return "REQUIRED_CLAIM_MISSING" + case errors.Is(err, jwt.ErrTokenInvalidAudience): + return "AUDIENCE_INVALID" + case errors.Is(err, jwt.ErrTokenInvalidIssuer): + return "ISSUER_INVALID" + case errors.Is(err, jwt.ErrTokenExpired): + return "TOKEN_EXPIRED" + case errors.Is(err, jwt.ErrTokenNotValidYet): + return "TOKEN_NOT_VALID_YET" + case errors.Is(err, jwt.ErrTokenUsedBeforeIssued): + return "TOKEN_USED_BEFORE_ISSUED" + case errors.Is(err, jwt.ErrTokenSignatureInvalid): + return "SIGNATURE_INVALID" + case errors.Is(err, jwt.ErrTokenMalformed): + return "TOKEN_MALFORMED" + case errors.Is(err, jwt.ErrTokenUnverifiable): + return "TOKEN_UNVERIFIABLE" + case errors.Is(err, jwt.ErrTokenInvalidClaims): + return "CLAIMS_INVALID" + default: + return "REGISTERED_CLAIMS_INVALID" + } +} + +func oidcUnauthorized(category, reason string, cause error) error { + return &oidcValidationError{ + category: category, + reason: reason, + cause: cause, } - return fmt.Errorf("%w: %s", ErrUnauthorized, reason) } func (v *OIDCVerifier) key(ctx context.Context, kid string) (any, error) { diff --git a/apps/api/internal/auth/oidc_client.go b/apps/api/internal/auth/oidc_client.go index 5472adb..608380e 100644 --- a/apps/api/internal/auth/oidc_client.go +++ b/apps/api/internal/auth/oidc_client.go @@ -121,30 +121,30 @@ func (c *OIDCPublicClient) Refresh(ctx context.Context, refreshToken string) (OI func (c *OIDCPublicClient) VerifyIDToken(ctx context.Context, raw, expectedNonce string) (string, error) { expectedNonce = strings.TrimSpace(expectedNonce) if strings.TrimSpace(raw) == "" || expectedNonce == "" { - return "", oidcUnauthorized("ID token validation context is invalid", nil) + return "", oidcUnauthorized("ID_TOKEN_CONTEXT_INVALID", "ID token validation context is invalid", nil) } if _, _, err := c.configuration(ctx); err != nil { - return "", oidcUnauthorized("ID token provider discovery failed", err) + return "", oidcUnauthorized("ID_TOKEN_DISCOVERY_FAILED", "ID token provider discovery failed", err) } c.mu.Lock() verifier := c.idTokenVerifier c.mu.Unlock() if verifier == nil { - return "", oidcUnauthorized("ID token verifier is unavailable", nil) + return "", oidcUnauthorized("ID_TOKEN_VERIFIER_UNAVAILABLE", "ID token verifier is unavailable", nil) } token, err := verifier.Verify(c.requestContext(ctx), raw) if err != nil { - return "", oidcUnauthorized("ID token signature or registered claims are invalid", nil) + return "", oidcUnauthorized("ID_TOKEN_REGISTERED_CLAIMS_INVALID", "ID token signature or registered claims are invalid", nil) } if token.Subject == "" || token.Nonce != expectedNonce { - return "", oidcUnauthorized("ID token subject or nonce is invalid", nil) + return "", oidcUnauthorized("ID_TOKEN_SUBJECT_OR_NONCE_INVALID", "ID token subject or nonce is invalid", nil) } var claims map[string]any if err := token.Claims(&claims); err != nil { - return "", oidcUnauthorized("ID token claims are invalid", nil) + return "", oidcUnauthorized("ID_TOKEN_CLAIMS_INVALID", "ID token claims are invalid", nil) } if _, ok := numericDateClaim(claims["nbf"]); !ok { - return "", oidcUnauthorized("ID token nbf is missing", nil) + return "", oidcUnauthorized("ID_TOKEN_NBF_MISSING", "ID token nbf is missing", nil) } return token.Subject, nil } diff --git a/apps/api/internal/auth/oidc_test.go b/apps/api/internal/auth/oidc_test.go index df7683d..d197cda 100644 --- a/apps/api/internal/auth/oidc_test.go +++ b/apps/api/internal/auth/oidc_test.go @@ -91,20 +91,30 @@ func TestOIDCVerifierRejectsMissingOrMismatchedSecurityClaims(t *testing.T) { RequiredScopes: []string{"gateway.access"}, HTTPClient: server.Client(), }) tests := []struct { - name string - mutate func(jwt.MapClaims) + name, wantCategory string + mutate func(jwt.MapClaims) }{ - {"missing nbf", func(claims jwt.MapClaims) { delete(claims, "nbf") }}, - {"wrong audience", func(claims jwt.MapClaims) { claims["aud"] = "other-api" }}, - {"wrong tenant", func(claims jwt.MapClaims) { claims["tid"] = "tenant-2" }}, - {"missing scope", func(claims jwt.MapClaims) { claims["scope"] = "openid" }}, - {"unmapped role", func(claims jwt.MapClaims) { claims["roles"] = []string{"other.admin"} }}, + {"missing nbf", "NBF_MISSING", func(claims jwt.MapClaims) { delete(claims, "nbf") }}, + {"missing exp", "EXP_MISSING", func(claims jwt.MapClaims) { delete(claims, "exp") }}, + {"missing issuer", "ISSUER_MISSING", func(claims jwt.MapClaims) { delete(claims, "iss") }}, + {"missing audience", "AUDIENCE_MISSING", func(claims jwt.MapClaims) { delete(claims, "aud") }}, + {"wrong audience", "AUDIENCE_INVALID", func(claims jwt.MapClaims) { claims["aud"] = "other-api" }}, + {"wrong tenant", "STABLE_IDENTITY_CLAIMS_INVALID", func(claims jwt.MapClaims) { claims["tid"] = "tenant-2" }}, + {"missing scope", "REQUIRED_SCOPE_MISSING", func(claims jwt.MapClaims) { claims["scope"] = "openid" }}, + {"unmapped role", "MAPPED_ROLE_MISSING", func(claims jwt.MapClaims) { claims["roles"] = []string{"other.admin"} }}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { raw := signedOIDCToken(t, issuer, "ec-key", jwt.SigningMethodES256, key, test.mutate) if _, err := verifier.Verify(context.Background(), raw); err == nil { t.Fatal("expected token to be rejected") + } else { + if !errors.Is(err, ErrUnauthorized) { + t.Fatalf("validation error no longer wraps ErrUnauthorized: %v", err) + } + if category := OIDCValidationCategory(err); category != test.wantCategory { + t.Fatalf("validation category=%q, want %q", category, test.wantCategory) + } } }) } diff --git a/apps/api/internal/httpapi/oidc_session.go b/apps/api/internal/httpapi/oidc_session.go index dc6ad81..1e6898d 100644 --- a/apps/api/internal/httpapi/oidc_session.go +++ b/apps/api/internal/httpapi/oidc_session.go @@ -111,17 +111,17 @@ func (s *Server) completeOIDCLogin(w http.ResponseWriter, r *http.Request) { } tokens, err := runtime.PublicClient.ExchangeCode(r.Context(), r.URL.Query().Get("code"), transaction.PKCEVerifier) if err != nil || tokens.AccessToken == "" || tokens.RefreshToken == "" || tokens.IDToken == "" { - s.writeOIDCCallbackError(w, r, http.StatusUnauthorized, "认证中心登录结果无效，请重新登录", errorCodeOIDCTokenExchangeFailed) + s.writeOIDCTokenFailure(w, r, "TOKEN_ENDPOINT_REJECTED", "TOKEN_RESPONSE_INVALID", "认证中心登录结果无效，请重新登录") return } identity, err := runtime.Verifier.Verify(r.Context(), tokens.AccessToken) if err != nil || identity == nil { - s.writeOIDCCallbackError(w, r, http.StatusUnauthorized, "认证中心访问令牌校验失败", errorCodeOIDCTokenExchangeFailed) + s.writeOIDCTokenFailure(w, r, "ACCESS_TOKEN_INVALID", auth.OIDCValidationCategory(err), "认证中心访问令牌校验失败") return } idSubject, err := runtime.PublicClient.VerifyIDToken(r.Context(), tokens.IDToken, transaction.Nonce) if err != nil || idSubject != identity.ID { - s.writeOIDCCallbackError(w, r, http.StatusUnauthorized, "认证中心身份令牌校验失败", errorCodeOIDCTokenExchangeFailed) + s.writeOIDCTokenFailure(w, r, "ID_TOKEN_INVALID", auth.OIDCValidationCategory(err), "认证中心身份令牌校验失败") return } projection, err := s.resolveOIDCUserProjectionForRevision(r.Context(), r, identity, runtime.Revision) @@ -253,6 +253,27 @@ func (s *Server) writeOIDCCallbackError(w http.ResponseWriter, r *http.Request, s.writeOIDCCallbackErrorWithDiagnostics(w, r, status, message, code, "", "") } +func (s *Server) writeOIDCTokenFailure(w http.ResponseWriter, r *http.Request, stage, validationCategory, message string) { + diagnosticID := newOIDCDiagnosticID() + if s.logger != nil { + s.logger.WarnContext(r.Context(), "OIDC token processing rejected", + "event", "oidc_token_processing_rejected", + "stage", stage, + "validationCategory", validationCategory, + "diagnosticId", diagnosticID, + ) + } + s.writeOIDCCallbackErrorWithDiagnostics( + w, + r, + http.StatusUnauthorized, + message, + errorCodeOIDCTokenExchangeFailed, + "", + diagnosticID, + ) +} + func (s *Server) writeOIDCLoginTransactionError(w http.ResponseWriter, r *http.Request, message, reason string) { diagnosticID := newOIDCDiagnosticID() if s.logger != nil { diff --git a/apps/api/internal/httpapi/oidc_session_test.go b/apps/api/internal/httpapi/oidc_session_test.go index e7c78fa..f4b1c97 100644 --- a/apps/api/internal/httpapi/oidc_session_test.go +++ b/apps/api/internal/httpapi/oidc_session_test.go @@ -128,6 +128,51 @@ func TestCompleteOIDCLoginReportsSafeTransactionFailureReason(t *testing.T) { } } +func TestOIDCTokenFailureReportsOnlyCorrelatedSafeStage(t *testing.T) { + var logs bytes.Buffer + server := &Server{ + identityTestRevision: identity.Revision{ + ID: "diagnostic-revision", WebBaseURL: "http://localhost:5178", + }, + logger: slog.New(slog.NewJSONHandler(&logs, nil)), + } + request := httptest.NewRequest( + http.MethodGet, + "/api/v1/auth/oidc/callback?code=sensitive-code-marker", + nil, + ) + recorder := httptest.NewRecorder() + + server.writeOIDCTokenFailure( + recorder, + request, + "ACCESS_TOKEN_INVALID", + "REQUIRED_SCOPE_MISSING", + "认证中心访问令牌校验失败", + ) + + if recorder.Code != http.StatusSeeOther { + t.Fatalf("callback status=%d, want 303", recorder.Code) + } + location, err := url.Parse(recorder.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + diagnosticID := location.Query().Get("oidcDiagnosticId") + if location.Query().Get("oidcError") != errorCodeOIDCTokenExchangeFailed || + location.Query().Get("oidcErrorReason") != "" || + diagnosticID == "" { + t.Fatalf("unsafe callback diagnostic: %q", location.RawQuery) + } + if !strings.Contains(logs.String(), `"diagnosticId":"`+diagnosticID+`"`) || + !strings.Contains(logs.String(), `"stage":"ACCESS_TOKEN_INVALID"`) || + !strings.Contains(logs.String(), `"validationCategory":"REQUIRED_SCOPE_MISSING"`) || + strings.Contains(logs.String(), "sensitive-code-marker") || + strings.Contains(location.RawQuery, "sensitive-code-marker") { + t.Fatalf("OIDC token diagnostic was missing or leaked callback material") + } +} + func TestDeleteOIDCBrowserSessionIsIdempotentAndExpiresCookie(t *testing.T) { sessions := &fakeOIDCSessions{} server := &Server{oidcSessions: sessions, identityTestCookieSecure: true}