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 <wxiaoguang@gmail.com>

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"
 )

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"

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))
+		})
 	})
 }

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

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