From ecfcbcb19fb460353588f60d9549a3625df80631 Mon Sep 17 00:00:00 2001
From: oliverpool <git@olivier.pfad.fr>
Date: Thu, 7 Mar 2024 10:50:04 +0100
Subject: [PATCH 1/2] [BUG] prevent removing session cookie when redirect_uri
query contains ://
---
tests/integration/oauth_test.go | 58 +++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go
index fed73696e3..1da1c6f9c0 100644
--- a/tests/integration/oauth_test.go
+++ b/tests/integration/oauth_test.go
@@ -470,6 +470,64 @@ func TestSignInOAuthCallbackSignIn(t *testing.T) {
assert.Greater(t, userAfterLogin.LastLoginUnix, userGitLab.LastLoginUnix)
}
+func TestSignInOAuthCallbackRedirectToEscaping(t *testing.T) {
+ defer tests.PrepareTestEnv(t)()
+
+ //
+ // OAuth2 authentication source GitLab
+ //
+ gitlabName := "gitlab"
+ gitlab := addAuthSource(t, authSourcePayloadGitLabCustom(gitlabName))
+
+ //
+ // Create a user as if it had been previously been created by the GitLab
+ // authentication source.
+ //
+ userGitLabUserID := "5678"
+ userGitLab := &user_model.User{
+ Name: "gitlabuser",
+ Email: "gitlabuser@example.com",
+ Passwd: "gitlabuserpassword",
+ Type: user_model.UserTypeIndividual,
+ LoginType: auth_model.OAuth2,
+ LoginSource: gitlab.ID,
+ LoginName: userGitLabUserID,
+ }
+ defer createUser(context.Background(), t, userGitLab)()
+
+ //
+ // A request for user information sent to Goth will return a
+ // goth.User exactly matching the user created above.
+ //
+ defer mockCompleteUserAuth(func(res http.ResponseWriter, req *http.Request) (goth.User, error) {
+ return goth.User{
+ Provider: gitlabName,
+ UserID: userGitLabUserID,
+ Email: userGitLab.Email,
+ }, nil
+ })()
+ req := NewRequest(t, "GET", fmt.Sprintf("/user/oauth2/%s/callback?code=XYZ&state=XYZ", gitlabName))
+ req.AddCookie(&http.Cookie{
+ Name: "redirect_to",
+ Value: "/login/oauth/authorize?redirect_uri=https%3A%2F%2Ftranslate.example.org",
+ Path: "/",
+ })
+ resp := MakeRequest(t, req, http.StatusSeeOther)
+
+ hasNewSessionCookie := false
+ sessionCookieName := setting.SessionConfig.CookieName
+ for _, c := range resp.Result().Cookies() {
+ if c.Name == sessionCookieName {
+ hasNewSessionCookie = true
+ break
+ }
+ t.Log("Got cookie", c.Name)
+ }
+
+ assert.True(t, hasNewSessionCookie, "Session cookie %q is missing", sessionCookieName)
+ assert.Equal(t, "/login/oauth/authorize?redirect_uri=https://translate.example.org", test.RedirectURL(resp))
+}
+
func TestSignUpViaOAuthWithMissingFields(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// enable auto-creation of accounts via OAuth2
From 7444f9981e3df243865f66572eac8a25ffb8c2f1 Mon Sep 17 00:00:00 2001
From: oliverpool <git@olivier.pfad.fr>
Date: Thu, 7 Mar 2024 10:53:43 +0100
Subject: [PATCH 2/2] implement fix
---
services/context/base.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/services/context/base.go b/services/context/base.go
index c4aa467ff4..25ff935055 100644
--- a/services/context/base.go
+++ b/services/context/base.go
@@ -256,7 +256,7 @@ func (b *Base) Redirect(location string, status ...int) {
code = status[0]
}
- if strings.Contains(location, "://") || strings.HasPrefix(location, "//") {
+ if httplib.IsRiskyRedirectURL(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)