diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index cf58316c..964dbb88 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -334,7 +334,7 @@ func TestValidateLinuxDoFrontendRedirectURL(t *testing.T) { cfg.LinuxDo.ClientSecret = "test-secret" cfg.LinuxDo.RedirectURL = "https://example.com/api/v1/auth/oauth/linuxdo/callback" cfg.LinuxDo.TokenAuthMethod = "client_secret_post" - cfg.LinuxDo.UsePKCE = false + cfg.LinuxDo.UsePKCE = true cfg.LinuxDo.FrontendRedirectURL = "javascript:alert(1)" err = cfg.Validate() diff --git a/backend/internal/handler/auth_oidc_oauth.go b/backend/internal/handler/auth_oidc_oauth.go index 9d24df88..37ef6833 100644 --- a/backend/internal/handler/auth_oidc_oauth.go +++ b/backend/internal/handler/auth_oidc_oauth.go @@ -127,30 +127,34 @@ func (h *AuthHandler) OIDCOAuthStart(c *gin.Context) { redirectTo = oidcOAuthDefaultRedirectTo } + browserSessionKey, err := generateOAuthPendingBrowserSession() + if err != nil { + response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_BROWSER_SESSION_GEN_FAILED", "failed to generate oauth browser session").WithCause(err)) + return + } + secureCookie := isRequestHTTPS(c) oidcSetCookie(c, oidcOAuthStateCookieName, encodeCookieValue(state), oidcOAuthCookieMaxAgeSec, secureCookie) oidcSetCookie(c, oidcOAuthRedirectCookie, encodeCookieValue(redirectTo), oidcOAuthCookieMaxAgeSec, secureCookie) + setOAuthPendingBrowserCookie(c, browserSessionKey, secureCookie) + clearOAuthPendingSessionCookie(c, secureCookie) codeChallenge := "" - 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) + 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) nonce := "" - 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) + 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) redirectURI := strings.TrimSpace(cfg.RedirectURL) if redirectURI == "" { @@ -212,23 +216,24 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { if redirectTo == "" { redirectTo = oidcOAuthDefaultRedirectTo } + browserSessionKey, _ := readOAuthPendingBrowserCookie(c) + if strings.TrimSpace(browserSessionKey) == "" { + redirectOAuthError(c, frontendCallback, "missing_browser_session", "missing oauth browser session", "") + return + } codeVerifier := "" - if cfg.UsePKCE { - codeVerifier, _ = readCookieDecoded(c, oidcOAuthVerifierCookie) - if codeVerifier == "" { - redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "") - return - } + codeVerifier, _ = readCookieDecoded(c, oidcOAuthVerifierCookie) + if codeVerifier == "" { + redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "") + return } expectedNonce := "" - if cfg.ValidateIDToken { - expectedNonce, _ = readCookieDecoded(c, oidcOAuthNonceCookie) - if expectedNonce == "" { - redirectOAuthError(c, frontendCallback, "missing_nonce", "missing oauth nonce", "") - return - } + expectedNonce, _ = readCookieDecoded(c, oidcOAuthNonceCookie) + if expectedNonce == "" { + redirectOAuthError(c, frontendCallback, "missing_nonce", "missing oauth nonce", "") + return } redirectURI := strings.TrimSpace(cfg.RedirectURL) @@ -258,7 +263,7 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { return } - if cfg.ValidateIDToken && strings.TrimSpace(tokenResp.IDToken) == "" { + if strings.TrimSpace(tokenResp.IDToken) == "" { redirectOAuthError(c, frontendCallback, "missing_id_token", "missing id_token", "") return } @@ -304,9 +309,13 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { return } } + if 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) - email := oidcSelectLoginEmail(userInfoClaims.Email, idClaims.Email, identityKey) + email := oidcSyntheticEmailFromIdentityKey(identityKey) username := firstNonEmpty( userInfoClaims.Username, idClaims.PreferredUsername, @@ -318,34 +327,73 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) { tokenPair, _, err := h.authService.LoginOrRegisterOAuthWithTokenPair(c.Request.Context(), email, username, "") if err != nil { if errors.Is(err, service.ErrOAuthInvitationRequired) { - pendingToken, tokenErr := h.authService.CreatePendingOAuthToken(email, username) - if tokenErr != nil { - redirectOAuthError(c, frontendCallback, "login_failed", "service_error", "") + if err := h.createOAuthPendingSession(c, oauthPendingSessionPayload{ + Intent: "login", + Identity: service.PendingAuthIdentityKey{ + ProviderType: "oidc", + ProviderKey: issuer, + ProviderSubject: subject, + }, + ResolvedEmail: email, + RedirectTo: redirectTo, + BrowserSessionKey: browserSessionKey, + UpstreamIdentityClaims: map[string]any{ + "email": email, + "username": username, + "subject": subject, + "issuer": issuer, + "email_verified": emailVerified != nil && *emailVerified, + "provider_fallback": strings.TrimSpace(cfg.ProviderName), + }, + CompletionResponse: map[string]any{ + "error": "invitation_required", + "redirect": redirectTo, + }, + }); err != nil { + redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "") return } - fragment := url.Values{} - fragment.Set("error", "invitation_required") - fragment.Set("pending_oauth_token", pendingToken) - fragment.Set("redirect", redirectTo) - redirectWithFragment(c, frontendCallback, fragment) + redirectToFrontendCallback(c, frontendCallback) return } redirectOAuthError(c, frontendCallback, "login_failed", infraerrors.Reason(err), infraerrors.Message(err)) return } - fragment := url.Values{} - fragment.Set("access_token", tokenPair.AccessToken) - fragment.Set("refresh_token", tokenPair.RefreshToken) - fragment.Set("expires_in", fmt.Sprintf("%d", tokenPair.ExpiresIn)) - fragment.Set("token_type", "Bearer") - fragment.Set("redirect", redirectTo) - redirectWithFragment(c, frontendCallback, fragment) + if err := h.createOAuthPendingSession(c, oauthPendingSessionPayload{ + Intent: "login", + Identity: service.PendingAuthIdentityKey{ + ProviderType: "oidc", + ProviderKey: issuer, + ProviderSubject: subject, + }, + ResolvedEmail: email, + RedirectTo: redirectTo, + BrowserSessionKey: browserSessionKey, + UpstreamIdentityClaims: map[string]any{ + "email": email, + "username": username, + "subject": subject, + "issuer": issuer, + "email_verified": emailVerified != nil && *emailVerified, + "provider_fallback": strings.TrimSpace(cfg.ProviderName), + }, + CompletionResponse: map[string]any{ + "access_token": tokenPair.AccessToken, + "refresh_token": tokenPair.RefreshToken, + "expires_in": tokenPair.ExpiresIn, + "token_type": "Bearer", + "redirect": redirectTo, + }, + }); err != nil { + redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "") + return + } + redirectToFrontendCallback(c, frontendCallback) } type completeOIDCOAuthRequest struct { - PendingOAuthToken string `json:"pending_oauth_token" binding:"required"` - InvitationCode string `json:"invitation_code" binding:"required"` + InvitationCode string `json:"invitation_code" binding:"required"` } // CompleteOIDCOAuthRegistration completes a pending OAuth registration by validating @@ -358,9 +406,38 @@ func (h *AuthHandler) CompleteOIDCOAuthRegistration(c *gin.Context) { return } - email, username, err := h.authService.VerifyPendingOAuthToken(req.PendingOAuthToken) + secureCookie := isRequestHTTPS(c) + sessionToken, err := readOAuthPendingSessionCookie(c) if err != nil { - c.JSON(http.StatusUnauthorized, gin.H{"error": "INVALID_TOKEN", "message": "invalid or expired registration token"}) + clearOAuthPendingSessionCookie(c, secureCookie) + clearOAuthPendingBrowserCookie(c, secureCookie) + response.ErrorFrom(c, service.ErrPendingAuthSessionNotFound) + return + } + browserSessionKey, err := readOAuthPendingBrowserCookie(c) + if err != nil { + clearOAuthPendingSessionCookie(c, secureCookie) + clearOAuthPendingBrowserCookie(c, secureCookie) + response.ErrorFrom(c, service.ErrPendingAuthBrowserMismatch) + return + } + pendingSvc, err := h.pendingIdentityService() + if err != nil { + response.ErrorFrom(c, err) + return + } + session, err := pendingSvc.GetBrowserSession(c.Request.Context(), sessionToken, browserSessionKey) + if err != nil { + clearOAuthPendingSessionCookie(c, secureCookie) + clearOAuthPendingBrowserCookie(c, secureCookie) + response.ErrorFrom(c, err) + return + } + + email := strings.TrimSpace(session.ResolvedEmail) + username := pendingSessionStringValue(session.UpstreamIdentityClaims, "username") + if email == "" || username == "" { + response.ErrorFrom(c, infraerrors.BadRequest("PENDING_AUTH_SESSION_INVALID", "pending auth registration context is invalid")) return } @@ -369,6 +446,14 @@ func (h *AuthHandler) CompleteOIDCOAuthRegistration(c *gin.Context) { response.ErrorFrom(c, err) return } + if _, err := pendingSvc.ConsumeBrowserSession(c.Request.Context(), sessionToken, browserSessionKey); err != nil { + clearOAuthPendingSessionCookie(c, secureCookie) + clearOAuthPendingBrowserCookie(c, secureCookie) + response.ErrorFrom(c, err) + return + } + clearOAuthPendingSessionCookie(c, secureCookie) + clearOAuthPendingBrowserCookie(c, secureCookie) c.JSON(http.StatusOK, gin.H{ "access_token": tokenPair.AccessToken, @@ -405,9 +490,7 @@ func oidcExchangeCode( form.Set("client_id", cfg.ClientID) form.Set("code", code) form.Set("redirect_uri", redirectURI) - if cfg.UsePKCE { - form.Set("code_verifier", codeVerifier) - } + form.Set("code_verifier", codeVerifier) r := client.R(). SetContext(ctx). @@ -592,13 +675,9 @@ func buildOIDCAuthorizeURL(cfg config.OIDCConnectConfig, state, nonce, codeChall q.Set("scope", cfg.Scopes) } q.Set("state", state) - if strings.TrimSpace(nonce) != "" { - q.Set("nonce", nonce) - } - if cfg.UsePKCE { - q.Set("code_challenge", codeChallenge) - q.Set("code_challenge_method", "S256") - } + q.Set("nonce", nonce) + q.Set("code_challenge", codeChallenge) + q.Set("code_challenge_method", "S256") u.RawQuery = q.Encode() return u.String(), nil @@ -831,14 +910,6 @@ func oidcSyntheticEmailFromIdentityKey(identityKey string) string { return "oidc-" + hex.EncodeToString(sum[:16]) + service.OIDCConnectSyntheticEmailDomain } -func oidcSelectLoginEmail(userInfoEmail, idTokenEmail, identityKey string) string { - email := strings.TrimSpace(firstNonEmpty(userInfoEmail, idTokenEmail)) - if email != "" { - return email - } - return oidcSyntheticEmailFromIdentityKey(identityKey) -} - func oidcFallbackUsername(subject string) string { subject = strings.TrimSpace(subject) if subject == "" { diff --git a/backend/internal/handler/auth_oidc_oauth_test.go b/backend/internal/handler/auth_oidc_oauth_test.go index a161aa77..a4cf776a 100644 --- a/backend/internal/handler/auth_oidc_oauth_test.go +++ b/backend/internal/handler/auth_oidc_oauth_test.go @@ -30,26 +30,11 @@ func TestOIDCSyntheticEmailStableAndDistinct(t *testing.T) { require.Contains(t, e1, "@oidc-connect.invalid") } -func TestOIDCSelectLoginEmailPrefersRealEmail(t *testing.T) { - identityKey := oidcIdentityKey("https://issuer.example.com", "subject-a") - - email := oidcSelectLoginEmail("user@example.com", "idtoken@example.com", identityKey) - require.Equal(t, "user@example.com", email) - - email = oidcSelectLoginEmail("", "idtoken@example.com", identityKey) - require.Equal(t, "idtoken@example.com", email) - - email = oidcSelectLoginEmail("", "", identityKey) - require.Contains(t, email, "@oidc-connect.invalid") - require.Equal(t, oidcSyntheticEmailFromIdentityKey(identityKey), email) -} - func TestBuildOIDCAuthorizeURLIncludesNonceAndPKCE(t *testing.T) { cfg := config.OIDCConnectConfig{ AuthorizeURL: "https://issuer.example.com/auth", ClientID: "cid", Scopes: "openid email profile", - UsePKCE: true, } u, err := buildOIDCAuthorizeURL(cfg, "state123", "nonce123", "challenge123", "https://app.example.com/callback") diff --git a/frontend/src/views/admin/SettingsView.vue b/frontend/src/views/admin/SettingsView.vue index ee6a4c6d..8bfa0f2b 100644 --- a/frontend/src/views/admin/SettingsView.vue +++ b/frontend/src/views/admin/SettingsView.vue @@ -1382,7 +1382,7 @@ {{ t('admin.settings.oidc.usePkce') }} - +
@@ -1391,7 +1391,7 @@ {{ t('admin.settings.oidc.validateIdToken') }}
- +
@@ -3024,7 +3024,7 @@ 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: false, + oidc_connect_use_pkce: true, oidc_connect_validate_id_token: true, oidc_connect_allowed_signing_algs: 'RS256,ES256,PS256', oidc_connect_clock_skew_seconds: 120, @@ -3613,8 +3613,8 @@ async function saveSettings() { oidc_connect_redirect_url: form.oidc_connect_redirect_url, 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: form.oidc_connect_use_pkce, - oidc_connect_validate_id_token: form.oidc_connect_validate_id_token, + oidc_connect_use_pkce: true, + oidc_connect_validate_id_token: true, 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: form.oidc_connect_require_email_verified,