fix

Author: wxiaoguang

routers/web/auth/auth.go

@@ -98,7 +98,7 @@ func autoSignIn(ctx *context.Context) (bool, error) {
 		return false, err
 	}
 
-	ctx.Csrf.DeleteCookie(ctx)
+	ctx.Csrf.PrepareForSessionUser(ctx)
 	return true, nil
 }
 
@@ -359,8 +359,8 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
 		ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)
 	}
 
-	// Clear whatever CSRF cookie has right now, force to generate a new one
-	ctx.Csrf.DeleteCookie(ctx)
+	// force to generate a new CSRF token
+	ctx.Csrf.PrepareForSessionUser(ctx)
 
 	// Register last login
 	if err := user_service.UpdateUser(ctx, u, &user_service.UpdateOptions{SetLastLogin: true}); err != nil {

routers/web/auth/oauth.go

@@ -358,8 +358,8 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
 			return
 		}
 
-		// Clear whatever CSRF cookie has right now, force to generate a new one
-		ctx.Csrf.DeleteCookie(ctx)
+		// force to generate a new CSRF token
+		ctx.Csrf.PrepareForSessionUser(ctx)
 
 		if err := resetLocale(ctx, u); err != nil {
 			ctx.ServerError("resetLocale", err)

services/auth/auth.go

@@ -103,8 +103,8 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
 
 	middleware.SetLocaleCookie(resp, user.Language, 0)
 
-	// Clear whatever CSRF has right now, force to generate a new one
+	// force to generate a new CSRF token
 	if ctx := gitea_context.GetWebContext(req); ctx != nil {
-		ctx.Csrf.DeleteCookie(ctx)
+		ctx.Csrf.PrepareForSessionUser(ctx)
 	}
 }

services/context/csrf.go

@@ -129,10 +129,8 @@ func (c *csrfProtector) PrepareForSessionUser(ctx *Context) {
 	}
 
 	if needsNew {
-		// FIXME: actionId.
 		c.token = GenerateCsrfToken(c.opt.Secret, c.id, "POST", time.Now())
-		cookie := newCsrfCookie(&c.opt, c.token)
-		ctx.Resp.Header().Add("Set-Cookie", cookie.String())
+		ctx.Resp.Header().Add("Set-Cookie", newCsrfCookie(&c.opt, c.token).String())
 	}
 
 	ctx.Data["CsrfToken"] = c.token
