From aae20ef4370516158e681d03ecd98071a45cd267 Mon Sep 17 00:00:00 2001 From: shaw Date: Thu, 21 May 2026 15:19:29 +0800 Subject: [PATCH] fix(oidc): harden verified-email fast path --- .../handler/auth_oauth_test_helpers_test.go | 18 +- backend/internal/handler/auth_oidc_oauth.go | 46 ++--- .../internal/handler/auth_oidc_oauth_test.go | 159 +++++++++++++++++- 3 files changed, 195 insertions(+), 28 deletions(-) diff --git a/backend/internal/handler/auth_oauth_test_helpers_test.go b/backend/internal/handler/auth_oauth_test_helpers_test.go index 47bad942..5022ffe8 100644 --- a/backend/internal/handler/auth_oauth_test_helpers_test.go +++ b/backend/internal/handler/auth_oauth_test_helpers_test.go @@ -2,6 +2,7 @@ package handler import ( "net/http" + "net/http/httptest" "net/url" "testing" @@ -32,6 +33,13 @@ func findCookie(cookies []*http.Cookie, name string) *http.Cookie { return nil } +func requireCookieCleared(t *testing.T, recorder *httptest.ResponseRecorder, name string) { + t.Helper() + cookie := findCookie(recorder.Result().Cookies(), name) + require.NotNil(t, cookie) + require.Equal(t, -1, cookie.MaxAge) +} + func decodeCookieValueForTest(t *testing.T, value string) string { t.Helper() decoded, err := decodeCookieValue(value) @@ -40,6 +48,13 @@ func decodeCookieValueForTest(t *testing.T, value string) string { } func assertOAuthRedirectError(t *testing.T, location string, errorCode string, errorMessage string) { + t.Helper() + values := parseOAuthRedirectFragment(t, location) + require.Equal(t, errorCode, values.Get("error")) + require.Equal(t, errorMessage, values.Get("error_message")) +} + +func parseOAuthRedirectFragment(t *testing.T, location string) url.Values { t.Helper() require.NotEmpty(t, location) @@ -52,6 +67,5 @@ func assertOAuthRedirectError(t *testing.T, location string, errorCode string, e } values, err := url.ParseQuery(rawValues) require.NoError(t, err) - require.Equal(t, errorCode, values.Get("error")) - require.Equal(t, errorMessage, values.Get("error_message")) + return values } diff --git a/backend/internal/handler/auth_oidc_oauth.go b/backend/internal/handler/auth_oidc_oauth.go index c82f7e2a..76eb9498 100644 --- a/backend/internal/handler/auth_oidc_oauth.go +++ b/backend/internal/handler/auth_oidc_oauth.go @@ -454,16 +454,12 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { } } - // Fast-path: when the upstream supplies a verified email and the deployment - // does not require any extra confirmation (force_email_on_third_party_signup - // disabled + invitation code disabled) and no local account collides with the - // upstream email, trust the upstream identity and finish login without - // rendering the choice page. Any failure falls through to the regular choice - // flow below. + // 快捷路径：当上游返回已验证邮箱、部署不要求额外确认且本地没有同邮箱账号时， + // 直接信任上游身份完成注册/登录，避免展示 choice 页。 if compatEmailUser == nil && strings.TrimSpace(compatEmail) != "" && emailVerified != nil && *emailVerified { - if h.tryOIDCVerifiedEmailFastPath( + if handled := h.tryOIDCVerifiedEmailFastPath( c, frontendCallback, redirectTo, @@ -471,7 +467,7 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { compatEmail, username, upstreamClaims, - ) { + ); handled { return } } @@ -1213,14 +1209,8 @@ func oidcClearCookie(c *gin.Context, name string, secure bool) { }) } -// tryOIDCVerifiedEmailFastPath attempts to skip the choice/pending page when -// the upstream identity carries a verified email and the deployment does not -// require any additional confirmation. It mirrors the behaviour already used -// for verified github/google logins via LoginOrRegisterVerifiedEmailOAuth. -// -// Returns true when the flow has completed (token issued and browser -// redirected); a false return tells the caller to fall through to the regular -// choice flow. +// tryOIDCVerifiedEmailFastPath 在 OIDC 上游已返回已验证邮箱时尝试跳过 choice/pending 页。 +// 返回 true 表示已经写出重定向响应；返回 false 表示调用方应继续回退到常规 choice 流程。 func (h *AuthHandler) tryOIDCVerifiedEmailFastPath( c *gin.Context, frontendCallback string, @@ -1240,31 +1230,39 @@ func (h *AuthHandler) tryOIDCVerifiedEmailFastPath( if h.settingSvc.IsInvitationCodeEnabled(ctx) { return false } + if err := h.ensureBackendModeAllowsNewUserLogin(ctx); err != nil { + log.Printf("[OIDC OAuth] verified-email fast path blocked by backend mode: reason=%s", infraerrors.Reason(err)) + clearOAuthPendingSessionCookie(c, isRequestHTTPS(c)) + clearOAuthPendingBrowserCookie(c, isRequestHTTPS(c)) + redirectOAuthError(c, frontendCallback, "login_blocked", infraerrors.Reason(err), infraerrors.Message(err)) + return true + } - upstreamMetadata := make(map[string]any, len(upstreamClaims)) + verifiedEmail := strings.TrimSpace(strings.ToLower(compatEmail)) + upstreamMetadata := make(map[string]any, len(upstreamClaims)+1) for k, v := range upstreamClaims { upstreamMetadata[k] = v } + if syntheticEmail := pendingSessionStringValue(upstreamClaims, "email"); syntheticEmail != "" && !strings.EqualFold(syntheticEmail, verifiedEmail) { + upstreamMetadata["synthetic_email"] = syntheticEmail + } + upstreamMetadata["email"] = verifiedEmail input := service.EmailOAuthIdentityInput{ ProviderType: strings.TrimSpace(identity.ProviderType), ProviderKey: strings.TrimSpace(identity.ProviderKey), ProviderSubject: strings.TrimSpace(identity.ProviderSubject), - Email: strings.TrimSpace(strings.ToLower(compatEmail)), + Email: verifiedEmail, EmailVerified: true, Username: strings.TrimSpace(username), DisplayName: pendingSessionStringValue(upstreamClaims, "suggested_display_name"), AvatarURL: pendingSessionStringValue(upstreamClaims, "suggested_avatar_url"), UpstreamMetadata: upstreamMetadata, } - tokenPair, user, err := h.authService.LoginOrRegisterVerifiedEmailOAuthWithInvitation(ctx, input, "", "") + tokenPair, _, err := h.authService.LoginOrRegisterVerifiedEmailOAuthWithInvitation(ctx, input, "", "") if err != nil { log.Printf("[OIDC OAuth] verified-email fast path skipped: reason=%s", infraerrors.Reason(err)) return false } - if err := h.ensureBackendModeAllowsUser(ctx, user); err != nil { - log.Printf("[OIDC OAuth] verified-email fast path blocked by backend mode: reason=%s", infraerrors.Reason(err)) - return false - } fragment := url.Values{} fragment.Set("access_token", tokenPair.AccessToken) @@ -1272,6 +1270,8 @@ func (h *AuthHandler) tryOIDCVerifiedEmailFastPath( fragment.Set("expires_in", fmt.Sprintf("%d", tokenPair.ExpiresIn)) fragment.Set("token_type", "Bearer") fragment.Set("redirect", redirectTo) + clearOAuthPendingSessionCookie(c, isRequestHTTPS(c)) + clearOAuthPendingBrowserCookie(c, isRequestHTTPS(c)) redirectWithFragment(c, frontendCallback, fragment) return true } diff --git a/backend/internal/handler/auth_oidc_oauth_test.go b/backend/internal/handler/auth_oidc_oauth_test.go index 1bb04195..08bb459a 100644 --- a/backend/internal/handler/auth_oidc_oauth_test.go +++ b/backend/internal/handler/auth_oidc_oauth_test.go @@ -10,6 +10,7 @@ import ( "math/big" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -966,20 +967,129 @@ func TestTryOIDCVerifiedEmailFastPathCreatesUserAndIdentity(t *testing.T) { require.Equal(t, "fastpath_user", user.Username) require.Equal(t, "oidc", user.SignupSource) - identityCount, err := client.AuthIdentity.Query().Where( + identityRecord, err := client.AuthIdentity.Query().Where( authidentity.ProviderTypeEQ("oidc"), authidentity.ProviderKeyEQ("https://issuer.example.com"), authidentity.ProviderSubjectEQ("fast-path-subject"), authidentity.UserIDEQ(user.ID), - ).Count(ctx) + ).Only(ctx) require.NoError(t, err) - require.Equal(t, 1, identityCount) + require.Equal(t, "fastpath@example.com", identityRecord.Metadata["email"]) + require.Equal(t, true, identityRecord.Metadata["email_verified"]) pendingCount, err := client.PendingAuthSession.Query().Count(ctx) require.NoError(t, err) require.Zero(t, pendingCount) } +func TestOIDCOAuthCallbackVerifiedEmailFastPathIssuesTokenWithoutPendingSession(t *testing.T) { + cfg, cleanup := newOIDCTestProvider(t, oidcProviderFixture{ + Subject: "oidc-fast-callback-subject", + PreferredUsername: "oidc_fast_callback", + DisplayName: "OIDC Fast Callback", + AvatarURL: "https://cdn.example/oidc-fast.png", + Email: "oidc-fast-callback@example.com", + EmailVerified: true, + }) + defer cleanup() + + handler, client := newOIDCOAuthHandlerAndClientWithSettings(t, false, cfg, nil) + t.Cleanup(func() { _ = client.Close() }) + + recorder := httptest.NewRecorder() + c, _ := gin.CreateTestContext(recorder) + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/oauth/oidc/callback?code=oidc-code&state=state-fast-callback", nil) + req.AddCookie(encodedCookie(oidcOAuthStateCookieName, "state-fast-callback")) + req.AddCookie(encodedCookie(oidcOAuthRedirectCookie, "/dashboard")) + req.AddCookie(encodedCookie(oidcOAuthVerifierCookie, "verifier-fast-callback")) + req.AddCookie(encodedCookie(oidcOAuthNonceCookie, "nonce-oidc-fast-callback-subject")) + req.AddCookie(encodedCookie(oidcOAuthIntentCookieName, oauthIntentLogin)) + req.AddCookie(encodedCookie(oauthPendingBrowserCookieName, "browser-fast-callback")) + c.Request = req + + handler.OIDCOAuthCallback(c) + + require.Equal(t, http.StatusFound, recorder.Code) + location := recorder.Header().Get("Location") + require.Contains(t, location, "/auth/oidc/callback#") + require.Contains(t, location, "access_token=") + require.Contains(t, location, "refresh_token=") + require.Contains(t, location, "token_type=Bearer") + fragmentValues := parseOAuthRedirectFragment(t, location) + require.Equal(t, "/dashboard", fragmentValues.Get("redirect")) + requireCookieCleared(t, recorder, oauthPendingSessionCookieName) + requireCookieCleared(t, recorder, oauthPendingBrowserCookieName) + + ctx := context.Background() + user, err := client.User.Query().Where(dbuser.EmailEQ("oidc-fast-callback@example.com")).Only(ctx) + require.NoError(t, err) + require.Equal(t, "oidc_fast_callback", user.Username) + require.Equal(t, "oidc", user.SignupSource) + + identity, err := client.AuthIdentity.Query().Where( + authidentity.ProviderTypeEQ("oidc"), + authidentity.ProviderKeyEQ(cfg.IssuerURL), + authidentity.ProviderSubjectEQ("oidc-fast-callback-subject"), + authidentity.UserIDEQ(user.ID), + ).Only(ctx) + require.NoError(t, err) + require.Equal(t, "oidc-fast-callback@example.com", identity.Metadata["email"]) + require.Equal(t, true, identity.Metadata["email_verified"]) + require.Equal(t, "OIDC Fast Callback", identity.Metadata["suggested_display_name"]) + require.NotEqual(t, identity.Metadata["email"], identity.Metadata["synthetic_email"]) + + pendingCount, err := client.PendingAuthSession.Query().Count(ctx) + require.NoError(t, err) + require.Zero(t, pendingCount) +} + +func TestOIDCOAuthCallbackVerifiedEmailFastPathBackendModeBlocksBeforeUserCreation(t *testing.T) { + cfg, cleanup := newOIDCTestProvider(t, oidcProviderFixture{ + Subject: "oidc-fast-backend-mode-subject", + PreferredUsername: "oidc_backend_mode", + DisplayName: "OIDC Backend Mode", + Email: "oidc-backend-mode@example.com", + EmailVerified: true, + }) + defer cleanup() + + handler, client := newOIDCOAuthHandlerAndClientWithSettings(t, false, cfg, map[string]string{ + service.SettingKeyBackendModeEnabled: "true", + }) + t.Cleanup(func() { _ = client.Close() }) + + recorder := httptest.NewRecorder() + c, _ := gin.CreateTestContext(recorder) + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/oauth/oidc/callback?code=oidc-code&state=state-backend-mode", nil) + req.AddCookie(encodedCookie(oidcOAuthStateCookieName, "state-backend-mode")) + req.AddCookie(encodedCookie(oidcOAuthRedirectCookie, "/dashboard")) + req.AddCookie(encodedCookie(oidcOAuthVerifierCookie, "verifier-backend-mode")) + req.AddCookie(encodedCookie(oidcOAuthNonceCookie, "nonce-oidc-fast-backend-mode-subject")) + req.AddCookie(encodedCookie(oidcOAuthIntentCookieName, oauthIntentLogin)) + req.AddCookie(encodedCookie(oauthPendingBrowserCookieName, "browser-backend-mode")) + c.Request = req + + handler.OIDCOAuthCallback(c) + + require.Equal(t, http.StatusFound, recorder.Code) + assertOAuthRedirectError(t, recorder.Header().Get("Location"), "login_blocked", "BACKEND_MODE_ADMIN_ONLY") + requireCookieCleared(t, recorder, oauthPendingSessionCookieName) + requireCookieCleared(t, recorder, oauthPendingBrowserCookieName) + + ctx := context.Background() + userCount, err := client.User.Query().Where(dbuser.EmailEQ("oidc-backend-mode@example.com")).Count(ctx) + require.NoError(t, err) + require.Zero(t, userCount) + identityCount, err := client.AuthIdentity.Query(). + Where(authidentity.ProviderSubjectEQ("oidc-fast-backend-mode-subject")). + Count(ctx) + require.NoError(t, err) + require.Zero(t, identityCount) + pendingCount, err := client.PendingAuthSession.Query().Count(ctx) + require.NoError(t, err) + require.Zero(t, pendingCount) +} + func TestTryOIDCVerifiedEmailFastPathSkippedWhenInvitationCodeRequired(t *testing.T) { handler, client := newOAuthPendingFlowTestHandler(t, true) t.Cleanup(func() { _ = client.Close() }) @@ -1074,6 +1184,49 @@ func newOIDCOAuthHandlerAndClient(t *testing.T, invitationEnabled bool, oauthCfg return handler, client } +func newOIDCOAuthHandlerAndClientWithSettings( + t *testing.T, + invitationEnabled bool, + oauthCfg config.OIDCConnectConfig, + settingValues map[string]string, +) (*AuthHandler, *dbent.Client) { + t.Helper() + + values := map[string]string{ + service.SettingKeyOIDCConnectEnabled: "true", + service.SettingKeyOIDCConnectProviderName: strings.TrimSpace(firstNonEmpty(oauthCfg.ProviderName, "OIDC")), + service.SettingKeyOIDCConnectClientID: oauthCfg.ClientID, + service.SettingKeyOIDCConnectClientSecret: oauthCfg.ClientSecret, + service.SettingKeyOIDCConnectIssuerURL: oauthCfg.IssuerURL, + service.SettingKeyOIDCConnectAuthorizeURL: oauthCfg.AuthorizeURL, + service.SettingKeyOIDCConnectTokenURL: oauthCfg.TokenURL, + service.SettingKeyOIDCConnectUserInfoURL: oauthCfg.UserInfoURL, + service.SettingKeyOIDCConnectJWKSURL: oauthCfg.JWKSURL, + service.SettingKeyOIDCConnectScopes: oauthCfg.Scopes, + service.SettingKeyOIDCConnectRedirectURL: oauthCfg.RedirectURL, + service.SettingKeyOIDCConnectFrontendRedirectURL: oauthCfg.FrontendRedirectURL, + service.SettingKeyOIDCConnectTokenAuthMethod: oauthCfg.TokenAuthMethod, + service.SettingKeyOIDCConnectUsePKCE: boolSettingValue(oauthCfg.UsePKCE), + service.SettingKeyOIDCConnectValidateIDToken: boolSettingValue(oauthCfg.ValidateIDToken), + service.SettingKeyOIDCConnectAllowedSigningAlgs: oauthCfg.AllowedSigningAlgs, + service.SettingKeyOIDCConnectClockSkewSeconds: "120", + service.SettingKeyOIDCConnectRequireEmailVerified: boolSettingValue(oauthCfg.RequireEmailVerified), + } + for key, value := range settingValues { + values[key] = value + } + + handler, client := newOAuthPendingFlowTestHandlerWithDependencies(t, oauthPendingFlowTestHandlerOptions{ + invitationEnabled: invitationEnabled, + settingValues: values, + }) + if handler.cfg == nil { + handler.cfg = &config.Config{} + } + handler.cfg.OIDC = oauthCfg + return handler, client +} + func newOIDCTestProvider(t *testing.T, fixture oidcProviderFixture) (config.OIDCConnectConfig, func()) { t.Helper()