From 5b9557aef59b190c55de9ea218bf51152bc04786 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 14 Apr 2023 03:45:33 +0800 Subject: [PATCH] Refactor cookie (#24107) Close #24062 At the beginning, I just wanted to fix the warning mentioned by #24062 But, the cookie code really doesn't look good to me, so clean up them. Complete the TODO on `SetCookie`: > TODO: Copied from gitea.com/macaron/macaron and should be improved after macaron removed. --- modules/context/auth.go | 4 +- modules/context/context.go | 100 +++++--------- modules/context/csrf.go | 79 +++++------ modules/setting/session.go | 4 +- modules/web/middleware/cookie.go | 192 ++++---------------------- modules/web/middleware/locale.go | 18 +-- routers/install/install.go | 2 +- routers/web/auth/auth.go | 23 ++- routers/web/auth/oauth.go | 4 +- routers/web/auth/openid.go | 2 +- routers/web/auth/password.go | 2 +- routers/web/home.go | 2 +- services/auth/auth.go | 5 +- services/auth/sspi_windows.go | 16 +-- tests/integration/editor_test.go | 5 +- tests/integration/git_test.go | 3 +- tests/integration/integration_test.go | 3 +- tests/integration/mirror_push_test.go | 5 +- 18 files changed, 141 insertions(+), 328 deletions(-) diff --git a/modules/context/auth.go b/modules/context/auth.go index 7cc29debbd..5e071b4fab 100644 --- a/modules/context/auth.go +++ b/modules/context/auth.go @@ -67,7 +67,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) { } if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" { - ctx.csrf.Validate(ctx) + ctx.Csrf.Validate(ctx) if ctx.Written() { return } @@ -89,7 +89,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) { // Redirect to log in page if auto-signin info is provided and has not signed in. if !options.SignOutRequired && !ctx.IsSigned && - len(ctx.GetCookie(setting.CookieUserName)) > 0 { + len(ctx.GetSiteCookie(setting.CookieUserName)) > 0 { if ctx.Req.URL.Path != "/user/events" { middleware.SetRedirectToCookie(ctx.Resp, setting.AppSubURL+ctx.Req.URL.RequestURI()) } diff --git a/modules/context/context.go b/modules/context/context.go index e2e120ba38..cee533e42a 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -45,6 +45,8 @@ import ( "golang.org/x/crypto/pbkdf2" ) +const CookieNameFlash = "gitea_flash" + // Render represents a template render type Render interface { TemplateLookup(tmpl string) (*template.Template, error) @@ -60,7 +62,7 @@ type Context struct { Render Render translation.Locale Cache cache.Cache - csrf CSRFProtector + Csrf CSRFProtector Flash *middleware.Flash Session session.Store @@ -478,38 +480,26 @@ func (ctx *Context) Redirect(location string, status ...int) { http.Redirect(ctx.Resp, ctx.Req, location, code) } -// SetCookie convenience function to set most cookies consistently +// SetSiteCookie convenience function to set most cookies consistently // CSRF and a few others are the exception here -func (ctx *Context) SetCookie(name, value string, expiry int) { - middleware.SetCookie(ctx.Resp, name, value, - expiry, - setting.AppSubURL, - setting.SessionConfig.Domain, - setting.SessionConfig.Secure, - true, - middleware.SameSite(setting.SessionConfig.SameSite)) +func (ctx *Context) SetSiteCookie(name, value string, maxAge int) { + middleware.SetSiteCookie(ctx.Resp, name, value, maxAge) } -// DeleteCookie convenience function to delete most cookies consistently +// DeleteSiteCookie convenience function to delete most cookies consistently // CSRF and a few others are the exception here -func (ctx *Context) DeleteCookie(name string) { - middleware.SetCookie(ctx.Resp, name, "", - -1, - setting.AppSubURL, - setting.SessionConfig.Domain, - setting.SessionConfig.Secure, - true, - middleware.SameSite(setting.SessionConfig.SameSite)) +func (ctx *Context) DeleteSiteCookie(name string) { + middleware.SetSiteCookie(ctx.Resp, name, "", -1) } -// GetCookie returns given cookie value from request header. -func (ctx *Context) GetCookie(name string) string { - return middleware.GetCookie(ctx.Req, name) +// GetSiteCookie returns given cookie value from request header. +func (ctx *Context) GetSiteCookie(name string) string { + return middleware.GetSiteCookie(ctx.Req, name) } // GetSuperSecureCookie returns given cookie value from request header with secret string. func (ctx *Context) GetSuperSecureCookie(secret, name string) (string, bool) { - val := ctx.GetCookie(name) + val := ctx.GetSiteCookie(name) return ctx.CookieDecrypt(secret, val) } @@ -530,10 +520,9 @@ func (ctx *Context) CookieDecrypt(secret, val string) (string, bool) { } // SetSuperSecureCookie sets given cookie value to response header with secret string. -func (ctx *Context) SetSuperSecureCookie(secret, name, value string, expiry int) { +func (ctx *Context) SetSuperSecureCookie(secret, name, value string, maxAge int) { text := ctx.CookieEncrypt(secret, value) - - ctx.SetCookie(name, text, expiry) + ctx.SetSiteCookie(name, text, maxAge) } // CookieEncrypt encrypts a given value using the provided secret @@ -549,19 +538,19 @@ func (ctx *Context) CookieEncrypt(secret, value string) string { // GetCookieInt returns cookie result in int type. func (ctx *Context) GetCookieInt(name string) int { - r, _ := strconv.Atoi(ctx.GetCookie(name)) + r, _ := strconv.Atoi(ctx.GetSiteCookie(name)) return r } // GetCookieInt64 returns cookie result in int64 type. func (ctx *Context) GetCookieInt64(name string) int64 { - r, _ := strconv.ParseInt(ctx.GetCookie(name), 10, 64) + r, _ := strconv.ParseInt(ctx.GetSiteCookie(name), 10, 64) return r } // GetCookieFloat64 returns cookie result in float64 type. func (ctx *Context) GetCookieFloat64(name string) float64 { - v, _ := strconv.ParseFloat(ctx.GetCookie(name), 64) + v, _ := strconv.ParseFloat(ctx.GetSiteCookie(name), 64) return v } @@ -659,7 +648,10 @@ func WithContext(req *http.Request, ctx *Context) *http.Request { // GetContext retrieves install context from request func GetContext(req *http.Request) *Context { - return req.Context().Value(contextKey).(*Context) + if ctx, ok := req.Context().Value(contextKey).(*Context); ok { + return ctx + } + return nil } // GetContextUser returns context user @@ -726,13 +718,13 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler { ctx.Data["Context"] = &ctx ctx.Req = WithContext(req, &ctx) - ctx.csrf = PrepareCSRFProtector(csrfOpts, &ctx) + ctx.Csrf = PrepareCSRFProtector(csrfOpts, &ctx) - // Get flash. - flashCookie := ctx.GetCookie("macaron_flash") - vals, _ := url.ParseQuery(flashCookie) - if len(vals) > 0 { - f := &middleware.Flash{ + // Get the last flash message from cookie + lastFlashCookie := middleware.GetSiteCookie(ctx.Req, CookieNameFlash) + if vals, _ := url.ParseQuery(lastFlashCookie); len(vals) > 0 { + // store last Flash message into the template data, to render it + ctx.Data["Flash"] = &middleware.Flash{ DataStore: &ctx, Values: vals, ErrorMsg: vals.Get("error"), @@ -740,40 +732,18 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler { InfoMsg: vals.Get("info"), WarningMsg: vals.Get("warning"), } - ctx.Data["Flash"] = f } - f := &middleware.Flash{ - DataStore: &ctx, - Values: url.Values{}, - ErrorMsg: "", - WarningMsg: "", - InfoMsg: "", - SuccessMsg: "", - } + // prepare an empty Flash message for current request + ctx.Flash = &middleware.Flash{DataStore: &ctx, Values: url.Values{}} ctx.Resp.Before(func(resp ResponseWriter) { - if flash := f.Encode(); len(flash) > 0 { - middleware.SetCookie(resp, "macaron_flash", flash, 0, - setting.SessionConfig.CookiePath, - middleware.Domain(setting.SessionConfig.Domain), - middleware.HTTPOnly(true), - middleware.Secure(setting.SessionConfig.Secure), - middleware.SameSite(setting.SessionConfig.SameSite), - ) - return + if val := ctx.Flash.Encode(); val != "" { + middleware.SetSiteCookie(ctx.Resp, CookieNameFlash, val, 0) + } else if lastFlashCookie != "" { + middleware.SetSiteCookie(ctx.Resp, CookieNameFlash, "", -1) } - - middleware.SetCookie(ctx.Resp, "macaron_flash", "", -1, - setting.SessionConfig.CookiePath, - middleware.Domain(setting.SessionConfig.Domain), - middleware.HTTPOnly(true), - middleware.Secure(setting.SessionConfig.Secure), - middleware.SameSite(setting.SessionConfig.SameSite), - ) }) - ctx.Flash = f - // If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid. if ctx.Req.Method == "POST" && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") { if err := ctx.Req.ParseMultipartForm(setting.Attachment.MaxSize << 20); err != nil && !strings.Contains(err.Error(), "EOF") { // 32MB max size @@ -785,7 +755,7 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler { httpcache.SetCacheControlInHeader(ctx.Resp.Header(), 0, "no-transform") ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions) - ctx.Data["CsrfToken"] = 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 6639a8b008..9b0dc2923b 100644 --- a/modules/context/csrf.go +++ b/modules/context/csrf.go @@ -42,37 +42,26 @@ type CSRFProtector interface { GetToken() string // Validate validates the token in http context. Validate(ctx *Context) + // DeleteCookie deletes the cookie + DeleteCookie(ctx *Context) } type csrfProtector struct { - // Header name value for setting and getting csrf token. - Header string - // Form name value for setting and getting csrf token. - Form string - // Cookie name value for setting and getting csrf token. - Cookie string - // Cookie domain - CookieDomain string - // Cookie path - CookiePath string - // Cookie HttpOnly flag value used for the csrf token. - CookieHTTPOnly bool + opt CsrfOptions // Token generated to pass via header, cookie, or hidden form value. Token string // This value must be unique per user. ID string - // Secret used along with the unique id above to generate the Token. - Secret string } // GetHeaderName returns the name of the HTTP header for csrf token. func (c *csrfProtector) GetHeaderName() string { - return c.Header + return c.opt.Header } // GetFormName returns the name of the form value for csrf token. func (c *csrfProtector) GetFormName() string { - return c.Form + return c.opt.Form } // GetToken returns the current token. This is typically used @@ -138,23 +127,32 @@ func prepareDefaultCsrfOptions(opt CsrfOptions) CsrfOptions { 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 { + return &http.Cookie{ + Name: c.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, + } +} + // 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{ - Secret: opt.Secret, - Header: opt.Header, - Form: opt.Form, - Cookie: opt.Cookie, - CookieDomain: opt.CookieDomain, - CookiePath: opt.CookiePath, - CookieHTTPOnly: opt.CookieHTTPOnly, - } + x := &csrfProtector{opt: opt} if opt.Origin && len(ctx.Req.Header.Get("Origin")) > 0 { return x @@ -175,7 +173,7 @@ func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector { oldUID := ctx.Session.Get(opt.oldSessionKey) uidChanged := oldUID == nil || oldUID.(string) != x.ID - cookieToken := ctx.GetCookie(opt.Cookie) + cookieToken := ctx.GetSiteCookie(opt.Cookie) needsNew := true if uidChanged { @@ -193,21 +191,10 @@ func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector { if needsNew { // FIXME: actionId. - x.Token = GenerateCsrfToken(x.Secret, x.ID, "POST", time.Now()) + x.Token = GenerateCsrfToken(x.opt.Secret, x.ID, "POST", time.Now()) if opt.SetCookie { - var expires interface{} - if opt.CookieLifeTime == 0 { - expires = time.Now().Add(CsrfTokenTimeout) - } - middleware.SetCookie(ctx.Resp, opt.Cookie, x.Token, - opt.CookieLifeTime, - opt.CookiePath, - opt.CookieDomain, - opt.Secure, - opt.CookieHTTPOnly, - expires, - middleware.SameSite(opt.SameSite), - ) + cookie := newCsrfCookie(x, x.Token) + ctx.Resp.Header().Add("Set-Cookie", cookie.String()) } } @@ -218,8 +205,8 @@ func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector { } func (c *csrfProtector) validateToken(ctx *Context, token string) { - if !ValidCsrfToken(token, c.Secret, c.ID, "POST", time.Now()) { - middleware.DeleteCSRFCookie(ctx.Resp) + 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) @@ -245,3 +232,11 @@ func (c *csrfProtector) Validate(ctx *Context) { } c.validateToken(ctx, "") // no csrf token, use an empty token to respond error } + +func (c *csrfProtector) DeleteCookie(ctx *Context) { + if c.opt.SetCookie { + cookie := newCsrfCookie(c, "") + cookie.MaxAge = -1 + ctx.Resp.Header().Add("Set-Cookie", cookie.String()) + } +} diff --git a/modules/setting/session.go b/modules/setting/session.go index b8498335d9..d0bc938973 100644 --- a/modules/setting/session.go +++ b/modules/setting/session.go @@ -21,7 +21,7 @@ var SessionConfig = struct { ProviderConfig string // Cookie name to save session ID. Default is "MacaronSession". CookieName string - // Cookie path to store. Default is "/". + // Cookie path to store. Default is "/". HINT: there was a bug, the old value doesn't have trailing slash, and could be empty "". CookiePath string // GC interval time in seconds. Default is 3600. Gclifetime int64 @@ -49,7 +49,7 @@ func loadSessionFrom(rootCfg ConfigProvider) { SessionConfig.ProviderConfig = path.Join(AppWorkPath, SessionConfig.ProviderConfig) } SessionConfig.CookieName = sec.Key("COOKIE_NAME").MustString("i_like_gitea") - SessionConfig.CookiePath = AppSubURL + SessionConfig.CookiePath = AppSubURL + "/" // there was a bug, old code only set CookePath=AppSubURL, no trailing slash SessionConfig.Secure = sec.Key("COOKIE_SECURE").MustBool(false) SessionConfig.Gclifetime = sec.Key("GC_INTERVAL_TIME").MustInt64(86400) SessionConfig.Maxlifetime = sec.Key("SESSION_LIFE_TIME").MustInt64(86400) diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go index 7c1aaf6daf..621640895b 100644 --- a/modules/web/middleware/cookie.go +++ b/modules/web/middleware/cookie.go @@ -7,184 +7,23 @@ package middleware import ( "net/http" "net/url" - "time" + "strings" "code.gitea.io/gitea/modules/setting" ) -// MaxAge sets the maximum age for a provided cookie -func MaxAge(maxAge int) func(*http.Cookie) { - return func(c *http.Cookie) { - c.MaxAge = maxAge - } -} - -// Path sets the path for a provided cookie -func Path(path string) func(*http.Cookie) { - return func(c *http.Cookie) { - c.Path = path - } -} - -// Domain sets the domain for a provided cookie -func Domain(domain string) func(*http.Cookie) { - return func(c *http.Cookie) { - c.Domain = domain - } -} - -// Secure sets the secure setting for a provided cookie -func Secure(secure bool) func(*http.Cookie) { - return func(c *http.Cookie) { - c.Secure = secure - } -} - -// HTTPOnly sets the HttpOnly setting for a provided cookie -func HTTPOnly(httpOnly bool) func(*http.Cookie) { - return func(c *http.Cookie) { - c.HttpOnly = httpOnly - } -} - -// Expires sets the expires and rawexpires for a provided cookie -func Expires(expires time.Time) func(*http.Cookie) { - return func(c *http.Cookie) { - c.Expires = expires - c.RawExpires = expires.Format(time.UnixDate) - } -} - -// SameSite sets the SameSite for a provided cookie -func SameSite(sameSite http.SameSite) func(*http.Cookie) { - return func(c *http.Cookie) { - c.SameSite = sameSite - } -} - -// NewCookie creates a cookie -func NewCookie(name, value string, maxAge int) *http.Cookie { - return &http.Cookie{ - Name: name, - Value: value, - HttpOnly: true, - Path: setting.SessionConfig.CookiePath, - Domain: setting.SessionConfig.Domain, - MaxAge: maxAge, - Secure: setting.SessionConfig.Secure, - } -} - // SetRedirectToCookie convenience function to set the RedirectTo cookie consistently func SetRedirectToCookie(resp http.ResponseWriter, value string) { - SetCookie(resp, "redirect_to", value, - 0, - setting.AppSubURL, - "", - setting.SessionConfig.Secure, - true, - SameSite(setting.SessionConfig.SameSite)) + SetSiteCookie(resp, "redirect_to", value, 0) } // DeleteRedirectToCookie convenience function to delete most cookies consistently func DeleteRedirectToCookie(resp http.ResponseWriter) { - SetCookie(resp, "redirect_to", "", - -1, - setting.AppSubURL, - "", - setting.SessionConfig.Secure, - true, - SameSite(setting.SessionConfig.SameSite)) + SetSiteCookie(resp, "redirect_to", "", -1) } -// DeleteCSRFCookie convenience function to delete SessionConfigPath cookies consistently -func DeleteCSRFCookie(resp http.ResponseWriter) { - SetCookie(resp, setting.CSRFCookieName, "", - -1, - setting.SessionConfig.CookiePath, - setting.SessionConfig.Domain) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too? -} - -// 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{} - cookie.Name = name - cookie.Value = url.QueryEscape(value) - - if len(others) > 0 { - switch v := others[0].(type) { - case int: - cookie.MaxAge = v - case int64: - cookie.MaxAge = int(v) - case int32: - cookie.MaxAge = int(v) - case func(*http.Cookie): - v(&cookie) - } - } - - cookie.Path = "/" - if len(others) > 1 { - if v, ok := others[1].(string); ok && len(v) > 0 { - cookie.Path = v - } else if v, ok := others[1].(func(*http.Cookie)); ok { - v(&cookie) - } - } - - if len(others) > 2 { - if v, ok := others[2].(string); ok && len(v) > 0 { - cookie.Domain = v - } else if v, ok := others[2].(func(*http.Cookie)); ok { - v(&cookie) - } - } - - if len(others) > 3 { - switch v := others[3].(type) { - case bool: - cookie.Secure = v - case func(*http.Cookie): - v(&cookie) - default: - if others[3] != nil { - cookie.Secure = true - } - } - } - - if len(others) > 4 { - if v, ok := others[4].(bool); ok && v { - cookie.HttpOnly = true - } else if v, ok := others[4].(func(*http.Cookie)); ok { - v(&cookie) - } - } - - if len(others) > 5 { - if v, ok := others[5].(time.Time); ok { - cookie.Expires = v - cookie.RawExpires = v.Format(time.UnixDate) - } else if v, ok := others[5].(func(*http.Cookie)); ok { - v(&cookie) - } - } - - if len(others) > 6 { - for _, other := range others[6:] { - if v, ok := other.(func(*http.Cookie)); ok { - v(&cookie) - } - } - } - - resp.Header().Add("Set-Cookie", cookie.String()) -} - -// GetCookie returns given cookie value from request header. -func GetCookie(req *http.Request, name string) string { +// GetSiteCookie returns given cookie value from request header. +func GetSiteCookie(req *http.Request, name string) string { cookie, err := req.Cookie(name) if err != nil { return "" @@ -192,3 +31,24 @@ func GetCookie(req *http.Request, name string) string { val, _ := url.QueryUnescape(cookie.Value) return val } + +// SetSiteCookie returns given cookie value from request header. +func SetSiteCookie(resp http.ResponseWriter, name, value string, maxAge int) { + cookie := &http.Cookie{ + Name: name, + Value: url.QueryEscape(value), + MaxAge: maxAge, + Path: setting.SessionConfig.CookiePath, + Domain: setting.SessionConfig.Domain, + Secure: setting.SessionConfig.Secure, + HttpOnly: true, + SameSite: setting.SessionConfig.SameSite, + } + resp.Header().Add("Set-Cookie", cookie.String()) + if maxAge < 0 { + // There was a bug in "setting.SessionConfig.CookiePath" code, the old default value of it was empty "". + // So we have to delete the cookie on path="" again, because some old code leaves cookies on path="". + cookie.Path = strings.TrimSuffix(setting.SessionConfig.CookiePath, "/") + resp.Header().Add("Set-Cookie", cookie.String()) + } +} diff --git a/modules/web/middleware/locale.go b/modules/web/middleware/locale.go index f60be4bbdb..34a16f04e7 100644 --- a/modules/web/middleware/locale.go +++ b/modules/web/middleware/locale.go @@ -6,7 +6,6 @@ package middleware import ( "net/http" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/translation/i18n" @@ -49,23 +48,12 @@ func Locale(resp http.ResponseWriter, req *http.Request) translation.Locale { } // SetLocaleCookie convenience function to set the locale cookie consistently -func SetLocaleCookie(resp http.ResponseWriter, lang string, expiry int) { - SetCookie(resp, "lang", lang, expiry, - setting.AppSubURL, - setting.SessionConfig.Domain, - setting.SessionConfig.Secure, - true, - SameSite(setting.SessionConfig.SameSite)) +func SetLocaleCookie(resp http.ResponseWriter, lang string, maxAge int) { + SetSiteCookie(resp, "lang", lang, maxAge) } // DeleteLocaleCookie convenience function to delete the locale cookie consistently // Setting the lang cookie will trigger the middleware to reset the language to previous state. func DeleteLocaleCookie(resp http.ResponseWriter) { - SetCookie(resp, "lang", "", - -1, - setting.AppSubURL, - setting.SessionConfig.Domain, - setting.SessionConfig.Secure, - true, - SameSite(setting.SessionConfig.SameSite)) + SetSiteCookie(resp, "lang", "", -1) } diff --git a/routers/install/install.go b/routers/install/install.go index 8e2d19c732..8f8656230a 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -559,7 +559,7 @@ func SubmitInstall(ctx *context.Context) { } days := 86400 * setting.LogInRememberDays - ctx.SetCookie(setting.CookieUserName, u.Name, days) + ctx.SetSiteCookie(setting.CookieUserName, u.Name, days) ctx.SetSuperSecureCookie(base.EncodeMD5(u.Rands+u.Passwd), setting.CookieRememberName, u.Name, days) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 5fba632817..d8042afecc 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -49,7 +49,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { return false, nil } - uname := ctx.GetCookie(setting.CookieUserName) + uname := ctx.GetSiteCookie(setting.CookieUserName) if len(uname) == 0 { return false, nil } @@ -58,8 +58,8 @@ func AutoSignIn(ctx *context.Context) (bool, error) { defer func() { if !isSucceed { log.Trace("auto-login cookie cleared: %s", uname) - ctx.DeleteCookie(setting.CookieUserName) - ctx.DeleteCookie(setting.CookieRememberName) + ctx.DeleteSiteCookie(setting.CookieUserName) + ctx.DeleteSiteCookie(setting.CookieRememberName) } }() @@ -90,7 +90,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { return false, err } - middleware.DeleteCSRFCookie(ctx.Resp) + ctx.Csrf.DeleteCookie(ctx) return true, nil } @@ -125,7 +125,7 @@ func checkAutoLogin(ctx *context.Context) bool { if len(redirectTo) > 0 { middleware.SetRedirectToCookie(ctx.Resp, redirectTo) } else { - redirectTo = ctx.GetCookie("redirect_to") + redirectTo = ctx.GetSiteCookie("redirect_to") } if isSucceed { @@ -291,7 +291,7 @@ func handleSignIn(ctx *context.Context, u *user_model.User, remember bool) { func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRedirect bool) string { if remember { days := 86400 * setting.LogInRememberDays - ctx.SetCookie(setting.CookieUserName, u.Name, days) + ctx.SetSiteCookie(setting.CookieUserName, u.Name, days) ctx.SetSuperSecureCookie(base.EncodeMD5(u.Rands+u.Passwd), setting.CookieRememberName, u.Name, days) } @@ -330,7 +330,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe } // Clear whatever CSRF cookie has right now, force to generate a new one - middleware.DeleteCSRFCookie(ctx.Resp) + ctx.Csrf.DeleteCookie(ctx) // Register last login u.SetLastLogin() @@ -339,7 +339,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe return setting.AppSubURL + "/" } - if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { + if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { middleware.DeleteRedirectToCookie(ctx.Resp) if obeyRedirect { ctx.RedirectToFirst(redirectTo) @@ -368,10 +368,9 @@ func getUserName(gothUser *goth.User) string { func HandleSignOut(ctx *context.Context) { _ = ctx.Session.Flush() _ = ctx.Session.Destroy(ctx.Resp, ctx.Req) - ctx.DeleteCookie(setting.CookieUserName) - ctx.DeleteCookie(setting.CookieRememberName) - middleware.DeleteCSRFCookie(ctx.Resp) - middleware.DeleteLocaleCookie(ctx.Resp) + ctx.DeleteSiteCookie(setting.CookieUserName) + ctx.DeleteSiteCookie(setting.CookieRememberName) + ctx.Csrf.DeleteCookie(ctx) middleware.DeleteRedirectToCookie(ctx.Resp) } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index b3c4a234c1..287136e64c 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1112,7 +1112,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model } // Clear whatever CSRF cookie has right now, force to generate a new one - middleware.DeleteCSRFCookie(ctx.Resp) + ctx.Csrf.DeleteCookie(ctx) // Register last login u.SetLastLogin() @@ -1148,7 +1148,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model return } - if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 { + if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 { middleware.DeleteRedirectToCookie(ctx.Resp) ctx.RedirectToFirst(redirectTo) return diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index aff2e5f780..5e0e7b258f 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -47,7 +47,7 @@ func SignInOpenID(ctx *context.Context) { if len(redirectTo) > 0 { middleware.SetRedirectToCookie(ctx.Resp, redirectTo) } else { - redirectTo = ctx.GetCookie("redirect_to") + redirectTo = ctx.GetSiteCookie("redirect_to") } if isSucceed { diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go index a5aa9c5344..ed0412d745 100644 --- a/routers/web/auth/password.go +++ b/routers/web/auth/password.go @@ -336,7 +336,7 @@ func MustChangePasswordPost(ctx *context.Context) { log.Trace("User updated password: %s", u.Name) - if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { + if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { middleware.DeleteRedirectToCookie(ctx.Resp) ctx.RedirectToFirst(redirectTo) return diff --git a/routers/web/home.go b/routers/web/home.go index ecfecf6e67..b94e3e9eb5 100644 --- a/routers/web/home.go +++ b/routers/web/home.go @@ -54,7 +54,7 @@ func Home(ctx *context.Context) { } // Check auto-login. - uname := ctx.GetCookie(setting.CookieUserName) + uname := ctx.GetSiteCookie(setting.CookieUserName) if len(uname) != 0 { ctx.Redirect(setting.AppSubURL + "/user/login") return diff --git a/services/auth/auth.go b/services/auth/auth.go index 00e277c41a..905c776e58 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/webauthn" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" @@ -91,5 +92,7 @@ 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 - middleware.DeleteCSRFCookie(resp) + if ctx := gitea_context.GetContext(req); ctx != nil { + ctx.Csrf.DeleteCookie(ctx) + } } diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index b6e8d42980..176f4f574f 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -13,9 +13,9 @@ import ( "code.gitea.io/gitea/models/avatars" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/auth/source/sspi" @@ -46,9 +46,7 @@ var ( // via the built-in SSPI module in Windows for SPNEGO authentication. // On successful authentication returns a valid user object. // Returns nil if authentication fails. -type SSPI struct { - rnd *templates.HTMLRender -} +type SSPI struct{} // Init creates a new global websspi.Authenticator object func (s *SSPI) Init(ctx context.Context) error { @@ -58,7 +56,6 @@ func (s *SSPI) Init(ctx context.Context) error { if err != nil { return err } - _, s.rnd = templates.HTMLRenderer(ctx) return nil } @@ -101,12 +98,9 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, } store.GetData()["EnableOpenIDSignIn"] = setting.Service.EnableOpenIDSignIn store.GetData()["EnableSSPI"] = true - - err := s.rnd.HTML(w, http.StatusUnauthorized, string(tplSignIn), templates.BaseVars().Merge(store.GetData())) - if err != nil { - log.Error("%v", err) - } - + // in this case, the store is Gitea's web Context + // FIXME: it doesn't look good to render the page here, why not redirect? + store.(*gitea_context.Context).HTML(http.StatusUnauthorized, tplSignIn) return nil, err } if outToken != "" { diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index 791506d8f7..495290ed56 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -10,6 +10,7 @@ import ( "path" "testing" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/json" "github.com/stretchr/testify/assert" @@ -52,7 +53,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { }) session.MakeRequest(t, req, http.StatusSeeOther) // Check if master branch has been locked successfully - flashCookie := session.GetCookie("macaron_flash") + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) assert.NotNil(t, flashCookie) assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2527master%2527%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value) @@ -92,7 +93,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { assert.EqualValues(t, "/user2/repo1/settings/branches", res["redirect"]) // Check if master branch has been locked successfully - flashCookie = session.GetCookie("macaron_flash") + flashCookie = session.GetCookie(gitea_context.CookieNameFlash) assert.NotNil(t, flashCookie) assert.EqualValues(t, "error%3DRemoving%2Bbranch%2Bprotection%2Brule%2B%25271%2527%2Bfailed.", flashCookie.Value) }) diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index d21f3994a1..95c6d83e54 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -23,6 +23,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" @@ -436,7 +437,7 @@ func doProtectBranch(ctx APITestContext, branch, userToWhitelist, unprotectedFil ctx.Session.MakeRequest(t, req, http.StatusSeeOther) } // Check if master branch has been locked successfully - flashCookie := ctx.Session.GetCookie("macaron_flash") + flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash) assert.NotNil(t, flashCookie) assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Brule%2B%2527"+url.QueryEscape(branch)+"%2527%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value) } diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 716f6d44ab..965bae576c 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/unittest" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -290,7 +291,7 @@ func getTokenForLoggedInUser(t testing.TB, session *TestSession, scopes ...auth. // Log the flash values on failure if !assert.Equal(t, resp.Result().Header["Location"], []string{"/user/settings/applications"}) { for _, cookie := range resp.Result().Cookies() { - if cookie.Name != "macaron_flash" { + if cookie.Name != gitea_context.CookieNameFlash { continue } flash, _ := url.ParseQuery(cookie.Value) diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index b2ec6c0932..9abae63b0a 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -15,6 +15,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" @@ -91,7 +92,7 @@ func doCreatePushMirror(ctx APITestContext, address, username, password string) }) ctx.Session.MakeRequest(t, req, http.StatusSeeOther) - flashCookie := ctx.Session.GetCookie("macaron_flash") + flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash) assert.NotNil(t, flashCookie) assert.Contains(t, flashCookie.Value, "success") } @@ -112,7 +113,7 @@ func doRemovePushMirror(ctx APITestContext, address, username, password string, }) ctx.Session.MakeRequest(t, req, http.StatusSeeOther) - flashCookie := ctx.Session.GetCookie("macaron_flash") + flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash) assert.NotNil(t, flashCookie) assert.Contains(t, flashCookie.Value, "success") }