From 02df269d2449462d7471a01b265be21bb5706fa2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 5 Mar 2024 21:03:45 +0800 Subject: [PATCH] Make "/user/login" page redirect if the current user has signed in (#29583) (#29599) Backport #29583 --- modules/contexttest/context_tests.go | 3 +- routers/web/auth/auth.go | 29 ++++++++++++------- routers/web/auth/auth_test.go | 43 ++++++++++++++++++++++++++++ routers/web/repo/wiki_test.go | 2 +- 4 files changed, 65 insertions(+), 12 deletions(-) create mode 100644 routers/web/auth/auth_test.go diff --git a/modules/contexttest/context_tests.go b/modules/contexttest/context_tests.go index b9c1ab29642e5..475756ef35c14 100644 --- a/modules/contexttest/context_tests.go +++ b/modules/contexttest/context_tests.go @@ -7,6 +7,7 @@ package contexttest import ( gocontext "context" "io" + "maps" "net/http" "net/http/httptest" "net/url" @@ -36,7 +37,7 @@ func mockRequest(t *testing.T, reqPath string) *http.Request { } requestURL, err := url.Parse(path) assert.NoError(t, err) - req := &http.Request{Method: method, URL: requestURL, Form: url.Values{}} + req := &http.Request{Method: method, URL: requestURL, Form: maps.Clone(requestURL.Query()), Header: http.Header{}} req = req.WithContext(middleware.WithContextData(req.Context())) return req } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 769093255e700..e6750c18c86d0 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -109,9 +109,21 @@ func resetLocale(ctx *context.Context, u *user_model.User) error { return nil } +func RedirectAfterLogin(ctx *context.Context) { + redirectTo := ctx.FormString("redirect_to") + if redirectTo == "" { + redirectTo = ctx.GetSiteCookie("redirect_to") + } + middleware.DeleteRedirectToCookie(ctx.Resp) + nextRedirectTo := setting.AppSubURL + string(setting.LandingPageURL) + if setting.LandingPageURL == setting.LandingPageLogin { + nextRedirectTo = setting.AppSubURL + "/" // do not cycle-redirect to the login page + } + ctx.RedirectToFirst(redirectTo, nextRedirectTo) +} + func checkAutoLogin(ctx *context.Context) bool { - // Check auto-login - isSucceed, err := AutoSignIn(ctx) + isSucceed, err := AutoSignIn(ctx) // try to auto-login if err != nil { ctx.ServerError("AutoSignIn", err) return true @@ -120,17 +132,10 @@ func checkAutoLogin(ctx *context.Context) bool { redirectTo := ctx.FormString("redirect_to") if len(redirectTo) > 0 { middleware.SetRedirectToCookie(ctx.Resp, redirectTo) - } else { - redirectTo = ctx.GetSiteCookie("redirect_to") } if isSucceed { - middleware.DeleteRedirectToCookie(ctx.Resp) - nextRedirectTo := setting.AppSubURL + string(setting.LandingPageURL) - if setting.LandingPageURL == setting.LandingPageLogin { - nextRedirectTo = setting.AppSubURL + "/" // do not cycle-redirect to the login page - } - ctx.RedirectToFirst(redirectTo, nextRedirectTo) + RedirectAfterLogin(ctx) return true } @@ -146,6 +151,10 @@ func SignIn(ctx *context.Context) { return } + if ctx.IsSigned { + RedirectAfterLogin(ctx) + return + } orderedOAuth2Names, oauth2Providers, err := oauth2.GetOAuth2ProvidersMap(true) if err != nil { ctx.ServerError("UserSignIn", err) diff --git a/routers/web/auth/auth_test.go b/routers/web/auth/auth_test.go new file mode 100644 index 0000000000000..05270a48fd299 --- /dev/null +++ b/routers/web/auth/auth_test.go @@ -0,0 +1,43 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "net/http" + "net/url" + "testing" + + "code.gitea.io/gitea/modules/contexttest" + "code.gitea.io/gitea/modules/test" + + "github.com/stretchr/testify/assert" +) + +func TestUserLogin(t *testing.T) { + ctx, resp := contexttest.MockContext(t, "/user/login") + SignIn(ctx) + assert.Equal(t, http.StatusOK, resp.Code) + + ctx, resp = contexttest.MockContext(t, "/user/login") + ctx.IsSigned = true + SignIn(ctx) + assert.Equal(t, http.StatusSeeOther, resp.Code) + assert.Equal(t, "/", test.RedirectURL(resp)) + + ctx, resp = contexttest.MockContext(t, "/user/login?redirect_to=/other") + ctx.IsSigned = true + SignIn(ctx) + assert.Equal(t, "/other", test.RedirectURL(resp)) + + ctx, resp = contexttest.MockContext(t, "/user/login") + ctx.Req.AddCookie(&http.Cookie{Name: "redirect_to", Value: "/other-cookie"}) + ctx.IsSigned = true + SignIn(ctx) + assert.Equal(t, "/other-cookie", test.RedirectURL(resp)) + + ctx, resp = contexttest.MockContext(t, "/user/login?redirect_to="+url.QueryEscape("https://example.com")) + ctx.IsSigned = true + SignIn(ctx) + assert.Equal(t, "/", test.RedirectURL(resp)) +} diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go index ae050df967a52..f3259877718de 100644 --- a/routers/web/repo/wiki_test.go +++ b/routers/web/repo/wiki_test.go @@ -78,7 +78,7 @@ func assertPagesMetas(t *testing.T, expectedNames []string, metas any) { func TestWiki(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1/wiki/?action=_pages") + ctx, _ := contexttest.MockContext(t, "user2/repo1/wiki") ctx.SetParams("*", "Home") contexttest.LoadRepo(t, ctx, 1) Wiki(ctx)