From 84ceaa98bd731431c7d3a7f65e59e7ad076a540f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 8 Apr 2022 13:21:05 +0800 Subject: [PATCH] Refactor CSRF protection modules, make sure CSRF tokens can be up-to-date. (#19337) Do a refactoring to the CSRF related code, remove most unnecessary functions. Parse the generated token's issue time, regenerate the token every a few minutes. --- integrations/csrf_test.go | 52 +++++++++ integrations/repo_topic_test.go | 1 + modules/context/auth.go | 2 +- modules/context/context.go | 10 +- modules/context/csrf.go | 188 ++++++++++--------------------- modules/context/xsrf.go | 68 +++++------ modules/context/xsrf_test.go | 20 ++-- modules/web/middleware/cookie.go | 13 +-- routers/web/auth/auth.go | 2 +- routers/web/auth/oauth.go | 2 +- 10 files changed, 166 insertions(+), 192 deletions(-) create mode 100644 integrations/csrf_test.go diff --git a/integrations/csrf_test.go b/integrations/csrf_test.go new file mode 100644 index 00000000000..5bfc97bbd13 --- /dev/null +++ b/integrations/csrf_test.go @@ -0,0 +1,52 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +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" + + "github.com/stretchr/testify/assert" +) + +func TestCsrfProtection(t *testing.T) { + defer prepareTestEnv(t)() + + // test web form csrf via form + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User) + session := loginUser(t, user.Name) + 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()), + ) + + // 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()), + ) +} diff --git a/integrations/repo_topic_test.go b/integrations/repo_topic_test.go index 146f90e710f..e049afdd7c1 100644 --- a/integrations/repo_topic_test.go +++ b/integrations/repo_topic_test.go @@ -10,6 +10,7 @@ import ( "testing" api "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/assert" ) diff --git a/modules/context/auth.go b/modules/context/auth.go index 1a46ab586a6..09c22954550 100644 --- a/modules/context/auth.go +++ b/modules/context/auth.go @@ -63,7 +63,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) { } if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" { - Validate(ctx, ctx.csrf) + ctx.csrf.Validate(ctx) if ctx.Written() { return } diff --git a/modules/context/context.go b/modules/context/context.go index 8ede3646a44..17e1673a360 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -59,7 +59,7 @@ type Context struct { Render Render translation.Locale Cache cache.Cache - csrf CSRF + csrf CSRFProtector Flash *middleware.Flash Session session.Store @@ -666,7 +666,9 @@ func Auth(authMethod auth.Method) func(*Context) { func Contexter() func(next http.Handler) http.Handler { rnd := templates.HTMLRenderer() csrfOpts := getCsrfOpts() - + if !setting.IsProd { + CsrfTokenRegenerationInterval = 5 * time.Second // in dev, re-generate the tokens more aggressively for debug purpose + } return func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { locale := middleware.Locale(resp, req) @@ -697,7 +699,7 @@ func Contexter() func(next http.Handler) http.Handler { ctx.Data["Context"] = &ctx ctx.Req = WithContext(req, &ctx) - ctx.csrf = Csrfer(csrfOpts, &ctx) + ctx.csrf = PrepareCSRFProtector(csrfOpts, &ctx) // Get flash. flashCookie := ctx.GetCookie("macaron_flash") @@ -755,7 +757,7 @@ func Contexter() func(next http.Handler) http.Handler { ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions) - ctx.Data["CsrfToken"] = html.EscapeString(ctx.csrf.GetToken()) + 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 diff --git a/modules/context/csrf.go b/modules/context/csrf.go index 4fc92705048..df775048cb6 100644 --- a/modules/context/csrf.go +++ b/modules/context/csrf.go @@ -31,29 +31,19 @@ import ( "code.gitea.io/gitea/modules/web/middleware" ) -// CSRF represents a CSRF service and is used to get the current token and validate a suspect token. -type CSRF interface { - // Return HTTP header to search for token. +// 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 - // Return form value to search for token. + // GetFormName returns form value to search for token. GetFormName() string - // Return cookie name to search for token. - GetCookieName() string - // Return cookie path - GetCookiePath() string - // Return the flag value used for the csrf token. - GetCookieHTTPOnly() bool - // Return cookie domain - GetCookieDomain() string - // Return the token. + // GetToken returns the token. GetToken() string - // Validate by token. - ValidToken(t string) bool - // Error replies to the request with a custom function when ValidToken fails. - Error(w http.ResponseWriter) + // Validate validates the token in http context. + Validate(ctx *Context) } -type csrf struct { +type csrfProtector struct { // Header name value for setting and getting csrf token. Header string // Form name value for setting and getting csrf token. @@ -72,56 +62,24 @@ type csrf struct { ID string // Secret used along with the unique id above to generate the Token. Secret string - // ErrorFunc is the custom function that replies to the request when ValidToken fails. - ErrorFunc func(w http.ResponseWriter) } // GetHeaderName returns the name of the HTTP header for csrf token. -func (c *csrf) GetHeaderName() string { +func (c *csrfProtector) GetHeaderName() string { return c.Header } // GetFormName returns the name of the form value for csrf token. -func (c *csrf) GetFormName() string { +func (c *csrfProtector) GetFormName() string { return c.Form } -// GetCookieName returns the name of the cookie for csrf token. -func (c *csrf) GetCookieName() string { - return c.Cookie -} - -// GetCookiePath returns the path of the cookie for csrf token. -func (c *csrf) GetCookiePath() string { - return c.CookiePath -} - -// GetCookieHTTPOnly returns the flag value used for the csrf token. -func (c *csrf) GetCookieHTTPOnly() bool { - return c.CookieHTTPOnly -} - -// GetCookieDomain returns the flag value used for the csrf token. -func (c *csrf) GetCookieDomain() string { - return c.CookieDomain -} - // GetToken returns the current token. This is typically used // to populate a hidden form in an HTML template. -func (c *csrf) GetToken() string { +func (c *csrfProtector) GetToken() string { return c.Token } -// ValidToken validates the passed token against the existing Secret and ID. -func (c *csrf) ValidToken(t string) bool { - return ValidToken(t, c.Secret, c.ID, "POST") -} - -// Error replies to the request when ValidToken fails. -func (c *csrf) Error(w http.ResponseWriter) { - c.ErrorFunc(w) -} - // CsrfOptions maintains options to manage behavior of Generate. type CsrfOptions struct { // The global secret value used to generate Tokens. @@ -143,7 +101,7 @@ type CsrfOptions struct { SessionKey string // oldSessionKey saves old value corresponding to SessionKey. oldSessionKey string - // If true, send token via X-CSRFToken header. + // If true, send token via X-Csrf-Token header. SetHeader bool // If true, send token via _csrf cookie. SetCookie bool @@ -151,20 +109,12 @@ type CsrfOptions struct { Secure bool // Disallow Origin appear in request header. Origin bool - // The function called when Validate fails. - ErrorFunc func(w http.ResponseWriter) - // Cookie life time. Default is 0 + // Cookie lifetime. Default is 0 CookieLifeTime int } -func prepareOptions(options []CsrfOptions) CsrfOptions { - var opt CsrfOptions - if len(options) > 0 { - opt = options[0] - } - - // Defaults. - if len(opt.Secret) == 0 { +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 @@ -172,36 +122,30 @@ func prepareOptions(options []CsrfOptions) CsrfOptions { } opt.Secret = base32.StdEncoding.EncodeToString(randBytes) } - if len(opt.Header) == 0 { - opt.Header = "X-CSRFToken" + if opt.Header == "" { + opt.Header = "X-Csrf-Token" } - if len(opt.Form) == 0 { + if opt.Form == "" { opt.Form = "_csrf" } - if len(opt.Cookie) == 0 { + if opt.Cookie == "" { opt.Cookie = "_csrf" } - if len(opt.CookiePath) == 0 { + if opt.CookiePath == "" { opt.CookiePath = "/" } - if len(opt.SessionKey) == 0 { + if opt.SessionKey == "" { opt.SessionKey = "uid" } opt.oldSessionKey = "_old_" + opt.SessionKey - if opt.ErrorFunc == nil { - opt.ErrorFunc = func(w http.ResponseWriter) { - http.Error(w, "Invalid csrf token.", http.StatusBadRequest) - } - } - return opt } -// Csrfer maps CSRF to each request. If this request is a Get request, it will generate a new token. +// 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 Csrfer(opt CsrfOptions, ctx *Context) CSRF { - opt = prepareOptions([]CsrfOptions{opt}) - x := &csrf{ +func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector { + opt = prepareDefaultCsrfOptions(opt) + x := &csrfProtector{ Secret: opt.Secret, Header: opt.Header, Form: opt.Form, @@ -209,7 +153,6 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF { CookieDomain: opt.CookieDomain, CookiePath: opt.CookiePath, CookieHTTPOnly: opt.CookieHTTPOnly, - ErrorFunc: opt.ErrorFunc, } if opt.Origin && len(ctx.Req.Header.Get("Origin")) > 0 { @@ -229,28 +172,31 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF { } } - needsNew := false oldUID := ctx.Session.Get(opt.oldSessionKey) - if oldUID == nil || oldUID.(string) != x.ID { - needsNew = true + uidChanged := oldUID == nil || oldUID.(string) != x.ID + cookieToken := ctx.GetCookie(opt.Cookie) + + needsNew := true + if uidChanged { _ = ctx.Session.Set(opt.oldSessionKey, x.ID) - } else { - // If cookie present, map existing token, else generate a new one. - if val := ctx.GetCookie(opt.Cookie); len(val) > 0 { - // FIXME: test coverage. - x.Token = val - } else { - needsNew = true + } 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 + needsNew = false + } } } if needsNew { // FIXME: actionId. - x.Token = GenerateToken(x.Secret, x.ID, "POST") + x.Token = GenerateCsrfToken(x.Secret, x.ID, "POST", time.Now()) if opt.SetCookie { var expires interface{} if opt.CookieLifeTime == 0 { - expires = time.Now().AddDate(0, 0, 1) + expires = time.Now().Add(CsrfTokenTimeout) } middleware.SetCookie(ctx.Resp, opt.Cookie, x.Token, opt.CookieLifeTime, @@ -270,47 +216,31 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF { return x } -// Validate should be used as a per route middleware. It attempts to get a token from a "X-CSRFToken" -// HTTP header and then a "_csrf" form value. If one of these is found, the token will be validated -// using ValidToken. If this validation fails, custom Error is sent in the reply. -// If neither a header or form value is found, http.StatusBadRequest is sent. -func Validate(ctx *Context, x CSRF) { - if token := ctx.Req.Header.Get(x.GetHeaderName()); len(token) > 0 { - if !x.ValidToken(token) { - // Delete the cookie - middleware.SetCookie(ctx.Resp, x.GetCookieName(), "", - -1, - x.GetCookiePath(), - x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too? - if middleware.IsAPIPath(ctx.Req) { - x.Error(ctx.Resp) - return - } +func (c *csrfProtector) validateToken(ctx *Context, token string) { + if !ValidCsrfToken(token, c.Secret, c.ID, "POST", time.Now()) { + middleware.DeleteCSRFCookie(ctx.Resp) + 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 + "/") } - return } - if token := ctx.Req.FormValue(x.GetFormName()); len(token) > 0 { - if !x.ValidToken(token) { - // Delete the cookie - middleware.SetCookie(ctx.Resp, x.GetCookieName(), "", - -1, - x.GetCookiePath(), - x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too? - if middleware.IsAPIPath(ctx.Req) { - x.Error(ctx.Resp) - return - } - ctx.Flash.Error(ctx.Tr("error.invalid_csrf")) - ctx.Redirect(setting.AppSubURL + "/") - } +} + +// 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. +func (c *csrfProtector) Validate(ctx *Context) { + if token := ctx.Req.Header.Get(c.GetHeaderName()); token != "" { + c.validateToken(ctx, token) return } - if middleware.IsAPIPath(ctx.Req) { - http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest) + if token := ctx.Req.FormValue(c.GetFormName()); token != "" { + c.validateToken(ctx, token) return } - ctx.Flash.Error(ctx.Tr("error.missing_csrf")) - ctx.Redirect(setting.AppSubURL + "/") + c.validateToken(ctx, "") // no csrf token, use an empty token to respond error } diff --git a/modules/context/xsrf.go b/modules/context/xsrf.go index 10e63a4180c..e3ecc82f6d7 100644 --- a/modules/context/xsrf.go +++ b/modules/context/xsrf.go @@ -28,69 +28,69 @@ import ( "time" ) -// Timeout represents the duration that XSRF tokens are valid. +// CsrfTokenTimeout represents the duration that XSRF tokens are valid. // It is exported so clients may set cookie timeouts that match generated tokens. -const Timeout = 24 * time.Hour +const CsrfTokenTimeout = 24 * time.Hour -// clean sanitizes a string for inclusion in a token by replacing all ":"s. -func clean(s string) string { - return strings.ReplaceAll(s, ":", "_") -} +// CsrfTokenRegenerationInterval is the interval between token generations, old tokens are still valid before CsrfTokenTimeout +var CsrfTokenRegenerationInterval = 10 * time.Minute -// GenerateToken returns a URL-safe secure XSRF token that expires in 24 hours. -// +var csrfTokenSep = []byte(":") + +// GenerateCsrfToken returns a URL-safe secure XSRF token that expires in CsrfTokenTimeout hours. // key is a secret key for your application. // userID is a unique identifier for the user. // actionID is the action the user is taking (e.g. POSTing to a particular path). -func GenerateToken(key, userID, actionID string) string { - return generateTokenAtTime(key, userID, actionID, time.Now()) -} - -// generateTokenAtTime is like Generate, but returns a token that expires 24 hours from now. -func generateTokenAtTime(key, userID, actionID string, now time.Time) string { +func GenerateCsrfToken(key, userID, actionID string, now time.Time) string { + nowUnixNano := now.UnixNano() + nowUnixNanoStr := strconv.FormatInt(nowUnixNano, 10) h := hmac.New(sha1.New, []byte(key)) - fmt.Fprintf(h, "%s:%s:%d", clean(userID), clean(actionID), now.UnixNano()) - tok := fmt.Sprintf("%s:%d", h.Sum(nil), now.UnixNano()) + h.Write([]byte(strings.ReplaceAll(userID, ":", "_"))) + h.Write(csrfTokenSep) + h.Write([]byte(strings.ReplaceAll(actionID, ":", "_"))) + h.Write(csrfTokenSep) + h.Write([]byte(nowUnixNanoStr)) + tok := fmt.Sprintf("%s:%s", h.Sum(nil), nowUnixNanoStr) return base64.RawURLEncoding.EncodeToString([]byte(tok)) } -// ValidToken returns true if token is a valid, unexpired token returned by Generate. -func ValidToken(token, key, userID, actionID string) bool { - return validTokenAtTime(token, key, userID, actionID, time.Now()) -} - -// validTokenAtTime is like Valid, but it uses now to check if the token is expired. -func validTokenAtTime(token, key, userID, actionID string, now time.Time) bool { - // Decode the token. +func ParseCsrfToken(token string) (issueTime time.Time, ok bool) { data, err := base64.RawURLEncoding.DecodeString(token) if err != nil { - return false + return time.Time{}, false } - // Extract the issue time of the token. - sep := bytes.LastIndex(data, []byte{':'}) - if sep < 0 { - return false + pos := bytes.LastIndex(data, csrfTokenSep) + if pos == -1 { + return time.Time{}, false } - nanos, err := strconv.ParseInt(string(data[sep+1:]), 10, 64) + nanos, err := strconv.ParseInt(string(data[pos+1:]), 10, 64) if err != nil { + return time.Time{}, false + } + return time.Unix(0, nanos), true +} + +// ValidCsrfToken returns true if token is a valid and unexpired token returned by Generate. +func ValidCsrfToken(token, key, userID, actionID string, now time.Time) bool { + issueTime, ok := ParseCsrfToken(token) + if !ok { return false } - issueTime := time.Unix(0, nanos) // Check that the token is not expired. - if now.Sub(issueTime) >= Timeout { + if now.Sub(issueTime) >= CsrfTokenTimeout { return false } // Check that the token is not from the future. - // Allow 1 minute grace period in case the token is being verified on a + // Allow 1-minute grace period in case the token is being verified on a // machine whose clock is behind the machine that issued the token. if issueTime.After(now.Add(1 * time.Minute)) { return false } - expected := generateTokenAtTime(key, userID, actionID, issueTime) + expected := GenerateCsrfToken(key, userID, actionID, issueTime) // Check that the token matches the expected value. // Use constant time comparison to avoid timing attacks. diff --git a/modules/context/xsrf_test.go b/modules/context/xsrf_test.go index c0c711bf07c..ef42d61d5a8 100644 --- a/modules/context/xsrf_test.go +++ b/modules/context/xsrf_test.go @@ -37,18 +37,18 @@ var ( func Test_ValidToken(t *testing.T) { t.Run("Validate token", func(t *testing.T) { - tok := generateTokenAtTime(key, userID, actionID, now) - assert.True(t, validTokenAtTime(tok, key, userID, actionID, oneMinuteFromNow)) - assert.True(t, validTokenAtTime(tok, key, userID, actionID, now.Add(Timeout-1*time.Nanosecond))) - assert.True(t, validTokenAtTime(tok, key, userID, actionID, now.Add(-1*time.Minute))) + tok := GenerateCsrfToken(key, userID, actionID, now) + assert.True(t, ValidCsrfToken(tok, key, userID, actionID, oneMinuteFromNow)) + assert.True(t, ValidCsrfToken(tok, key, userID, actionID, now.Add(CsrfTokenTimeout-1*time.Nanosecond))) + assert.True(t, ValidCsrfToken(tok, key, userID, actionID, now.Add(-1*time.Minute))) }) } // Test_SeparatorReplacement tests that separators are being correctly substituted func Test_SeparatorReplacement(t *testing.T) { t.Run("Test two separator replacements", func(t *testing.T) { - assert.NotEqual(t, generateTokenAtTime("foo:bar", "baz", "wah", now), - generateTokenAtTime("foo", "bar:baz", "wah", now)) + assert.NotEqual(t, GenerateCsrfToken("foo:bar", "baz", "wah", now), + GenerateCsrfToken("foo", "bar:baz", "wah", now)) }) } @@ -61,13 +61,13 @@ func Test_InvalidToken(t *testing.T) { {"Bad key", "foobar", userID, actionID, oneMinuteFromNow}, {"Bad userID", key, "foobar", actionID, oneMinuteFromNow}, {"Bad actionID", key, userID, "foobar", oneMinuteFromNow}, - {"Expired", key, userID, actionID, now.Add(Timeout)}, + {"Expired", key, userID, actionID, now.Add(CsrfTokenTimeout)}, {"More than 1 minute from the future", key, userID, actionID, now.Add(-1*time.Nanosecond - 1*time.Minute)}, } - tok := generateTokenAtTime(key, userID, actionID, now) + tok := GenerateCsrfToken(key, userID, actionID, now) for _, itt := range invalidTokenTests { - assert.False(t, validTokenAtTime(tok, itt.key, itt.userID, itt.actionID, itt.t)) + assert.False(t, ValidCsrfToken(tok, itt.key, itt.userID, itt.actionID, itt.t)) } }) } @@ -84,7 +84,7 @@ func Test_ValidateBadData(t *testing.T) { } for _, bdt := range badDataTests { - assert.False(t, validTokenAtTime(bdt.tok, key, userID, actionID, oneMinuteFromNow)) + assert.False(t, ValidCsrfToken(bdt.tok, key, userID, actionID, oneMinuteFromNow)) } }) } diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go index 80fe3021373..b5904d6713a 100644 --- a/modules/web/middleware/cookie.go +++ b/modules/web/middleware/cookie.go @@ -98,17 +98,6 @@ func DeleteRedirectToCookie(resp http.ResponseWriter) { SameSite(setting.SessionConfig.SameSite)) } -// DeleteSesionConfigPathCookie convenience function to delete SessionConfigPath cookies consistently -func DeleteSesionConfigPathCookie(resp http.ResponseWriter, name string) { - SetCookie(resp, name, "", - -1, - setting.SessionConfig.CookiePath, - setting.SessionConfig.Domain, - setting.SessionConfig.Secure, - true, - SameSite(setting.SessionConfig.SameSite)) -} - // DeleteCSRFCookie convenience function to delete SessionConfigPath cookies consistently func DeleteCSRFCookie(resp http.ResponseWriter) { SetCookie(resp, setting.CSRFCookieName, "", @@ -117,7 +106,7 @@ func DeleteCSRFCookie(resp http.ResponseWriter) { setting.SessionConfig.Domain) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too? } -// SetCookie set the cookies +// SetCookie set the cookies. (name, value, lifetime, path, domain, secure, httponly, expires, {sameSite, ...}) // TODO: Copied from gitea.com/macaron/macaron and should be improved after macaron removed. func SetCookie(resp http.ResponseWriter, name, value string, others ...interface{}) { cookie := http.Cookie{} diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index ab538f0e5f6..c82fde49eb2 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -345,7 +345,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req) } - // Clear whatever CSRF has right now, force to generate a new one + // Clear whatever CSRF cookie has right now, force to generate a new one middleware.DeleteCSRFCookie(ctx.Resp) // Register last login diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 4369c333ac0..12de208ad7b 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1007,7 +1007,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model log.Error("Error storing session: %v", err) } - // Clear whatever CSRF has right now, force to generate a new one + // Clear whatever CSRF cookie has right now, force to generate a new one middleware.DeleteCSRFCookie(ctx.Resp) // Register last login