Refactor CSRF protector (#32057) (fix forgejo tests)

Fix the tests unique to Forgejo that are impacted by the refactor.

(cherry picked from commit 6275d1bc50)
This commit is contained in:
Earl Warren 2024-09-22 12:57:03 +02:00 committed by forgejo-backport-action
parent d26b7902ec
commit 5b6d8a303d
3 changed files with 59 additions and 38 deletions

View file

@ -30,8 +30,9 @@ import (
) )
const ( const (
CsrfHeaderName = "X-Csrf-Token" CsrfHeaderName = "X-Csrf-Token"
CsrfFormName = "_csrf" CsrfFormName = "_csrf"
CsrfErrorString = "Invalid CSRF token."
) )
// CSRFProtector represents a CSRF protector and is used to get the current token and validate the token. // CSRFProtector represents a CSRF protector and is used to get the current token and validate the token.
@ -144,7 +145,7 @@ func (c *csrfProtector) validateToken(ctx *Context, token string) {
c.DeleteCookie(ctx) c.DeleteCookie(ctx)
// currently, there should be no access to the APIPath with CSRF token. because templates shouldn't use the `/api/` endpoints. // 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) // FIXME: distinguish what the response is for: HTML (web page) or JSON (fetch)
http.Error(ctx.Resp, "Invalid CSRF token.", http.StatusBadRequest) http.Error(ctx.Resp, CsrfErrorString, http.StatusBadRequest)
} }
} }

