From 9bc4887cb714d18e4fb104cdb43744e1dcec6a44 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 18 May 2023 09:10:23 +0800 Subject: [PATCH] Fix safari cookie session bug (#24772) Partically backport #24330 Related: #24176 Maybe fix #24770 (cherry picked from commit 64cc691b7f5977915b686bdf85e4c6ffe9692a35) --- modules/context/context.go | 20 ++++++++++++++++++++ modules/context/context_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 modules/context/context_test.go diff --git a/modules/context/context.go b/modules/context/context.go index 955c3d9a70621..eecad20606102 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -473,6 +473,17 @@ func (ctx *Context) JSON(status int, content interface{}) { } } +func removeSessionCookieHeader(w http.ResponseWriter) { + cookies := w.Header()["Set-Cookie"] + w.Header().Del("Set-Cookie") + for _, cookie := range cookies { + if strings.HasPrefix(cookie, setting.SessionConfig.CookieName+"=") { + continue + } + w.Header().Add("Set-Cookie", cookie) + } +} + // Redirect redirects the request func (ctx *Context) Redirect(location string, status ...int) { code := http.StatusSeeOther @@ -480,6 +491,15 @@ func (ctx *Context) Redirect(location string, status ...int) { code = status[0] } + if strings.Contains(location, "://") || strings.HasPrefix(location, "//") { + // Some browsers (Safari) have buggy behavior for Cookie + Cache + External Redirection, eg: /my-path => https://other/path + // 1. the first request to "/my-path" contains cookie + // 2. some time later, the request to "/my-path" doesn't contain cookie (caused by Prevent web tracking) + // 3. Gitea's Sessioner doesn't see the session cookie, so it generates a new session id, and returns it to browser + // 4. then the browser accepts the empty session, then the user is logged out + // So in this case, we should remove the session cookie from the response header + removeSessionCookieHeader(ctx.Resp) + } http.Redirect(ctx.Resp, ctx.Req, location, code) } diff --git a/modules/context/context_test.go b/modules/context/context_test.go new file mode 100644 index 0000000000000..033ce2ef0ad52 --- /dev/null +++ b/modules/context/context_test.go @@ -0,0 +1,24 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package context + +import ( + "net/http" + "net/http/httptest" + "testing" + + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func TestRemoveSessionCookieHeader(t *testing.T) { + w := httptest.NewRecorder() + w.Header().Add("Set-Cookie", (&http.Cookie{Name: setting.SessionConfig.CookieName, Value: "foo"}).String()) + w.Header().Add("Set-Cookie", (&http.Cookie{Name: "other", Value: "bar"}).String()) + assert.Len(t, w.Header().Values("Set-Cookie"), 2) + removeSessionCookieHeader(w) + assert.Len(t, w.Header().Values("Set-Cookie"), 1) + assert.Contains(t, "other=bar", w.Header().Get("Set-Cookie")) +}