Merge pull request '[BUG] prevent removing session cookie when redirect_uri query contains ://' (#2606) from oliverpool/forgejo:backport2590 into v1.21/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2606
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

modules/context/base.go

@@ -255,7 +255,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)

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