View file

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/test"
forgejo_context "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -190,11 +191,6 @@ func TestRedirectsWebhooks(t *testing.T) {
{from: "/user/settings/hooks/" + kind + "/new", to: "/user/login", verb: "GET"}, {from: "/user/settings/hooks/" + kind + "/new", to: "/user/login", verb: "GET"},
{from: "/admin/system-hooks/" + kind + "/new", to: "/user/login", verb: "GET"}, {from: "/admin/system-hooks/" + kind + "/new", to: "/user/login", verb: "GET"},
{from: "/admin/default-hooks/" + kind + "/new", to: "/user/login", verb: "GET"}, {from: "/admin/default-hooks/" + kind + "/new", to: "/user/login", verb: "GET"},
{from: "/user2/repo1/settings/hooks/" + kind + "/new", to: "/", verb: "POST"},
{from: "/admin/system-hooks/" + kind + "/new", to: "/", verb: "POST"},
{from: "/admin/default-hooks/" + kind + "/new", to: "/", verb: "POST"},
{from: "/user2/repo1/settings/hooks/1", to: "/", verb: "POST"},
{from: "/admin/hooks/1", to: "/", verb: "POST"},
} }
for _, info := range redirects { for _, info := range redirects {
req := NewRequest(t, info.verb, info.from) req := NewRequest(t, info.verb, info.from)
@ -202,6 +198,24 @@ func TestRedirectsWebhooks(t *testing.T) {
assert.EqualValues(t, path.Join(setting.AppSubURL, info.to), test.RedirectURL(resp), info.from) assert.EqualValues(t, path.Join(setting.AppSubURL, info.to), test.RedirectURL(resp), info.from)
} }
} }
for _, kind := range []string{"forgejo", "gitea"} {
csrf := []struct {
from string
verb string
}{
{from: "/user2/repo1/settings/hooks/" + kind + "/new", verb: "POST"},
{from: "/admin/hooks/1", verb: "POST"},
{from: "/admin/system-hooks/" + kind + "/new", verb: "POST"},
{from: "/admin/default-hooks/" + kind + "/new", verb: "POST"},
{from: "/user2/repo1/settings/hooks/1", verb: "POST"},
}
for _, info := range csrf {
req := NewRequest(t, info.verb, info.from)
resp := MakeRequest(t, req, http.StatusBadRequest)
assert.Contains(t, resp.Body.String(), forgejo_context.CsrfErrorString)
}
}
} }
func TestRepoLinks(t *testing.T) { func TestRepoLinks(t *testing.T) {

View file

@ -11,6 +11,7 @@ import (
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
"net/http/httptest"
"net/url" "net/url"
"strings" "strings"
"testing" "testing"
@ -24,6 +25,7 @@ import (
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/routers/web/auth" "code.gitea.io/gitea/routers/web/auth"
forgejo_context "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
"github.com/markbates/goth" "github.com/markbates/goth"
@ -803,6 +805,16 @@ func TestOAuthIntrospection(t *testing.T) {
}) })
} }
func requireCookieCSRF(t *testing.T, resp http.ResponseWriter) string {
for _, c := range resp.(*httptest.ResponseRecorder).Result().Cookies() {
if c.Name == "_csrf" {
return c.Value
}
}
require.True(t, false, "_csrf not found in cookies")
return ""
}
func TestOAuth_GrantScopesReadUser(t *testing.T) { func TestOAuth_GrantScopesReadUser(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()
@ -840,19 +852,18 @@ func TestOAuth_GrantScopesReadUser(t *testing.T) {
authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther)
authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0] authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0]
htmlDoc := NewHTMLParser(t, authorizeResp.Body)
grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"client_id": app.ClientID, "client_id": app.ClientID,
"redirect_uri": "a", "redirect_uri": "a",
"state": "thestate", "state": "thestate",
"granted": "true", "granted": "true",
}) })
grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest)
htmlDocGrant := NewHTMLParser(t, grantResp.Body) assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString)
accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"_csrf": htmlDocGrant.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"grant_type": "authorization_code", "grant_type": "authorization_code",
"client_id": app.ClientID, "client_id": app.ClientID,
"client_secret": app.ClientSecret, "client_secret": app.ClientSecret,
@ -921,19 +932,18 @@ func TestOAuth_GrantScopesFailReadRepository(t *testing.T) {
authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther)
authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0] authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0]
htmlDoc := NewHTMLParser(t, authorizeResp.Body)
grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"client_id": app.ClientID, "client_id": app.ClientID,
"redirect_uri": "a", "redirect_uri": "a",
"state": "thestate", "state": "thestate",
"granted": "true", "granted": "true",
}) })
grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest)
htmlDocGrant := NewHTMLParser(t, grantResp.Body) assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString)
accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"_csrf": htmlDocGrant.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"grant_type": "authorization_code", "grant_type": "authorization_code",
"client_id": app.ClientID, "client_id": app.ClientID,
"client_secret": app.ClientSecret, "client_secret": app.ClientSecret,
@ -1000,19 +1010,18 @@ func TestOAuth_GrantScopesReadRepository(t *testing.T) {
authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther)
authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0] authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0]
htmlDoc := NewHTMLParser(t, authorizeResp.Body)
grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"client_id": app.ClientID, "client_id": app.ClientID,
"redirect_uri": "a", "redirect_uri": "a",
"state": "thestate", "state": "thestate",
"granted": "true", "granted": "true",
}) })
grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest)
htmlDocGrant := NewHTMLParser(t, grantResp.Body) assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString)
accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"_csrf": htmlDocGrant.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"grant_type": "authorization_code", "grant_type": "authorization_code",
"client_id": app.ClientID, "client_id": app.ClientID,
"client_secret": app.ClientSecret, "client_secret": app.ClientSecret,
@ -1082,19 +1091,18 @@ func TestOAuth_GrantScopesReadPrivateGroups(t *testing.T) {
authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther)
authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0] authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0]
htmlDoc := NewHTMLParser(t, authorizeResp.Body)
grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"client_id": app.ClientID, "client_id": app.ClientID,
"redirect_uri": "a", "redirect_uri": "a",
"state": "thestate", "state": "thestate",
"granted": "true", "granted": "true",
}) })
grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest)
htmlDocGrant := NewHTMLParser(t, grantResp.Body) assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString)
accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"_csrf": htmlDocGrant.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"grant_type": "authorization_code", "grant_type": "authorization_code",
"client_id": app.ClientID, "client_id": app.ClientID,
"client_secret": app.ClientSecret, "client_secret": app.ClientSecret,
@ -1164,19 +1172,18 @@ func TestOAuth_GrantScopesReadOnlyPublicGroups(t *testing.T) {
authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther)
authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0] authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0]
htmlDoc := NewHTMLParser(t, authorizeResp.Body)
grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"client_id": app.ClientID, "client_id": app.ClientID,
"redirect_uri": "a", "redirect_uri": "a",
"state": "thestate", "state": "thestate",
"granted": "true", "granted": "true",
}) })
grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest)
htmlDocGrant := NewHTMLParser(t, grantResp.Body) assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString)
accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"_csrf": htmlDocGrant.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"grant_type": "authorization_code", "grant_type": "authorization_code",
"client_id": app.ClientID, "client_id": app.ClientID,
"client_secret": app.ClientSecret, "client_secret": app.ClientSecret,
@ -1260,19 +1267,18 @@ func TestOAuth_GrantScopesReadPublicGroupsWithTheReadScope(t *testing.T) {
authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther)
authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0] authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&amp")[0]
htmlDoc := NewHTMLParser(t, authorizeResp.Body)
grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{
"_csrf": htmlDoc.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"client_id": app.ClientID, "client_id": app.ClientID,
"redirect_uri": "a", "redirect_uri": "a",
"state": "thestate", "state": "thestate",
"granted": "true", "granted": "true",
}) })
grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest)
htmlDocGrant := NewHTMLParser(t, grantResp.Body) assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString)
accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"_csrf": htmlDocGrant.GetCSRF(), "_csrf": requireCookieCSRF(t, authorizeResp),
"grant_type": "authorization_code", "grant_type": "authorization_code",
"client_id": app.ClientID, "client_id": app.ClientID,
"client_secret": app.ClientSecret, "client_secret": app.ClientSecret,