From 4852091e851060d2233fcbf789727b1c5dba4bfc Mon Sep 17 00:00:00 2001 From: "roman.s" <55788842+majorissuerep@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:39:44 +0300 Subject: [PATCH] fix(auth): record last sign-in on reverse proxy login (#38672) Reverse proxy and SSPI logins establish a session but never recorded `last_login_unix`, so those users stayed "Never Signed-In" in admin. The write is folded into the language update that `handleSignIn` already does, so it stays at one query and only runs when a session is established. Fixes https://github.com/go-gitea/gitea/issues/7836 --------- Co-authored-by: roman s Co-authored-by: silverwind Co-authored-by: wxiaoguang --- services/auth/auth.go | 18 ++++++------- services/auth/reverseproxy.go | 2 +- services/auth/reverseproxy_test.go | 41 ++++++++++++++++++++++++++++++ services/auth/sspi.go | 2 +- 4 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 services/auth/reverseproxy_test.go diff --git a/services/auth/auth.go b/services/auth/auth.go index 97f29cdbf4e..fb7d0fa9a19 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -38,8 +38,9 @@ func Init() { webauthn.Init() } -// handleSignIn clears existing session variables and stores new ones for the specified user object -func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *user_model.User) { +// handleSignInNonInteractive clears existing session variables and stores new ones for the specified user object +// it is mainly for middleware sign-in which doesn't need user's interaction. +func handleSignInNonInteractive(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *user_model.User) { // We need to regenerate the session... newSess, err := session.RegenerateSession(resp, req) if err != nil { @@ -54,17 +55,14 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore log.Error(fmt.Sprintf("Error setting session: %v", err)) } + opts := &user_service.UpdateOptions{SetLastLogin: true} // Language setting of the user overwrites the one previously set // If the user does not have a locale set, we save the current one. if len(user.Language) == 0 { - lc := middleware.Locale(resp, req) - opts := &user_service.UpdateOptions{ - Language: optional.Some(lc.Language()), - } - if err := user_service.UpdateUser(req.Context(), user, opts); err != nil { - log.Error(fmt.Sprintf("Error updating user language [user: %d, locale: %s]", user.ID, user.Language)) - return - } + opts.Language = optional.Some(middleware.Locale(resp, req).Language()) + } + if err := user_service.UpdateUser(req.Context(), user, opts); err != nil { + log.Error("Error updating user on sign-in [user: %d]: %v", user.ID, err) } middleware.SetLocaleCookie(resp, user.Language, 0) diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index ff95329c9e6..b7d8d013c33 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -121,7 +121,7 @@ func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, store Da if r.CreateSession && sess != nil { sessionUID, ok := sess.Get(session.KeyUID).(int64) if !ok || sessionUID != user.ID { - handleSignIn(w, req, sess, user) + handleSignInNonInteractive(w, req, sess, user) } } store.GetData()["IsReverseProxy"] = true diff --git a/services/auth/reverseproxy_test.go b/services/auth/reverseproxy_test.go new file mode 100644 index 00000000000..0602d295b37 --- /dev/null +++ b/services/auth/reverseproxy_test.go @@ -0,0 +1,41 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "testing" + + "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + "gitea.dev/modules/session" + "gitea.dev/modules/setting" + "gitea.dev/modules/test" + "gitea.dev/services/contexttest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReverseProxyLastLogin(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.ReverseProxyAuthUser, "X-WEBAUTH-USER")() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + require.Zero(t, user.LastLoginUnix) + + ctx, resp := contexttest.MockContext(t, "/", contexttest.MockContextOption{SessionStore: session.NewMockMemStore("reverse-proxy-last-login")}) + ctx.Req.Header.Set(setting.ReverseProxyAuthUser, user.Name) + rp := &ReverseProxy{CreateSession: true} + + _, err := rp.Verify(ctx.Req, resp, ctx, ctx.Session) + require.NoError(t, err) + assert.NotZero(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID}).LastLoginUnix) + + user.LastLoginUnix = 1 + require.NoError(t, user_model.UpdateUserCols(t.Context(), user, "last_login_unix")) + + _, err = rp.Verify(ctx.Req, resp, ctx, ctx.Session) + require.NoError(t, err) + assert.EqualValues(t, 1, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID}).LastLoginUnix) // no write without a new session +} diff --git a/services/auth/sspi.go b/services/auth/sspi.go index 185c37edda1..d3cca5d1391 100644 --- a/services/auth/sspi.go +++ b/services/auth/sspi.go @@ -121,7 +121,7 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, } if s.CreateSession { - handleSignIn(w, req, sess, user) + handleSignInNonInteractive(w, req, sess, user) } log.Trace("SSPI Authorization: Logged in user %-v", user)