diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index f355a15d..32ad91b7 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -1202,7 +1202,7 @@ func setDefaults() { viper.SetDefault("linuxdo_connect.redirect_url", "") viper.SetDefault("linuxdo_connect.frontend_redirect_url", "/auth/linuxdo/callback") viper.SetDefault("linuxdo_connect.token_auth_method", "client_secret_post") - viper.SetDefault("linuxdo_connect.use_pkce", true) + viper.SetDefault("linuxdo_connect.use_pkce", false) viper.SetDefault("linuxdo_connect.userinfo_email_path", "") viper.SetDefault("linuxdo_connect.userinfo_id_path", "") viper.SetDefault("linuxdo_connect.userinfo_username_path", "") @@ -1222,8 +1222,8 @@ func setDefaults() { viper.SetDefault("oidc_connect.redirect_url", "") viper.SetDefault("oidc_connect.frontend_redirect_url", "/auth/oidc/callback") viper.SetDefault("oidc_connect.token_auth_method", "client_secret_post") - viper.SetDefault("oidc_connect.use_pkce", true) - viper.SetDefault("oidc_connect.validate_id_token", true) + viper.SetDefault("oidc_connect.use_pkce", false) + viper.SetDefault("oidc_connect.validate_id_token", false) viper.SetDefault("oidc_connect.allowed_signing_algs", "RS256,ES256,PS256") viper.SetDefault("oidc_connect.clock_skew_seconds", 120) viper.SetDefault("oidc_connect.require_email_verified", false) @@ -1613,9 +1613,6 @@ func (c *Config) Validate() error { return fmt.Errorf("security.csp.policy is required when CSP is enabled") } if c.LinuxDo.Enabled { - if !c.LinuxDo.UsePKCE { - return fmt.Errorf("linuxdo_connect.use_pkce must be true when linuxdo_connect.enabled=true") - } if strings.TrimSpace(c.LinuxDo.ClientID) == "" { return fmt.Errorf("linuxdo_connect.client_id is required when linuxdo_connect.enabled=true") } @@ -1668,12 +1665,6 @@ func (c *Config) Validate() error { warnIfInsecureURL("linuxdo_connect.frontend_redirect_url", c.LinuxDo.FrontendRedirectURL) } if c.OIDC.Enabled { - if !c.OIDC.UsePKCE { - return fmt.Errorf("oidc_connect.use_pkce must be true when oidc_connect.enabled=true") - } - if !c.OIDC.ValidateIDToken { - return fmt.Errorf("oidc_connect.validate_id_token must be true when oidc_connect.enabled=true") - } if strings.TrimSpace(c.OIDC.ClientID) == "" { return fmt.Errorf("oidc_connect.client_id is required when oidc_connect.enabled=true") } diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index fe48541b..f40a5f57 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -346,7 +346,7 @@ func TestValidateLinuxDoFrontendRedirectURL(t *testing.T) { } } -func TestValidateLinuxDoPKCERequiredForPublicClient(t *testing.T) { +func TestValidateLinuxDoAllowsDisablingPKCEForCompatibility(t *testing.T) { resetViperWithJWTSecret(t) cfg, err := Load() @@ -363,11 +363,8 @@ func TestValidateLinuxDoPKCERequiredForPublicClient(t *testing.T) { cfg.LinuxDo.UsePKCE = false err = cfg.Validate() - if err == nil { - t.Fatalf("Validate() expected error when token_auth_method=none and use_pkce=false, got nil") - } - if !strings.Contains(err.Error(), "linuxdo_connect.use_pkce") { - t.Fatalf("Validate() expected use_pkce error, got: %v", err) + if err != nil { + t.Fatalf("Validate() expected LinuxDo config without PKCE to pass for compatibility, got: %v", err) } } @@ -427,6 +424,35 @@ func TestValidateOIDCAllowsIssuerOnlyEndpointsWithDiscoveryFallback(t *testing.T } } +func TestValidateOIDCAllowsDisablingPKCEAndIDTokenValidation(t *testing.T) { + resetViperWithJWTSecret(t) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error: %v", err) + } + + cfg.OIDC.Enabled = true + cfg.OIDC.ClientID = "oidc-client" + cfg.OIDC.ClientSecret = "oidc-secret" + cfg.OIDC.IssuerURL = "https://issuer.example.com" + cfg.OIDC.AuthorizeURL = "https://issuer.example.com/auth" + cfg.OIDC.TokenURL = "https://issuer.example.com/token" + cfg.OIDC.UserInfoURL = "https://issuer.example.com/userinfo" + cfg.OIDC.RedirectURL = "https://example.com/api/v1/auth/oauth/oidc/callback" + cfg.OIDC.FrontendRedirectURL = "/auth/oidc/callback" + cfg.OIDC.Scopes = "openid email profile" + cfg.OIDC.UsePKCE = false + cfg.OIDC.ValidateIDToken = false + cfg.OIDC.JWKSURL = "" + cfg.OIDC.AllowedSigningAlgs = "" + + err = cfg.Validate() + if err != nil { + t.Fatalf("Validate() expected OIDC config without PKCE/id_token validation to pass for compatibility, got: %v", err) + } +} + func TestLoadDefaultDashboardCacheConfig(t *testing.T) { resetViperWithJWTSecret(t) diff --git a/backend/internal/handler/admin/setting_handler.go b/backend/internal/handler/admin/setting_handler.go index e6609c97..f85f199b 100644 --- a/backend/internal/handler/admin/setting_handler.go +++ b/backend/internal/handler/admin/setting_handler.go @@ -653,20 +653,22 @@ func (h *SettingHandler) UpdateSettings(c *gin.Context) { req.WeChatConnectScopes = service.DefaultWeChatConnectScopesForMode(req.WeChatConnectMode) } } - if req.WeChatConnectRedirectURL == "" { - response.BadRequest(c, "WeChat Redirect URL is required when enabled") - return - } - if err := config.ValidateAbsoluteHTTPURL(req.WeChatConnectRedirectURL); err != nil { - response.BadRequest(c, "WeChat Redirect URL must be an absolute http(s) URL") - return - } - if req.WeChatConnectFrontendRedirectURL == "" { - req.WeChatConnectFrontendRedirectURL = "/auth/wechat/callback" - } - if err := config.ValidateFrontendRedirectURL(req.WeChatConnectFrontendRedirectURL); err != nil { - response.BadRequest(c, "WeChat Frontend Redirect URL is invalid") - return + if req.WeChatConnectOpenEnabled || req.WeChatConnectMPEnabled { + if req.WeChatConnectRedirectURL == "" { + response.BadRequest(c, "WeChat Redirect URL is required when web oauth is enabled") + return + } + if err := config.ValidateAbsoluteHTTPURL(req.WeChatConnectRedirectURL); err != nil { + response.BadRequest(c, "WeChat Redirect URL must be an absolute http(s) URL") + return + } + if req.WeChatConnectFrontendRedirectURL == "" { + req.WeChatConnectFrontendRedirectURL = "/auth/wechat/callback" + } + if err := config.ValidateFrontendRedirectURL(req.WeChatConnectFrontendRedirectURL); err != nil { + response.BadRequest(c, "WeChat Frontend Redirect URL is invalid") + return + } } } @@ -749,14 +751,6 @@ func (h *SettingHandler) UpdateSettings(c *gin.Context) { response.BadRequest(c, "OIDC scopes must contain openid") return } - if !req.OIDCConnectUsePKCE { - response.BadRequest(c, "OIDC PKCE must be enabled") - return - } - if !req.OIDCConnectValidateIDToken { - response.BadRequest(c, "OIDC ID Token validation must be enabled") - return - } switch req.OIDCConnectTokenAuthMethod { case "", "client_secret_post", "client_secret_basic", "none": default: @@ -767,7 +761,7 @@ func (h *SettingHandler) UpdateSettings(c *gin.Context) { response.BadRequest(c, "OIDC clock skew seconds must be between 0 and 600") return } - if req.OIDCConnectAllowedSigningAlgs == "" { + if req.OIDCConnectValidateIDToken && req.OIDCConnectAllowedSigningAlgs == "" { response.BadRequest(c, "OIDC Allowed Signing Algs is required when validate_id_token=true") return } diff --git a/backend/internal/handler/auth_linuxdo_oauth.go b/backend/internal/handler/auth_linuxdo_oauth.go index 2bd44e78..ef9a5bca 100644 --- a/backend/internal/handler/auth_linuxdo_oauth.go +++ b/backend/internal/handler/auth_linuxdo_oauth.go @@ -123,13 +123,16 @@ func (h *AuthHandler) LinuxDoOAuthStart(c *gin.Context) { clearCookie(c, linuxDoOAuthBindUserCookieName, secureCookie) } - verifier, err := oauth.GenerateCodeVerifier() - if err != nil { - response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_PKCE_GEN_FAILED", "failed to generate pkce verifier").WithCause(err)) - return + codeChallenge := "" + if cfg.UsePKCE { + verifier, err := oauth.GenerateCodeVerifier() + if err != nil { + response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_PKCE_GEN_FAILED", "failed to generate pkce verifier").WithCause(err)) + return + } + codeChallenge = oauth.GenerateCodeChallenge(verifier) + setCookie(c, linuxDoOAuthVerifierCookie, encodeCookieValue(verifier), linuxDoOAuthCookieMaxAgeSec, secureCookie) } - codeChallenge := oauth.GenerateCodeChallenge(verifier) - setCookie(c, linuxDoOAuthVerifierCookie, encodeCookieValue(verifier), linuxDoOAuthCookieMaxAgeSec, secureCookie) redirectURI := strings.TrimSpace(cfg.RedirectURL) if redirectURI == "" { @@ -200,10 +203,13 @@ func (h *AuthHandler) LinuxDoOAuthCallback(c *gin.Context) { intent, _ := readCookieDecoded(c, linuxDoOAuthIntentCookieName) intent = normalizeOAuthIntent(intent) - codeVerifier, _ := readCookieDecoded(c, linuxDoOAuthVerifierCookie) - if codeVerifier == "" { - redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "") - return + codeVerifier := "" + if cfg.UsePKCE { + codeVerifier, _ = readCookieDecoded(c, linuxDoOAuthVerifierCookie) + if codeVerifier == "" { + redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "") + return + } } redirectURI := strings.TrimSpace(cfg.RedirectURL) @@ -292,25 +298,16 @@ func (h *AuthHandler) LinuxDoOAuthCallback(c *gin.Context) { return } if existingIdentityUser != nil { - tokenPair, user, err := h.authService.LoginOrRegisterOAuthWithTokenPair(c.Request.Context(), existingIdentityUser.Email, username, "") - if err != nil { - redirectOAuthError(c, frontendCallback, "login_failed", infraerrors.Reason(err), infraerrors.Message(err)) - return - } if err := h.createOAuthPendingSession(c, oauthPendingSessionPayload{ Intent: oauthIntentLogin, Identity: identityKey, - TargetUserID: &user.ID, + TargetUserID: &existingIdentityUser.ID, ResolvedEmail: existingIdentityUser.Email, RedirectTo: redirectTo, BrowserSessionKey: browserSessionKey, UpstreamIdentityClaims: upstreamClaims, CompletionResponse: map[string]any{ - "access_token": tokenPair.AccessToken, - "refresh_token": tokenPair.RefreshToken, - "expires_in": tokenPair.ExpiresIn, - "token_type": "Bearer", - "redirect": redirectTo, + "redirect": redirectTo, }, }); err != nil { redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "") @@ -546,7 +543,9 @@ func linuxDoExchangeCode( form.Set("client_id", cfg.ClientID) form.Set("code", code) form.Set("redirect_uri", redirectURI) - form.Set("code_verifier", codeVerifier) + if strings.TrimSpace(codeVerifier) != "" { + form.Set("code_verifier", codeVerifier) + } r := client.R(). SetContext(ctx). @@ -699,8 +698,10 @@ func buildLinuxDoAuthorizeURL(cfg config.LinuxDoConnectConfig, state string, cod q.Set("scope", cfg.Scopes) } q.Set("state", state) - q.Set("code_challenge", codeChallenge) - q.Set("code_challenge_method", "S256") + if strings.TrimSpace(codeChallenge) != "" { + q.Set("code_challenge", codeChallenge) + q.Set("code_challenge_method", "S256") + } u.RawQuery = q.Encode() return u.String(), nil diff --git a/backend/internal/handler/auth_linuxdo_oauth_test.go b/backend/internal/handler/auth_linuxdo_oauth_test.go index a3d87dfb..841dc442 100644 --- a/backend/internal/handler/auth_linuxdo_oauth_test.go +++ b/backend/internal/handler/auth_linuxdo_oauth_test.go @@ -171,6 +171,80 @@ func TestLinuxDoOAuthBindStartRedirectsAndSetsBindCookies(t *testing.T) { require.Equal(t, int64(42), userID) } +func TestLinuxDoOAuthStartOmitsPKCEWhenDisabled(t *testing.T) { + handler := newLinuxDoOAuthTestHandler(t, false, config.LinuxDoConnectConfig{ + Enabled: true, + ClientID: "linuxdo-client", + ClientSecret: "linuxdo-secret", + AuthorizeURL: "https://connect.linux.do/oauth/authorize", + TokenURL: "https://connect.linux.do/oauth/token", + UserInfoURL: "https://connect.linux.do/api/user", + Scopes: "read", + RedirectURL: "https://api.example.com/api/v1/auth/oauth/linuxdo/callback", + FrontendRedirectURL: "/auth/linuxdo/callback", + TokenAuthMethod: "client_secret_post", + UsePKCE: false, + }) + + recorder := httptest.NewRecorder() + c, _ := gin.CreateTestContext(recorder) + c.Request = httptest.NewRequest(http.MethodGet, "/api/v1/auth/oauth/linuxdo/start?redirect=/dashboard", nil) + + handler.LinuxDoOAuthStart(c) + + require.Equal(t, http.StatusFound, recorder.Code) + require.NotContains(t, recorder.Header().Get("Location"), "code_challenge=") + require.Nil(t, findCookie(recorder.Result().Cookies(), linuxDoOAuthVerifierCookie)) +} + +func TestLinuxDoOAuthCallbackAllowsMissingVerifierWhenPKCEDisabled(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/token": + require.NoError(t, r.ParseForm()) + require.Empty(t, r.PostForm.Get("code_verifier")) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"linuxdo-access","token_type":"Bearer","expires_in":3600}`)) + case "/userinfo": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"compat-subject","username":"linuxdo_user","name":"LinuxDo Display"}`)) + default: + http.NotFound(w, r) + } + })) + defer upstream.Close() + + handler, client := newLinuxDoOAuthHandlerAndClient(t, false, config.LinuxDoConnectConfig{ + Enabled: true, + ClientID: "linuxdo-client", + ClientSecret: "linuxdo-secret", + AuthorizeURL: upstream.URL + "/authorize", + TokenURL: upstream.URL + "/token", + UserInfoURL: upstream.URL + "/userinfo", + Scopes: "read", + RedirectURL: "https://api.example.com/api/v1/auth/oauth/linuxdo/callback", + FrontendRedirectURL: "/auth/linuxdo/callback", + TokenAuthMethod: "client_secret_post", + UsePKCE: false, + }) + t.Cleanup(func() { _ = client.Close() }) + + recorder := httptest.NewRecorder() + c, _ := gin.CreateTestContext(recorder) + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/oauth/linuxdo/callback?code=linuxdo-code&state=state-123", nil) + req.AddCookie(encodedCookie(linuxDoOAuthStateCookieName, "state-123")) + req.AddCookie(encodedCookie(linuxDoOAuthRedirectCookie, "/dashboard")) + req.AddCookie(encodedCookie(linuxDoOAuthIntentCookieName, oauthIntentLogin)) + req.AddCookie(encodedCookie(oauthPendingBrowserCookieName, "browser-123")) + c.Request = req + + handler.LinuxDoOAuthCallback(c) + + require.Equal(t, http.StatusFound, recorder.Code) + require.Equal(t, "/auth/linuxdo/callback", recorder.Header().Get("Location")) + require.NotNil(t, findCookie(recorder.Result().Cookies(), oauthPendingSessionCookieName)) +} + func TestLinuxDoOAuthBindStartAcceptsAccessTokenCookie(t *testing.T) { handler, client := newLinuxDoOAuthHandlerAndClient(t, false, config.LinuxDoConnectConfig{ Enabled: true, @@ -327,7 +401,10 @@ func TestLinuxDoOAuthCallbackCreatesLoginPendingSessionForExistingIdentityUser(t completion, ok := session.LocalFlowState[oauthCompletionResponseKey].(map[string]any) require.True(t, ok) require.Equal(t, "/dashboard", completion["redirect"]) - require.NotEmpty(t, completion["access_token"]) + _, hasAccessToken := completion["access_token"] + require.False(t, hasAccessToken) + _, hasRefreshToken := completion["refresh_token"] + require.False(t, hasRefreshToken) require.Nil(t, completion["error"]) } diff --git a/backend/internal/handler/auth_oidc_oauth.go b/backend/internal/handler/auth_oidc_oauth.go index d2042a87..7fe4b8d9 100644 --- a/backend/internal/handler/auth_oidc_oauth.go +++ b/backend/internal/handler/auth_oidc_oauth.go @@ -157,21 +157,25 @@ func (h *AuthHandler) OIDCOAuthStart(c *gin.Context) { } codeChallenge := "" - verifier, genErr := oauth.GenerateCodeVerifier() - if genErr != nil { - response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_PKCE_GEN_FAILED", "failed to generate pkce verifier").WithCause(genErr)) - return + if cfg.UsePKCE { + verifier, genErr := oauth.GenerateCodeVerifier() + if genErr != nil { + response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_PKCE_GEN_FAILED", "failed to generate pkce verifier").WithCause(genErr)) + return + } + codeChallenge = oauth.GenerateCodeChallenge(verifier) + oidcSetCookie(c, oidcOAuthVerifierCookie, encodeCookieValue(verifier), oidcOAuthCookieMaxAgeSec, secureCookie) } - codeChallenge = oauth.GenerateCodeChallenge(verifier) - oidcSetCookie(c, oidcOAuthVerifierCookie, encodeCookieValue(verifier), oidcOAuthCookieMaxAgeSec, secureCookie) nonce := "" - nonce, err = oauth.GenerateState() - if err != nil { - response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_NONCE_GEN_FAILED", "failed to generate oauth nonce").WithCause(err)) - return + if cfg.ValidateIDToken { + nonce, err = oauth.GenerateState() + if err != nil { + response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_NONCE_GEN_FAILED", "failed to generate oauth nonce").WithCause(err)) + return + } + oidcSetCookie(c, oidcOAuthNonceCookie, encodeCookieValue(nonce), oidcOAuthCookieMaxAgeSec, secureCookie) } - oidcSetCookie(c, oidcOAuthNonceCookie, encodeCookieValue(nonce), oidcOAuthCookieMaxAgeSec, secureCookie) redirectURI := strings.TrimSpace(cfg.RedirectURL) if redirectURI == "" { @@ -244,17 +248,21 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { intent = normalizeOAuthIntent(intent) codeVerifier := "" - codeVerifier, _ = readCookieDecoded(c, oidcOAuthVerifierCookie) - if codeVerifier == "" { - redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "") - return + if cfg.UsePKCE { + codeVerifier, _ = readCookieDecoded(c, oidcOAuthVerifierCookie) + if codeVerifier == "" { + redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "") + return + } } expectedNonce := "" - expectedNonce, _ = readCookieDecoded(c, oidcOAuthNonceCookie) - if expectedNonce == "" { - redirectOAuthError(c, frontendCallback, "missing_nonce", "missing oauth nonce", "") - return + if cfg.ValidateIDToken { + expectedNonce, _ = readCookieDecoded(c, oidcOAuthNonceCookie) + if expectedNonce == "" { + redirectOAuthError(c, frontendCallback, "missing_nonce", "missing oauth nonce", "") + return + } } redirectURI := strings.TrimSpace(cfg.RedirectURL) @@ -284,16 +292,19 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { return } - if strings.TrimSpace(tokenResp.IDToken) == "" { - redirectOAuthError(c, frontendCallback, "missing_id_token", "missing id_token", "") - return - } + var idClaims *oidcIDTokenClaims + if cfg.ValidateIDToken { + if strings.TrimSpace(tokenResp.IDToken) == "" { + redirectOAuthError(c, frontendCallback, "missing_id_token", "missing id_token", "") + return + } - idClaims, err := oidcParseAndValidateIDToken(c.Request.Context(), cfg, tokenResp.IDToken, expectedNonce) - if err != nil { - log.Printf("[OIDC OAuth] id_token validation failed: %v", err) - redirectOAuthError(c, frontendCallback, "invalid_id_token", "failed to validate id_token", "") - return + idClaims, err = oidcParseAndValidateIDToken(c.Request.Context(), cfg, tokenResp.IDToken, expectedNonce) + if err != nil { + log.Printf("[OIDC OAuth] id_token validation failed: %v", err) + redirectOAuthError(c, frontendCallback, "invalid_id_token", "failed to validate id_token", "") + return + } } userInfoClaims, err := oidcFetchUserInfo(c.Request.Context(), cfg, tokenResp) @@ -303,7 +314,10 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { return } - subject := strings.TrimSpace(idClaims.Subject) + subject := "" + if idClaims != nil { + subject = strings.TrimSpace(idClaims.Subject) + } if subject == "" { subject = strings.TrimSpace(userInfoClaims.Subject) } @@ -311,7 +325,10 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { redirectOAuthError(c, frontendCallback, "missing_subject", "missing subject claim", "") return } - issuer := strings.TrimSpace(idClaims.Issuer) + issuer := "" + if idClaims != nil { + issuer = strings.TrimSpace(idClaims.Issuer) + } if issuer == "" { issuer = strings.TrimSpace(cfg.IssuerURL) } @@ -321,21 +338,34 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { } emailVerified := userInfoClaims.EmailVerified - if emailVerified == nil { + if emailVerified == nil && idClaims != nil { emailVerified = idClaims.EmailVerified } - if userInfoClaims.Subject != "" && idClaims.Subject != "" && strings.TrimSpace(userInfoClaims.Subject) != strings.TrimSpace(idClaims.Subject) { + if idClaims != nil && userInfoClaims.Subject != "" && idClaims.Subject != "" && strings.TrimSpace(userInfoClaims.Subject) != strings.TrimSpace(idClaims.Subject) { redirectOAuthError(c, frontendCallback, "subject_mismatch", "userinfo subject does not match id_token", "") return } identityKey := oidcIdentityKey(issuer, subject) - compatEmail := strings.TrimSpace(firstNonEmpty(userInfoClaims.Email, idClaims.Email)) + compatEmail := strings.TrimSpace(userInfoClaims.Email) + if compatEmail == "" && idClaims != nil { + compatEmail = strings.TrimSpace(idClaims.Email) + } email := oidcSyntheticEmailFromIdentityKey(identityKey) username := firstNonEmpty( userInfoClaims.Username, - idClaims.PreferredUsername, - idClaims.Name, + func() string { + if idClaims != nil { + return idClaims.PreferredUsername + } + return "" + }(), + func() string { + if idClaims != nil { + return idClaims.Name + } + return "" + }(), oidcFallbackUsername(subject), ) identityRef := service.PendingAuthIdentityKey{ @@ -350,7 +380,12 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { "issuer": issuer, "email_verified": emailVerified != nil && *emailVerified, "provider_fallback": strings.TrimSpace(cfg.ProviderName), - "suggested_display_name": firstNonEmpty(userInfoClaims.DisplayName, idClaims.Name, username), + "suggested_display_name": firstNonEmpty(userInfoClaims.DisplayName, func() string { + if idClaims != nil { + return idClaims.Name + } + return "" + }(), username), "suggested_avatar_url": userInfoClaims.AvatarURL, } if compatEmail != "" && !strings.EqualFold(strings.TrimSpace(compatEmail), strings.TrimSpace(email)) { @@ -387,25 +422,16 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { return } if existingIdentityUser != nil { - tokenPair, user, err := h.authService.LoginOrRegisterOAuthWithTokenPair(c.Request.Context(), existingIdentityUser.Email, username, "") - if err != nil { - redirectOAuthError(c, frontendCallback, "login_failed", infraerrors.Reason(err), infraerrors.Message(err)) - return - } if err := h.createOAuthPendingSession(c, oauthPendingSessionPayload{ Intent: oauthIntentLogin, Identity: identityRef, - TargetUserID: &user.ID, + TargetUserID: &existingIdentityUser.ID, ResolvedEmail: existingIdentityUser.Email, RedirectTo: redirectTo, BrowserSessionKey: browserSessionKey, UpstreamIdentityClaims: upstreamClaims, CompletionResponse: map[string]any{ - "access_token": tokenPair.AccessToken, - "refresh_token": tokenPair.RefreshToken, - "expires_in": tokenPair.ExpiresIn, - "token_type": "Bearer", - "redirect": redirectTo, + "redirect": redirectTo, }, }); err != nil { redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "") @@ -670,7 +696,9 @@ func oidcExchangeCode( form.Set("client_id", cfg.ClientID) form.Set("code", code) form.Set("redirect_uri", redirectURI) - form.Set("code_verifier", codeVerifier) + if strings.TrimSpace(codeVerifier) != "" { + form.Set("code_verifier", codeVerifier) + } r := client.R(). SetContext(ctx). @@ -872,9 +900,13 @@ func buildOIDCAuthorizeURL(cfg config.OIDCConnectConfig, state, nonce, codeChall q.Set("scope", cfg.Scopes) } q.Set("state", state) - q.Set("nonce", nonce) - q.Set("code_challenge", codeChallenge) - q.Set("code_challenge_method", "S256") + if strings.TrimSpace(nonce) != "" { + q.Set("nonce", nonce) + } + if strings.TrimSpace(codeChallenge) != "" { + q.Set("code_challenge", codeChallenge) + q.Set("code_challenge_method", "S256") + } u.RawQuery = q.Encode() return u.String(), nil diff --git a/backend/internal/handler/auth_oidc_oauth_test.go b/backend/internal/handler/auth_oidc_oauth_test.go index 2acca18a..a600fd56 100644 --- a/backend/internal/handler/auth_oidc_oauth_test.go +++ b/backend/internal/handler/auth_oidc_oauth_test.go @@ -186,6 +186,89 @@ func TestOIDCOAuthBindStartRedirectsAndSetsBindCookies(t *testing.T) { require.Equal(t, int64(84), userID) } +func TestOIDCOAuthStartOmitsPKCEAndNonceWhenDisabled(t *testing.T) { + handler := newOIDCOAuthTestHandler(t, false, config.OIDCConnectConfig{ + Enabled: true, + ClientID: "oidc-client", + ClientSecret: "oidc-secret", + IssuerURL: "https://issuer.example.com", + AuthorizeURL: "https://issuer.example.com/oauth/authorize", + TokenURL: "https://issuer.example.com/oauth/token", + UserInfoURL: "https://issuer.example.com/oauth/userinfo", + Scopes: "openid profile email", + RedirectURL: "https://api.example.com/api/v1/auth/oauth/oidc/callback", + FrontendRedirectURL: "/auth/oidc/callback", + TokenAuthMethod: "client_secret_post", + UsePKCE: false, + ValidateIDToken: false, + RequireEmailVerified: false, + }) + + recorder := httptest.NewRecorder() + c, _ := gin.CreateTestContext(recorder) + c.Request = httptest.NewRequest(http.MethodGet, "/api/v1/auth/oauth/oidc/start?redirect=/dashboard", nil) + + handler.OIDCOAuthStart(c) + + require.Equal(t, http.StatusFound, recorder.Code) + location := recorder.Header().Get("Location") + require.NotContains(t, location, "code_challenge=") + require.NotContains(t, location, "nonce=") + require.Nil(t, findCookie(recorder.Result().Cookies(), oidcOAuthVerifierCookie)) + require.Nil(t, findCookie(recorder.Result().Cookies(), oidcOAuthNonceCookie)) +} + +func TestOIDCOAuthCallbackAllowsOptionalPKCEAndIDTokenValidation(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/token": + require.NoError(t, r.ParseForm()) + require.Empty(t, r.PostForm.Get("code_verifier")) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"oidc-access","token_type":"Bearer","expires_in":3600}`)) + case "/userinfo": + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"sub":"oidc-subject-compat","preferred_username":"oidc_user","name":"OIDC Display","email":"oidc@example.com"}`)) + default: + http.NotFound(w, r) + } + })) + defer upstream.Close() + + handler, client := newOIDCOAuthHandlerAndClient(t, false, config.OIDCConnectConfig{ + Enabled: true, + ClientID: "oidc-client", + ClientSecret: "oidc-secret", + IssuerURL: "https://issuer.example.com", + AuthorizeURL: upstream.URL + "/authorize", + TokenURL: upstream.URL + "/token", + UserInfoURL: upstream.URL + "/userinfo", + Scopes: "openid profile email", + RedirectURL: "https://api.example.com/api/v1/auth/oauth/oidc/callback", + FrontendRedirectURL: "/auth/oidc/callback", + TokenAuthMethod: "client_secret_post", + UsePKCE: false, + ValidateIDToken: false, + RequireEmailVerified: false, + }) + 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-123", nil) + req.AddCookie(encodedCookie(oidcOAuthStateCookieName, "state-123")) + req.AddCookie(encodedCookie(oidcOAuthRedirectCookie, "/dashboard")) + req.AddCookie(encodedCookie(oidcOAuthIntentCookieName, oauthIntentLogin)) + req.AddCookie(encodedCookie(oauthPendingBrowserCookieName, "browser-123")) + c.Request = req + + handler.OIDCOAuthCallback(c) + + require.Equal(t, http.StatusFound, recorder.Code) + require.Equal(t, "/auth/oidc/callback", recorder.Header().Get("Location")) + require.NotNil(t, findCookie(recorder.Result().Cookies(), oauthPendingSessionCookieName)) +} + func TestOIDCOAuthCallbackCreatesLoginPendingSessionForExistingIdentityUser(t *testing.T) { cfg, cleanup := newOIDCTestProvider(t, oidcProviderFixture{ Subject: "oidc-subject-login", @@ -250,7 +333,10 @@ func TestOIDCOAuthCallbackCreatesLoginPendingSessionForExistingIdentityUser(t *t completion, ok := session.LocalFlowState[oauthCompletionResponseKey].(map[string]any) require.True(t, ok) require.Equal(t, "/dashboard", completion["redirect"]) - require.NotEmpty(t, completion["access_token"]) + _, hasAccessToken := completion["access_token"] + require.False(t, hasAccessToken) + _, hasRefreshToken := completion["refresh_token"] + require.False(t, hasRefreshToken) require.Nil(t, completion["error"]) } diff --git a/backend/internal/handler/auth_wechat_oauth.go b/backend/internal/handler/auth_wechat_oauth.go index 78f5d7c2..39703ce7 100644 --- a/backend/internal/handler/auth_wechat_oauth.go +++ b/backend/internal/handler/auth_wechat_oauth.go @@ -279,12 +279,7 @@ func (h *AuthHandler) WeChatOAuthCallback(c *gin.Context) { redirectOAuthError(c, frontendCallback, "session_error", infraerrors.Reason(err), infraerrors.Message(err)) return } - tokenPair, user, err := h.authService.LoginOrRegisterOAuthWithTokenPair(c.Request.Context(), existingIdentityUser.Email, username, "") - if err != nil { - redirectOAuthError(c, frontendCallback, "login_failed", infraerrors.Reason(err), infraerrors.Message(err)) - return - } - if err := h.createWeChatPendingSession(c, normalizedIntent, providerSubject, existingIdentityUser.Email, redirectTo, browserSessionKey, upstreamClaims, tokenPair, nil, &user.ID); err != nil { + if err := h.createWeChatPendingSession(c, normalizedIntent, providerSubject, existingIdentityUser.Email, redirectTo, browserSessionKey, upstreamClaims, nil, nil, &existingIdentityUser.ID); err != nil { redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "") return } diff --git a/backend/internal/handler/auth_wechat_oauth_test.go b/backend/internal/handler/auth_wechat_oauth_test.go index 937daa6d..99006701 100644 --- a/backend/internal/handler/auth_wechat_oauth_test.go +++ b/backend/internal/handler/auth_wechat_oauth_test.go @@ -213,6 +213,86 @@ func TestWeChatOAuthCallbackFallsBackToOpenIDWhenUnionIDMissingInSingleChannelMo require.Equal(t, "third_party_signup", completion["choice_reason"]) } +func TestWeChatOAuthCallbackCreatesLoginPendingSessionForExistingIdentityUserWithoutStoredTokens(t *testing.T) { + originalAccessTokenURL := wechatOAuthAccessTokenURL + originalUserInfoURL := wechatOAuthUserInfoURL + t.Cleanup(func() { + wechatOAuthAccessTokenURL = originalAccessTokenURL + wechatOAuthUserInfoURL = originalUserInfoURL + }) + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/sns/oauth2/access_token"): + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"wechat-access","openid":"openid-123","unionid":"union-456","scope":"snsapi_login"}`)) + case strings.Contains(r.URL.Path, "/sns/userinfo"): + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"openid":"openid-123","unionid":"union-456","nickname":"WeChat Display","headimgurl":"https://cdn.example/wechat-login.png"}`)) + default: + http.NotFound(w, r) + } + })) + defer upstream.Close() + wechatOAuthAccessTokenURL = upstream.URL + "/sns/oauth2/access_token" + wechatOAuthUserInfoURL = upstream.URL + "/sns/userinfo" + + handler, client := newWeChatOAuthTestHandlerWithSettings(t, false, wechatOAuthTestSettings("open", "wx-open-app", "wx-open-secret", "https://app.example.com/auth/wechat/callback")) + defer client.Close() + + ctx := context.Background() + existingUser, err := client.User.Create(). + SetEmail(wechatSyntheticEmail("union-456")). + SetUsername("wechat-existing-user"). + SetPasswordHash("hash"). + SetRole(service.RoleUser). + SetStatus(service.StatusActive). + Save(ctx) + require.NoError(t, err) + _, err = client.AuthIdentity.Create(). + SetUserID(existingUser.ID). + SetProviderType("wechat"). + SetProviderKey(wechatOAuthProviderKey). + SetProviderSubject("union-456"). + SetMetadata(map[string]any{"username": "wechat-existing-user"}). + Save(ctx) + require.NoError(t, err) + + recorder := httptest.NewRecorder() + c, _ := gin.CreateTestContext(recorder) + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/oauth/wechat/callback?code=wechat-code&state=state-123", nil) + req.Host = "api.example.com" + req.AddCookie(encodedCookie(wechatOAuthStateCookieName, "state-123")) + req.AddCookie(encodedCookie(wechatOAuthRedirectCookieName, "/dashboard")) + req.AddCookie(encodedCookie(wechatOAuthModeCookieName, "open")) + req.AddCookie(encodedCookie(oauthPendingBrowserCookieName, "browser-123")) + c.Request = req + + handler.WeChatOAuthCallback(c) + + require.Equal(t, http.StatusFound, recorder.Code) + require.Equal(t, "https://app.example.com/auth/wechat/callback", recorder.Header().Get("Location")) + + sessionCookie := findCookie(recorder.Result().Cookies(), oauthPendingSessionCookieName) + require.NotNil(t, sessionCookie) + + session, err := client.PendingAuthSession.Query(). + Where(pendingauthsession.SessionTokenEQ(decodeCookieValueForTest(t, sessionCookie.Value))). + Only(ctx) + require.NoError(t, err) + require.Equal(t, oauthIntentLogin, session.Intent) + require.NotNil(t, session.TargetUserID) + require.Equal(t, existingUser.ID, *session.TargetUserID) + require.Equal(t, existingUser.Email, session.ResolvedEmail) + + completion := session.LocalFlowState[oauthCompletionResponseKey].(map[string]any) + require.Equal(t, "/dashboard", completion["redirect"]) + _, hasAccessToken := completion["access_token"] + require.False(t, hasAccessToken) + _, hasRefreshToken := completion["refresh_token"] + require.False(t, hasRefreshToken) +} + func TestWeChatPaymentOAuthCallbackRedirectsWithOpaqueResumeToken(t *testing.T) { originalAccessTokenURL := wechatOAuthAccessTokenURL t.Cleanup(func() { diff --git a/backend/internal/service/setting_service.go b/backend/internal/service/setting_service.go index fe566fec..059bbcd3 100644 --- a/backend/internal/service/setting_service.go +++ b/backend/internal/service/setting_service.go @@ -631,7 +631,7 @@ func (s *SettingService) weChatOAuthCapabilitiesFromSettings(settings map[string mpReady := mpEnabled && webRedirectReady && mpAppID != "" && mpAppSecret != "" mobileReady := mobileEnabled && mobileAppID != "" && mobileAppSecret != "" - return openReady || mpReady || mobileReady, openReady, mpReady, mobileReady + return openReady || mpReady, openReady, mpReady, mobileReady } // filterUserVisibleMenuItems filters out admin-only menu items from a raw JSON @@ -1693,8 +1693,6 @@ func (s *SettingService) parseSettings(settings map[string]string) *SystemSettin } else { result.OIDCConnectValidateIDToken = oidcBase.ValidateIDToken } - result.OIDCConnectUsePKCE = true - result.OIDCConnectValidateIDToken = true if v, ok := settings[SettingKeyOIDCConnectAllowedSigningAlgs]; ok && strings.TrimSpace(v) != "" { result.OIDCConnectAllowedSigningAlgs = strings.TrimSpace(v) } else { @@ -2196,8 +2194,6 @@ func (s *SettingService) GetLinuxDoConnectOAuthConfig(ctx context.Context) (conf if v, ok := settings[SettingKeyLinuxDoConnectRedirectURL]; ok && strings.TrimSpace(v) != "" { effective.RedirectURL = strings.TrimSpace(v) } - effective.UsePKCE = true - if !effective.Enabled { return config.LinuxDoConnectConfig{}, infraerrors.NotFound("OAUTH_DISABLED", "oauth login is disabled") } @@ -2421,8 +2417,6 @@ func (s *SettingService) GetOIDCConnectOAuthConfig(ctx context.Context) (config. if raw, ok := settings[SettingKeyOIDCConnectValidateIDToken]; ok { effective.ValidateIDToken = raw == "true" } - effective.UsePKCE = true - effective.ValidateIDToken = true if v, ok := settings[SettingKeyOIDCConnectAllowedSigningAlgs]; ok && strings.TrimSpace(v) != "" { effective.AllowedSigningAlgs = strings.TrimSpace(v) } diff --git a/backend/internal/service/setting_service_oidc_config_test.go b/backend/internal/service/setting_service_oidc_config_test.go index 3809b332..a5a3959a 100644 --- a/backend/internal/service/setting_service_oidc_config_test.go +++ b/backend/internal/service/setting_service_oidc_config_test.go @@ -101,3 +101,47 @@ func TestGetOIDCConnectOAuthConfig_ResolvesEndpointsFromIssuerDiscovery(t *testi require.Equal(t, srv.URL+"/issuer/protocol/openid-connect/userinfo", got.UserInfoURL) require.Equal(t, srv.URL+"/issuer/protocol/openid-connect/certs", got.JWKSURL) } + +func TestSettingService_ParseSettings_PreservesOptionalOIDCCompatibilityFlags(t *testing.T) { + svc := NewSettingService(&settingOIDCRepoStub{values: map[string]string{}}, &config.Config{}) + + got := svc.parseSettings(map[string]string{ + SettingKeyOIDCConnectEnabled: "true", + SettingKeyOIDCConnectUsePKCE: "false", + SettingKeyOIDCConnectValidateIDToken: "false", + }) + + require.False(t, got.OIDCConnectUsePKCE) + require.False(t, got.OIDCConnectValidateIDToken) +} + +func TestGetOIDCConnectOAuthConfig_AllowsCompatibilityFlagsToDisablePKCEAndIDTokenValidation(t *testing.T) { + cfg := &config.Config{ + OIDC: config.OIDCConnectConfig{ + Enabled: true, + ProviderName: "OIDC", + ClientID: "oidc-client", + ClientSecret: "oidc-secret", + IssuerURL: "https://issuer.example.com", + AuthorizeURL: "https://issuer.example.com/auth", + TokenURL: "https://issuer.example.com/token", + UserInfoURL: "https://issuer.example.com/userinfo", + RedirectURL: "https://example.com/api/v1/auth/oauth/oidc/callback", + FrontendRedirectURL: "/auth/oidc/callback", + Scopes: "openid email profile", + TokenAuthMethod: "client_secret_post", + }, + } + + repo := &settingOIDCRepoStub{values: map[string]string{ + SettingKeyOIDCConnectEnabled: "true", + SettingKeyOIDCConnectUsePKCE: "false", + SettingKeyOIDCConnectValidateIDToken: "false", + }} + svc := NewSettingService(repo, cfg) + + got, err := svc.GetOIDCConnectOAuthConfig(context.Background()) + require.NoError(t, err) + require.False(t, got.UsePKCE) + require.False(t, got.ValidateIDToken) +} diff --git a/backend/internal/service/setting_service_public_test.go b/backend/internal/service/setting_service_public_test.go index 497d1e36..4c7ca14b 100644 --- a/backend/internal/service/setting_service_public_test.go +++ b/backend/internal/service/setting_service_public_test.go @@ -112,3 +112,23 @@ func TestSettingService_GetPublicSettings_ExposesWeChatOAuthModeCapabilities(t * require.True(t, settings.WeChatOAuthOpenEnabled) require.True(t, settings.WeChatOAuthMPEnabled) } + +func TestSettingService_GetPublicSettings_DoesNotExposeMobileOnlyWeChatAsWebOAuthAvailable(t *testing.T) { + svc := NewSettingService(&settingPublicRepoStub{ + values: map[string]string{ + SettingKeyWeChatConnectEnabled: "true", + SettingKeyWeChatConnectMobileEnabled: "true", + SettingKeyWeChatConnectMode: "mobile", + SettingKeyWeChatConnectMobileAppID: "wx-mobile-app", + SettingKeyWeChatConnectMobileAppSecret: "wx-mobile-secret", + SettingKeyWeChatConnectFrontendRedirectURL: "/auth/wechat/callback", + }, + }, &config.Config{}) + + settings, err := svc.GetPublicSettings(context.Background()) + require.NoError(t, err) + require.False(t, settings.WeChatOAuthEnabled) + require.False(t, settings.WeChatOAuthOpenEnabled) + require.False(t, settings.WeChatOAuthMPEnabled) + require.True(t, settings.WeChatOAuthMobileEnabled) +} diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index bc444af5..c16d810b 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -248,12 +248,59 @@ func (s *UserService) GetProfileIdentitySummaries(ctx context.Context, userID in return UserIdentitySummarySet{}, err } - return UserIdentitySummarySet{ + summaries := UserIdentitySummarySet{ Email: s.buildEmailIdentitySummary(user, records), LinuxDo: s.buildProviderIdentitySummary("linuxdo", user, records), OIDC: s.buildProviderIdentitySummary("oidc", user, records), WeChat: s.buildProviderIdentitySummary("wechat", user, records), - }, nil + } + + s.applyExplicitProviderAvailability(ctx, &summaries) + return summaries, nil +} + +func (s *UserService) applyExplicitProviderAvailability(ctx context.Context, summaries *UserIdentitySummarySet) { + if s == nil || summaries == nil || s.settingRepo == nil { + return + } + + settings, err := s.settingRepo.GetMultiple(ctx, []string{ + SettingKeyLinuxDoConnectEnabled, + SettingKeyOIDCConnectEnabled, + SettingKeyWeChatConnectEnabled, + SettingKeyWeChatConnectOpenEnabled, + SettingKeyWeChatConnectMPEnabled, + SettingKeyWeChatConnectMobileEnabled, + SettingKeyWeChatConnectMode, + }) + if err != nil { + return + } + + if raw, ok := settings[SettingKeyLinuxDoConnectEnabled]; ok && strings.TrimSpace(raw) != "" && raw != "true" { + disableIdentityBindAction(&summaries.LinuxDo) + } + if raw, ok := settings[SettingKeyOIDCConnectEnabled]; ok && strings.TrimSpace(raw) != "" && raw != "true" { + disableIdentityBindAction(&summaries.OIDC) + } + if raw, ok := settings[SettingKeyWeChatConnectEnabled]; ok && strings.TrimSpace(raw) != "" { + if raw != "true" { + disableIdentityBindAction(&summaries.WeChat) + return + } + openEnabled, mpEnabled, _ := parseWeChatConnectCapabilitySettings(settings, true, settings[SettingKeyWeChatConnectMode]) + if !openEnabled && !mpEnabled { + disableIdentityBindAction(&summaries.WeChat) + } + } +} + +func disableIdentityBindAction(summary *UserIdentitySummary) { + if summary == nil || summary.Bound { + return + } + summary.CanBind = false + summary.BindStartPath = "" } func (s *UserService) PrepareIdentityBindingStart(_ context.Context, req StartUserIdentityBindingRequest) (*StartUserIdentityBindingResult, error) { diff --git a/backend/internal/service/user_service_test.go b/backend/internal/service/user_service_test.go index 88bb1637..109d459d 100644 --- a/backend/internal/service/user_service_test.go +++ b/backend/internal/service/user_service_test.go @@ -51,6 +51,44 @@ type mockUserRepoTxState struct { deleteAvatarIDs []int64 } +type mockUserSettingRepo struct { + values map[string]string +} + +func (m *mockUserSettingRepo) Get(context.Context, string) (*Setting, error) { + panic("unexpected Get call") +} + +func (m *mockUserSettingRepo) GetValue(context.Context, string) (string, error) { + panic("unexpected GetValue call") +} + +func (m *mockUserSettingRepo) Set(context.Context, string, string) error { + panic("unexpected Set call") +} + +func (m *mockUserSettingRepo) GetMultiple(_ context.Context, keys []string) (map[string]string, error) { + out := make(map[string]string, len(keys)) + for _, key := range keys { + if value, ok := m.values[key]; ok { + out[key] = value + } + } + return out, nil +} + +func (m *mockUserSettingRepo) SetMultiple(context.Context, map[string]string) error { + panic("unexpected SetMultiple call") +} + +func (m *mockUserSettingRepo) GetAll(context.Context) (map[string]string, error) { + panic("unexpected GetAll call") +} + +func (m *mockUserSettingRepo) Delete(context.Context, string) error { + panic("unexpected Delete call") +} + func (m *mockUserRepo) Create(context.Context, *User) error { return nil } func (m *mockUserRepo) GetByID(ctx context.Context, _ int64) (*User, error) { if m.getByIDErr != nil { @@ -382,6 +420,35 @@ func TestUnbindUserAuthProviderRemovesProviderAndReturnsUpdatedProfile(t *testin require.True(t, summaries.LinuxDo.CanBind) } +func TestGetProfileIdentitySummaries_HidesBindActionWhenProviderExplicitlyDisabled(t *testing.T) { + repo := &mockUserRepo{ + getByIDUser: &User{ + ID: 15, + Email: "alice@example.com", + }, + identities: []UserAuthIdentityRecord{ + { + ProviderType: "email", + ProviderKey: "email", + ProviderSubject: "alice@example.com", + }, + }, + } + settingRepo := &mockUserSettingRepo{ + values: map[string]string{ + SettingKeyLinuxDoConnectEnabled: "false", + }, + } + svc := NewUserService(repo, settingRepo, nil, nil) + + summaries, err := svc.GetProfileIdentitySummaries(context.Background(), 15, repo.getByIDUser) + + require.NoError(t, err) + require.False(t, summaries.LinuxDo.Bound) + require.False(t, summaries.LinuxDo.CanBind) + require.Empty(t, summaries.LinuxDo.BindStartPath) +} + func TestUpdateBalance_NilBillingCache_NoPanic(t *testing.T) { repo := &mockUserRepo{} svc := NewUserService(repo, nil, nil, nil) // billingCache = nil diff --git a/frontend/src/components/user/profile/ProfileIdentityBindingsSection.vue b/frontend/src/components/user/profile/ProfileIdentityBindingsSection.vue index 848789d9..48b1b879 100644 --- a/frontend/src/components/user/profile/ProfileIdentityBindingsSection.vue +++ b/frontend/src/components/user/profile/ProfileIdentityBindingsSection.vue @@ -362,6 +362,16 @@ function getBindingDetails(provider: UserAuthProvider): UserAuthBindingStatus | return binding } +function isProviderEnabledForBinding(provider: BindableProvider): boolean { + if (provider === 'linuxdo') { + return props.linuxdoEnabled + } + if (provider === 'oidc') { + return props.oidcEnabled + } + return resolvedWeChatBinding.value.mode !== null +} + const providerItems = computed(() => [ { provider: 'email' as const, @@ -375,7 +385,10 @@ const providerItems = computed(() => [ provider: 'linuxdo' as const, label: t('profile.authBindings.providers.linuxdo'), bound: getBindingStatus('linuxdo'), - canBind: getBindingDetails('linuxdo')?.can_bind ?? (props.linuxdoEnabled && !getBindingStatus('linuxdo')), + canBind: + !getBindingStatus('linuxdo') && + isProviderEnabledForBinding('linuxdo') && + (getBindingDetails('linuxdo')?.can_bind ?? true), canUnbind: Boolean(getBindingStatus('linuxdo') && getBindingDetails('linuxdo')?.can_unbind), details: getBindingDetails('linuxdo'), }, @@ -383,7 +396,10 @@ const providerItems = computed(() => [ provider: 'oidc' as const, label: t('profile.authBindings.providers.oidc', { providerName: props.oidcProviderName }), bound: getBindingStatus('oidc'), - canBind: getBindingDetails('oidc')?.can_bind ?? (props.oidcEnabled && !getBindingStatus('oidc')), + canBind: + !getBindingStatus('oidc') && + isProviderEnabledForBinding('oidc') && + (getBindingDetails('oidc')?.can_bind ?? true), canUnbind: Boolean(getBindingStatus('oidc') && getBindingDetails('oidc')?.can_unbind), details: getBindingDetails('oidc'), }, @@ -391,7 +407,10 @@ const providerItems = computed(() => [ provider: 'wechat' as const, label: t('profile.authBindings.providers.wechat'), bound: getBindingStatus('wechat'), - canBind: getBindingDetails('wechat')?.can_bind ?? (resolvedWeChatBinding.value.mode !== null && !getBindingStatus('wechat')), + canBind: + !getBindingStatus('wechat') && + isProviderEnabledForBinding('wechat') && + (getBindingDetails('wechat')?.can_bind ?? true), canUnbind: Boolean(getBindingStatus('wechat') && getBindingDetails('wechat')?.can_unbind), details: getBindingDetails('wechat'), }, diff --git a/frontend/src/components/user/profile/__tests__/ProfileIdentityBindingsSection.spec.ts b/frontend/src/components/user/profile/__tests__/ProfileIdentityBindingsSection.spec.ts index 345e0209..9d8c88d4 100644 --- a/frontend/src/components/user/profile/__tests__/ProfileIdentityBindingsSection.spec.ts +++ b/frontend/src/components/user/profile/__tests__/ProfileIdentityBindingsSection.spec.ts @@ -474,4 +474,26 @@ describe('ProfileIdentityBindingsSection', () => { expect(userApiMocks.unbindAuthIdentity).toHaveBeenCalledWith('linuxdo') expect(wrapper.get('[data-testid="profile-binding-linuxdo-status"]').text()).toBe('Not bound') }) + + it('hides bind actions when provider details say bindable but the provider is disabled', () => { + const wrapper = mount(ProfileIdentityBindingsSection, { + global: { + plugins: [pinia], + }, + props: { + user: createUser({ + auth_bindings: { + linuxdo: { bound: false, can_bind: true }, + oidc: { bound: false, can_bind: true }, + }, + }), + linuxdoEnabled: false, + oidcEnabled: false, + wechatEnabled: false, + }, + }) + + expect(wrapper.find('[data-testid="profile-binding-linuxdo-action"]').exists()).toBe(false) + expect(wrapper.find('[data-testid="profile-binding-oidc-action"]').exists()).toBe(false) + }) }) diff --git a/frontend/src/views/admin/SettingsView.vue b/frontend/src/views/admin/SettingsView.vue index a13f1981..5772c501 100644 --- a/frontend/src/views/admin/SettingsView.vue +++ b/frontend/src/views/admin/SettingsView.vue @@ -2032,7 +2032,7 @@ @@ -2046,7 +2046,7 @@ @@ -4961,8 +4961,8 @@ const form = reactive({ oidc_connect_redirect_url: "", oidc_connect_frontend_redirect_url: "/auth/oidc/callback", oidc_connect_token_auth_method: "client_secret_post", - oidc_connect_use_pkce: true, - oidc_connect_validate_id_token: true, + oidc_connect_use_pkce: false, + oidc_connect_validate_id_token: false, oidc_connect_allowed_signing_algs: "RS256,ES256,PS256", oidc_connect_clock_skew_seconds: 120, oidc_connect_require_email_verified: false, @@ -5846,8 +5846,8 @@ async function saveSettings() { oidc_connect_frontend_redirect_url: form.oidc_connect_frontend_redirect_url, oidc_connect_token_auth_method: form.oidc_connect_token_auth_method, - oidc_connect_use_pkce: true, - oidc_connect_validate_id_token: true, + oidc_connect_use_pkce: form.oidc_connect_use_pkce, + oidc_connect_validate_id_token: form.oidc_connect_validate_id_token, oidc_connect_allowed_signing_algs: form.oidc_connect_allowed_signing_algs, oidc_connect_clock_skew_seconds: form.oidc_connect_clock_skew_seconds, oidc_connect_require_email_verified: diff --git a/frontend/src/views/admin/__tests__/SettingsView.spec.ts b/frontend/src/views/admin/__tests__/SettingsView.spec.ts index 27a43c9f..10c51b2a 100644 --- a/frontend/src/views/admin/__tests__/SettingsView.spec.ts +++ b/frontend/src/views/admin/__tests__/SettingsView.spec.ts @@ -776,4 +776,28 @@ describe("admin SettingsView wechat connect controls", () => { ).toBe(true); expect(wrapper.text()).toContain("首次绑定时授权"); }); + + it("preserves optional OIDC compatibility flags instead of forcing them on save", async () => { + getSettings.mockResolvedValueOnce({ + ...baseSettingsResponse, + oidc_connect_enabled: true, + oidc_connect_use_pkce: false, + oidc_connect_validate_id_token: false, + }); + + const wrapper = mountView(); + + await flushPromises(); + await openSecurityTab(wrapper); + await wrapper.find("form").trigger("submit.prevent"); + await flushPromises(); + + expect(updateSettings).toHaveBeenCalledTimes(1); + expect(updateSettings).toHaveBeenCalledWith( + expect.objectContaining({ + oidc_connect_use_pkce: false, + oidc_connect_validate_id_token: false, + }), + ); + }); });