From e67dd4c81806fe63583068badac30bc1ee0bde4f Mon Sep 17 00:00:00 2001 From: Harsh Satyajit Thakur Date: Tue, 28 Jul 2026 01:35:48 +1000 Subject: [PATCH] fix: skip OIDC end-session after password login for OAuth2 users (#38439) Fixes #38209 OAuth2-linked accounts that sign in via the password form were still redirected to the provider end_session_endpoint on logout because the redirect was keyed off account LoginType. Store the session sign-in method (password vs oauth2) and only use RP-initiated OIDC logout when this session was authenticated via OAuth2. Sessions without the new key keep the previous LoginType behavior. --------- Co-authored-by: wxiaoguang --- modules/session/key.go | 6 ++++++ routers/web/auth/auth.go | 10 +++++++-- routers/web/auth/auth_test.go | 37 +++++++++++++++++++++++---------- routers/web/auth/linkaccount.go | 8 ++++--- routers/web/auth/oauth.go | 6 ++++-- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/modules/session/key.go b/modules/session/key.go index 5a6b14ec403..7734a62afd1 100644 --- a/modules/session/key.go +++ b/modules/session/key.go @@ -9,4 +9,10 @@ const ( KeyImpersonatorData = "impersonatorData" KeyUserHasTwoFactorAuth = "userHasTwoFactorAuth" + + // KeySignInMethod records how the current session was authenticated so logout + // can decide whether RP-initiated OIDC logout is appropriate. + KeySignInMethod = "signInMethod" + + SignInMethodOAuth2 = "oauth2" ) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index b60592e39b4..13ea2e8bb16 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -480,20 +480,26 @@ func SignOut(ctx *context.Context) { } func buildSignOutRedirectURL(ctx *context.Context) string { - if ctx.Doer != nil && ctx.Doer.LoginType == auth.OAuth2 { + if ctx.Doer != nil && shouldRedirectToOIDCEndSession(ctx) { if s := buildOIDCEndSessionURL(ctx, ctx.Doer); s != "" { return s } } // The assumption is: if reverse proxy auth is enabled, then the users should only sign-in via reverse proxy auth. - // TODO: in the future, if we need to distinguish different sign-in methods, we need to save the sign-in method in session and check here if setting.Service.EnableReverseProxyAuth && setting.ReverseProxyLogoutRedirect != "" { return setting.ReverseProxyLogoutRedirect } return setting.AppSubURL + "/" } +// shouldRedirectToOIDCEndSession reports whether this session should end at the +// OIDC provider. Prefer the session sign-in method so an OAuth2-linked account +// that signed in with a password does not hit end_session_endpoint. +func shouldRedirectToOIDCEndSession(ctx *context.Context) bool { + return ctx.Session.Get(session.KeySignInMethod) == session.SignInMethodOAuth2 +} + func prepareSignUpPageData(ctx *context.Context) bool { ctx.Data["Title"] = ctx.Tr("sign_up") ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/sign_up" diff --git a/routers/web/auth/auth_test.go b/routers/web/auth/auth_test.go index 54104061614..f31511c4ff9 100644 --- a/routers/web/auth/auth_test.go +++ b/routers/web/auth/auth_test.go @@ -154,16 +154,31 @@ func TestWebAuthOAuth2(t *testing.T) { authSource, err := auth_model.GetActiveOAuth2SourceByAuthName(t.Context(), "oidc-auth-source") require.NoError(t, err) - mockOpt := contexttest.MockContextOption{SessionStore: session.NewMockMemStore("dummy-sid")} - ctx, resp := contexttest.MockContext(t, "/user/logout", mockOpt) - ctx.Doer = &user_model.User{ID: 1, LoginType: auth_model.OAuth2, LoginSource: authSource.ID} - SignOut(ctx) - assert.Equal(t, http.StatusSeeOther, resp.Code) - u, err := url.Parse(test.RedirectURL(resp)) - require.NoError(t, err) - expectedValues := url.Values{"oidc-key": []string{"oidc-val"}, "post_logout_redirect_uri": []string{setting.AppURL}, "client_id": []string{"mock-client-id"}} - assert.Equal(t, expectedValues, u.Query()) - u.RawQuery = "" - assert.Equal(t, "https://example.com/oidc-logout", u.String()) + oauthUser := &user_model.User{ID: 1, LoginType: auth_model.OAuth2, LoginSource: authSource.ID} + + t.Run("OAuth2SignInRedirectsToOIDC", func(t *testing.T) { + mockOpt := contexttest.MockContextOption{SessionStore: session.NewMockMemStore("dummy-sid-oauth")} + ctx, resp := contexttest.MockContext(t, "/user/logout", mockOpt) + ctx.Doer = oauthUser + require.NoError(t, ctx.Session.Set(session.KeySignInMethod, session.SignInMethodOAuth2)) + SignOut(ctx) + assert.Equal(t, http.StatusSeeOther, resp.Code) + u, err := url.Parse(test.RedirectURL(resp)) + require.NoError(t, err) + expectedValues := url.Values{"oidc-key": []string{"oidc-val"}, "post_logout_redirect_uri": []string{setting.AppURL}, "client_id": []string{"mock-client-id"}} + assert.Equal(t, expectedValues, u.Query()) + u.RawQuery = "" + assert.Equal(t, "https://example.com/oidc-logout", u.String()) + }) + + t.Run("PasswordSignInSkipsOIDC", func(t *testing.T) { + // OAuth2-linked account signed in via password form must not hit end_session_endpoint. + mockOpt := contexttest.MockContextOption{SessionStore: session.NewMockMemStore("dummy-sid-password")} + ctx, resp := contexttest.MockContext(t, "/user/logout", mockOpt) + ctx.Doer = oauthUser + SignOut(ctx) + assert.Equal(t, http.StatusSeeOther, resp.Code) + assert.Equal(t, "/", test.RedirectURL(resp)) + }) }) } diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index 93dd577d8c4..831701da516 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -11,6 +11,7 @@ import ( "gitea.dev/models/auth" user_model "gitea.dev/models/user" "gitea.dev/modules/log" + "gitea.dev/modules/session" "gitea.dev/modules/setting" "gitea.dev/modules/templates" "gitea.dev/modules/util" @@ -171,9 +172,10 @@ func oauth2LinkAccount(ctx *context.Context, u *user_model.User, linkAccountData if err := regenerateSession(ctx, map[string]any{ // User needs to use 2FA, save data and redirect to 2FA page. - "twofaUid": u.ID, - "twofaRemember": remember, - "linkAccount": true, + "twofaUid": u.ID, + "twofaRemember": remember, + "linkAccount": true, + session.KeySignInMethod: session.SignInMethodOAuth2, }); err != nil { ctx.ServerError("RegenerateSession", err) return diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index e5dc1ac5eee..27b32d09dfa 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -431,6 +431,7 @@ func handleOAuth2SignIn(ctx *context.Context, authSource *auth.Source, u *user_m if err := regenerateSession(ctx, map[string]any{ session.KeyUID: u.ID, session.KeyUserHasTwoFactorAuth: userHasTwoFactorAuth, + session.KeySignInMethod: session.SignInMethodOAuth2, }); err != nil { ctx.ServerError("updateSession", err) return @@ -454,8 +455,9 @@ func handleOAuth2SignIn(ctx *context.Context, authSource *auth.Source, u *user_m if err := regenerateSession(ctx, map[string]any{ // User needs to use 2FA, save data and redirect to 2FA page. - "twofaUid": u.ID, - "twofaRemember": false, + "twofaUid": u.ID, + "twofaRemember": false, + session.KeySignInMethod: session.SignInMethodOAuth2, }); err != nil { ctx.ServerError("updateSession", err) return