@@ -141,7 +139,6 @@ func (c *csrfProtector) PrepareForSessionUser(ctx *Context) {
 
 func (c *csrfProtector) validateToken(ctx *Context, token string) {
 	if !ValidCsrfToken(token, c.opt.Secret, c.id, "POST", time.Now()) {
-		c.DeleteCookie(ctx)
 		// currently, there should be no access to the APIPath with CSRF token. because templates shouldn't use the `/api/` endpoints.
 		// FIXME: distinguish what the response is for: HTML (web page) or JSON (fetch)
 		http.Error(ctx.Resp, "Invalid CSRF token.", http.StatusBadRequest)

tests/integration/admin_user_test.go

@@ -51,7 +51,7 @@ func testSuccessfullEdit(t *testing.T, formData user_model.User) {
 
 func makeRequest(t *testing.T, formData user_model.User, headerCode int) {
 	session := loginUser(t, "user1")
-	csrf := GetCSRF(t, session, "/admin/users/"+strconv.Itoa(int(formData.ID))+"/edit")
+	csrf := GetUserCSRFToken(t, session)
 	req := NewRequestWithValues(t, "POST", "/admin/users/"+strconv.Itoa(int(formData.ID))+"/edit", map[string]string{
 		"_csrf":      csrf,
 		"user_name":  formData.Name,
@@ -72,7 +72,7 @@ func TestAdminDeleteUser(t *testing.T) {
 
 	session := loginUser(t, "user1")
 
-	csrf := GetCSRF(t, session, "/admin/users/8/edit")
+	csrf := GetUserCSRFToken(t, session)
 	req := NewRequestWithValues(t, "POST", "/admin/users/8/delete", map[string]string{
 		"_csrf": csrf,
 	})

tests/integration/api_httpsig_test.go

@@ -95,7 +95,7 @@ func TestHTTPSigCert(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	session := loginUser(t, "user1")
 
-	csrf := GetCSRF(t, session, "/user/settings/keys")
+	csrf := GetUserCSRFToken(t, session)
 	req := NewRequestWithValues(t, "POST", "/user/settings/keys", map[string]string{
 		"_csrf":   csrf,
 		"content": "user1",

tests/integration/api_packages_container_test.go

@@ -784,7 +784,7 @@ func TestPackageContainer(t *testing.T) {
 		newOwnerName := "newUsername"
 
 		req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
-			"_csrf":    GetCSRF(t, session, "/user/settings"),
+			"_csrf":    GetUserCSRFToken(t, session),
 			"name":     newOwnerName,
 			"email":    "[email protected]",
 			"language": "en-US",
@@ -794,7 +794,7 @@ func TestPackageContainer(t *testing.T) {
 		t.Run(fmt.Sprintf("Catalog[%s]", newOwnerName), checkCatalog(newOwnerName))
 
 		req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
-			"_csrf":    GetCSRF(t, session, "/user/settings"),
+			"_csrf":    GetUserCSRFToken(t, session),
 			"name":     user.Name,
 			"email":    "[email protected]",
 			"language": "en-US",

tests/integration/attachment_test.go

@@ -57,14 +57,14 @@ func createAttachment(t *testing.T, session *TestSession, csrf, repoURL, filenam
 func TestCreateAnonymousAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	session := emptyTestSession(t)
-	createAttachment(t, session, GetCSRF(t, session, "/user/login"), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
+	createAttachment(t, session, GetAnonymousCSRFToken(t, session), "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
 }
 
 func TestCreateIssueAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 	const repoURL = "user2/repo1"
 	session := loginUser(t, "user2")
-	uuid := createAttachment(t, session, GetCSRF(t, session, repoURL), repoURL, "image.png", generateImg(), http.StatusOK)
+	uuid := createAttachment(t, session, GetUserCSRFToken(t, session), repoURL, "image.png", generateImg(), http.StatusOK)
 
 	req := NewRequest(t, "GET", repoURL+"/issues/new")
 	resp := session.MakeRequest(t, req, http.StatusOK)

tests/integration/auth_ldap_test.go

@@ -156,7 +156,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupM
 		groupTeamMap = groupMapParams[1]
 	}
 	session := loginUser(t, "user1")
-	csrf := GetCSRF(t, session, "/admin/auths/new")
+	csrf := GetUserCSRFToken(t, session)
 	req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval))
 	session.MakeRequest(t, req, http.StatusSeeOther)
 }
@@ -252,7 +252,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
 	session := loginUser(t, "user1")
-	csrf := GetCSRF(t, session, "/admin/auths/new")
+	csrf := GetUserCSRFToken(t, session)
 	payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "")
 	payload["attribute_username"] = ""
 	req := NewRequestWithValues(t, "POST", "/admin/auths/new", payload)
@@ -487,7 +487,7 @@ func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
 	session := loginUser(t, "user1")
-	csrf := GetCSRF(t, session, "/admin/auths/new")
+	csrf := GetUserCSRFToken(t, session)
 	req := NewRequestWithValues(t, "POST", "/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off"))
 	session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok
 }

tests/integration/change_default_branch_test.go

@@ -22,15 +22,15 @@ func TestChangeDefaultBranch(t *testing.T) {
 	session := loginUser(t, owner.Name)
 	branchesURL := fmt.Sprintf("/%s/%s/settings/branches", owner.Name, repo.Name)
 
-	csrf := GetCSRF(t, session, branchesURL)
+	csrf := GetUserCSRFToken(t, session)
 	req := NewRequestWithValues(t, "POST", branchesURL, map[string]string{
 		"_csrf":  csrf,
 		"action": "default_branch",
 		"branch": "DefaultBranch",
 	})
 	session.MakeRequest(t, req, http.StatusSeeOther)
 
-	csrf = GetCSRF(t, session, branchesURL)
+	csrf = GetUserCSRFToken(t, session)
 	req = NewRequestWithValues(t, "POST", branchesURL, map[string]string{
 		"_csrf":  csrf,
 		"action": "default_branch",