diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 951994253a..dc85d7c97c 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -222,8 +222,6 @@ string.desc = Z - A
[error]
occurred = An error occurred
report_message = If you believe that this is a Gitea bug, please search for issues on GitHub or open a new issue if necessary.
-missing_csrf = Bad Request: no CSRF token present
-invalid_csrf = Bad Request: invalid CSRF token
not_found = The target couldn't be found.
network_error = Network error
diff --git a/routers/web/web.go b/routers/web/web.go
index 41b019e4b5..f1e941a84e 100644
--- a/routers/web/web.go
+++ b/routers/web/web.go
@@ -129,6 +129,8 @@ func webAuth(authMethod auth_service.Method) func(*context.Context) {
// ensure the session uid is deleted
_ = ctx.Session.Delete("uid")
}
+
+ ctx.Csrf.PrepareForSessionUser(ctx)
}
}
diff --git a/services/context/context.go b/services/context/context.go
index 69b65cbddb..42f7c3d9d1 100644
--- a/services/context/context.go
+++ b/services/context/context.go
@@ -138,10 +138,8 @@ func Contexter() func(next http.Handler) http.Handler {
csrfOpts := CsrfOptions{
Secret: hex.EncodeToString(setting.GetGeneralTokenSigningSecret()),
Cookie: setting.CSRFCookieName,
- SetCookie: true,
Secure: setting.SessionConfig.Secure,
CookieHTTPOnly: setting.CSRFCookieHTTPOnly,
- Header: "X-Csrf-Token",
CookieDomain: setting.SessionConfig.Domain,
CookiePath: setting.SessionConfig.CookiePath,
SameSite: setting.SessionConfig.SameSite,
@@ -167,7 +165,7 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Base.AppendContextValue(WebContextKey, ctx)
ctx.Base.AppendContextValueFunc(gitrepo.RepositoryContextKey, func() any { return ctx.Repo.GitRepo })
- ctx.Csrf = PrepareCSRFProtector(csrfOpts, ctx)
+ ctx.Csrf = NewCSRFProtector(csrfOpts)
// Get the last flash message from cookie
lastFlashCookie := middleware.GetSiteCookie(ctx.Req, CookieNameFlash)
@@ -204,8 +202,6 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
ctx.Data["SystemConfig"] = setting.Config()
- ctx.Data["CsrfToken"] = ctx.Csrf.GetToken()
- ctx.Data["CsrfTokenHtml"] = template.HTML(``)
// FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these
ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations
diff --git a/services/context/csrf.go b/services/context/csrf.go
index 9b0dc2923b..9b66d613e3 100644
--- a/services/context/csrf.go
+++ b/services/context/csrf.go
@@ -20,64 +20,42 @@
package context
import (
- "encoding/base32"
- "fmt"
+ "html/template"
"net/http"
"strconv"
"time"
"code.gitea.io/gitea/modules/log"
- "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
- "code.gitea.io/gitea/modules/web/middleware"
+)
+
+const (
+ CsrfHeaderName = "X-Csrf-Token"
+ CsrfFormName = "_csrf"
)
// CSRFProtector represents a CSRF protector and is used to get the current token and validate the token.
type CSRFProtector interface {
- // GetHeaderName returns HTTP header to search for token.
- GetHeaderName() string
- // GetFormName returns form value to search for token.
- GetFormName() string
- // GetToken returns the token.
- GetToken() string
- // Validate validates the token in http context.
+ // PrepareForSessionUser prepares the csrf protector for the current session user.
+ PrepareForSessionUser(ctx *Context)
+ // Validate validates the csrf token in http context.
Validate(ctx *Context)
- // DeleteCookie deletes the cookie
+ // DeleteCookie deletes the csrf cookie
DeleteCookie(ctx *Context)
}
type csrfProtector struct {
opt CsrfOptions
- // Token generated to pass via header, cookie, or hidden form value.
- Token string
- // This value must be unique per user.
- ID string
-}
-
-// GetHeaderName returns the name of the HTTP header for csrf token.
-func (c *csrfProtector) GetHeaderName() string {
- return c.opt.Header
-}
-
-// GetFormName returns the name of the form value for csrf token.
-func (c *csrfProtector) GetFormName() string {
- return c.opt.Form
-}
-
-// GetToken returns the current token. This is typically used
-// to populate a hidden form in an HTML template.
-func (c *csrfProtector) GetToken() string {
- return c.Token
+ // id must be unique per user.
+ id string
+ // token is the valid one which wil be used by end user and passed via header, cookie, or hidden form value.
+ token string
}
// CsrfOptions maintains options to manage behavior of Generate.
type CsrfOptions struct {
// The global secret value used to generate Tokens.
Secret string
- // HTTP header used to set and get token.
- Header string
- // Form value used to set and get token.
- Form string
// Cookie value used to set and get token.
Cookie string
// Cookie domain.
@@ -87,103 +65,64 @@ type CsrfOptions struct {
CookieHTTPOnly bool
// SameSite set the cookie SameSite type
SameSite http.SameSite
- // Key used for getting the unique ID per user.
- SessionKey string
- // oldSessionKey saves old value corresponding to SessionKey.
- oldSessionKey string
- // If true, send token via X-Csrf-Token header.
- SetHeader bool
- // If true, send token via _csrf cookie.
- SetCookie bool
// Set the Secure flag to true on the cookie.
Secure bool
- // Disallow Origin appear in request header.
- Origin bool
- // Cookie lifetime. Default is 0
- CookieLifeTime int
+ // sessionKey is the key used for getting the unique ID per user.
+ sessionKey string
+ // oldSessionKey saves old value corresponding to sessionKey.
+ oldSessionKey string
}
-func prepareDefaultCsrfOptions(opt CsrfOptions) CsrfOptions {
- if opt.Secret == "" {
- randBytes, err := util.CryptoRandomBytes(8)
- if err != nil {
- // this panic can be handled by the recover() in http handlers
- panic(fmt.Errorf("failed to generate random bytes: %w", err))
- }
- opt.Secret = base32.StdEncoding.EncodeToString(randBytes)
- }
- if opt.Header == "" {
- opt.Header = "X-Csrf-Token"
- }
- if opt.Form == "" {
- opt.Form = "_csrf"
- }
- if opt.Cookie == "" {
- opt.Cookie = "_csrf"
- }
- if opt.CookiePath == "" {
- opt.CookiePath = "/"
- }
- if opt.SessionKey == "" {
- opt.SessionKey = "uid"
- }
- if opt.CookieLifeTime == 0 {
- opt.CookieLifeTime = int(CsrfTokenTimeout.Seconds())
- }
-
- opt.oldSessionKey = "_old_" + opt.SessionKey
- return opt
-}
-
-func newCsrfCookie(c *csrfProtector, value string) *http.Cookie {
+func newCsrfCookie(opt *CsrfOptions, value string) *http.Cookie {
return &http.Cookie{
- Name: c.opt.Cookie,
+ Name: opt.Cookie,
Value: value,
- Path: c.opt.CookiePath,
- Domain: c.opt.CookieDomain,
- MaxAge: c.opt.CookieLifeTime,
- Secure: c.opt.Secure,
- HttpOnly: c.opt.CookieHTTPOnly,
- SameSite: c.opt.SameSite,
+ Path: opt.CookiePath,
+ Domain: opt.CookieDomain,
+ MaxAge: int(CsrfTokenTimeout.Seconds()),
+ Secure: opt.Secure,
+ HttpOnly: opt.CookieHTTPOnly,
+ SameSite: opt.SameSite,
}
}
-// PrepareCSRFProtector returns a CSRFProtector to be used for every request.
-// Additionally, depending on options set, generated tokens will be sent via Header and/or Cookie.
-func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector {
- opt = prepareDefaultCsrfOptions(opt)
- x := &csrfProtector{opt: opt}
-
- if opt.Origin && len(ctx.Req.Header.Get("Origin")) > 0 {
- return x
+func NewCSRFProtector(opt CsrfOptions) CSRFProtector {
+ if opt.Secret == "" {
+ panic("CSRF secret is empty but it must be set") // it shouldn't happen because it is always set in code
}
+ opt.Cookie = util.IfZero(opt.Cookie, "_csrf")
+ opt.CookiePath = util.IfZero(opt.CookiePath, "/")
+ opt.sessionKey = "uid"
+ opt.oldSessionKey = "_old_" + opt.sessionKey
+ return &csrfProtector{opt: opt}
+}
- x.ID = "0"
- uidAny := ctx.Session.Get(opt.SessionKey)
- if uidAny != nil {
+func (c *csrfProtector) PrepareForSessionUser(ctx *Context) {
+ c.id = "0"
+ if uidAny := ctx.Session.Get(c.opt.sessionKey); uidAny != nil {
switch uidVal := uidAny.(type) {
case string:
- x.ID = uidVal
+ c.id = uidVal
case int64:
- x.ID = strconv.FormatInt(uidVal, 10)
+ c.id = strconv.FormatInt(uidVal, 10)
default:
log.Error("invalid uid type in session: %T", uidAny)
}
}
- oldUID := ctx.Session.Get(opt.oldSessionKey)
- uidChanged := oldUID == nil || oldUID.(string) != x.ID
- cookieToken := ctx.GetSiteCookie(opt.Cookie)
+ oldUID := ctx.Session.Get(c.opt.oldSessionKey)
+ uidChanged := oldUID == nil || oldUID.(string) != c.id
+ cookieToken := ctx.GetSiteCookie(c.opt.Cookie)
needsNew := true
if uidChanged {
- _ = ctx.Session.Set(opt.oldSessionKey, x.ID)
+ _ = ctx.Session.Set(c.opt.oldSessionKey, c.id)
} else if cookieToken != "" {
// If cookie token presents, re-use existing unexpired token, else generate a new one.
if issueTime, ok := ParseCsrfToken(cookieToken); ok {
dur := time.Since(issueTime) // issueTime is not a monotonic-clock, the server time may change a lot to an early time.
if dur >= -CsrfTokenRegenerationInterval && dur <= CsrfTokenRegenerationInterval {
- x.Token = cookieToken
+ c.token = cookieToken
needsNew = false
}
}
@@ -191,42 +130,33 @@ func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector {
if needsNew {
// FIXME: actionId.
- x.Token = GenerateCsrfToken(x.opt.Secret, x.ID, "POST", time.Now())
- if opt.SetCookie {
- cookie := newCsrfCookie(x, x.Token)
- ctx.Resp.Header().Add("Set-Cookie", cookie.String())
- }
+ 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())
}
- if opt.SetHeader {
- ctx.Resp.Header().Add(opt.Header, x.Token)
- }
- return x
+ ctx.Data["CsrfToken"] = c.token
+ ctx.Data["CsrfTokenHtml"] = template.HTML(``)
}
func (c *csrfProtector) validateToken(ctx *Context, token string) {
- if !ValidCsrfToken(token, c.opt.Secret, c.ID, "POST", time.Now()) {
+ if !ValidCsrfToken(token, c.opt.Secret, c.id, "POST", time.Now()) {
c.DeleteCookie(ctx)
- if middleware.IsAPIPath(ctx.Req) {
- // currently, there should be no access to the APIPath with CSRF token. because templates shouldn't use the `/api/` endpoints.
- http.Error(ctx.Resp, "Invalid CSRF token.", http.StatusBadRequest)
- } else {
- ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
- ctx.Redirect(setting.AppSubURL + "/")
- }
+ // 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)
}
}
// Validate should be used as a per route middleware. It attempts to get a token from an "X-Csrf-Token"
// HTTP header and then a "_csrf" form value. If one of these is found, the token will be validated.
-// If this validation fails, custom Error is sent in the reply.
-// If neither a header nor form value is found, http.StatusBadRequest is sent.
+// If this validation fails, http.StatusBadRequest is sent.
func (c *csrfProtector) Validate(ctx *Context) {
- if token := ctx.Req.Header.Get(c.GetHeaderName()); token != "" {
+ if token := ctx.Req.Header.Get(CsrfHeaderName); token != "" {
c.validateToken(ctx, token)
return
}
- if token := ctx.Req.FormValue(c.GetFormName()); token != "" {
+ if token := ctx.Req.FormValue(CsrfFormName); token != "" {
c.validateToken(ctx, token)
return
}
@@ -234,9 +164,7 @@ func (c *csrfProtector) Validate(ctx *Context) {
}
func (c *csrfProtector) DeleteCookie(ctx *Context) {
- if c.opt.SetCookie {
- cookie := newCsrfCookie(c, "")
- cookie.MaxAge = -1
- ctx.Resp.Header().Add("Set-Cookie", cookie.String())
- }
+ cookie := newCsrfCookie(&c.opt, "")
+ cookie.MaxAge = -1
+ ctx.Resp.Header().Add("Set-Cookie", cookie.String())
}
diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go
index 8206d8f4dc..40969d26f2 100644
--- a/tests/integration/attachment_test.go
+++ b/tests/integration/attachment_test.go
@@ -59,7 +59,8 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri
func TestCreateAnonymousAttachment(t *testing.T) {
defer tests.PrepareTestEnv(t)()
session := emptyTestSession(t)
- createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
+ // this test is not right because it just doesn't pass the CSRF validation
+ createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusBadRequest)
}
func TestCreateIssueAttachment(t *testing.T) {
diff --git a/tests/integration/csrf_test.go b/tests/integration/csrf_test.go
index a789859889..fcb9661b8a 100644
--- a/tests/integration/csrf_test.go
+++ b/tests/integration/csrf_test.go
@@ -5,12 +5,10 @@ package integration
import (
"net/http"
- "strings"
"testing"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
- "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
@@ -25,28 +23,12 @@ func TestCsrfProtection(t *testing.T) {
req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
"_csrf": "fake_csrf",
})
- session.MakeRequest(t, req, http.StatusSeeOther)
-
- resp := session.MakeRequest(t, req, http.StatusSeeOther)
- loc := resp.Header().Get("Location")
- assert.Equal(t, setting.AppSubURL+"/", loc)
- resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
- htmlDoc := NewHTMLParser(t, resp.Body)
- assert.Equal(t, "Bad Request: invalid CSRF token",
- strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
- )
+ resp := session.MakeRequest(t, req, http.StatusBadRequest)
+ assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
// test web form csrf via header. TODO: should use an UI api to test
req = NewRequest(t, "POST", "/user/settings")
req.Header.Add("X-Csrf-Token", "fake_csrf")
- session.MakeRequest(t, req, http.StatusSeeOther)
-
- resp = session.MakeRequest(t, req, http.StatusSeeOther)
- loc = resp.Header().Get("Location")
- assert.Equal(t, setting.AppSubURL+"/", loc)
- resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
- htmlDoc = NewHTMLParser(t, resp.Body)
- assert.Equal(t, "Bad Request: invalid CSRF token",
- strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
- )
+ resp = session.MakeRequest(t, req, http.StatusBadRequest)
+ assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
}
diff --git a/tests/integration/repo_branch_test.go b/tests/integration/repo_branch_test.go
index d1bc9198c3..f5217374b0 100644
--- a/tests/integration/repo_branch_test.go
+++ b/tests/integration/repo_branch_test.go
@@ -17,7 +17,6 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
- "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/translation"
@@ -146,15 +145,8 @@ func TestCreateBranchInvalidCSRF(t *testing.T) {
"_csrf": "fake_csrf",
"new_branch_name": "test",
})
- resp := session.MakeRequest(t, req, http.StatusSeeOther)
- loc := resp.Header().Get("Location")
- assert.Equal(t, setting.AppSubURL+"/", loc)
- resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
- htmlDoc := NewHTMLParser(t, resp.Body)
- assert.Equal(t,
- "Bad Request: invalid CSRF token",
- strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
- )
+ resp := session.MakeRequest(t, req, http.StatusBadRequest)
+ assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
}
func prepareBranch(t *testing.T, session *TestSession, repo *repo_model.Repository) {