From 961766744bb5b7bf138cb508c8f9afe69622d514 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Sun, 15 Sep 2024 00:40:36 +0800 Subject: [PATCH 01/12] Check if the `due_date` is nil when editing issues (#32035) (cherry picked from commit 3a51c37672d2fbad1f222922e75ce704d5a1ac71) --- routers/api/v1/repo/issue.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index afcfbc00e3..22779e38d2 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -839,10 +839,16 @@ func EditIssue(ctx *context.APIContext) { if (form.Deadline != nil || form.RemoveDeadline != nil) && canWrite { var deadlineUnix timeutil.TimeStamp - if (form.RemoveDeadline == nil || !*form.RemoveDeadline) && !form.Deadline.IsZero() { - deadline := time.Date(form.Deadline.Year(), form.Deadline.Month(), form.Deadline.Day(), - 23, 59, 59, 0, form.Deadline.Location()) - deadlineUnix = timeutil.TimeStamp(deadline.Unix()) + if form.RemoveDeadline == nil || !*form.RemoveDeadline { + if form.Deadline == nil { + ctx.Error(http.StatusBadRequest, "", "The due_date cannot be empty") + return + } + if !form.Deadline.IsZero() { + deadline := time.Date(form.Deadline.Year(), form.Deadline.Month(), form.Deadline.Day(), + 23, 59, 59, 0, form.Deadline.Location()) + deadlineUnix = timeutil.TimeStamp(deadline.Unix()) + } } if err := issues_model.UpdateIssueDeadline(ctx, issue, deadlineUnix, ctx.Doer); err != nil { From 0cafec4c7a2faf810953e9d522faf5dc019e1522 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Mon, 16 Sep 2024 23:10:33 +0200 Subject: [PATCH 02/12] Do not escape relative path in RPM primary index (#32038) Fixes #32021 Do not escape the relative path. (cherry picked from commit f528df944bb9436afcb9272add2ee0cccefbdb55) --- services/packages/rpm/repository.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/packages/rpm/repository.go b/services/packages/rpm/repository.go index 8a2db8670f..2cea04212a 100644 --- a/services/packages/rpm/repository.go +++ b/services/packages/rpm/repository.go @@ -13,7 +13,6 @@ import ( "errors" "fmt" "io" - "net/url" "strings" "time" @@ -440,7 +439,7 @@ func buildPrimary(ctx context.Context, pv *packages_model.PackageVersion, pfs [] Archive: pd.FileMetadata.ArchiveSize, }, Location: Location{ - Href: fmt.Sprintf("package/%s/%s/%s/%s", url.PathEscape(pd.Package.Name), url.PathEscape(packageVersion), url.PathEscape(pd.FileMetadata.Architecture), url.PathEscape(fmt.Sprintf("%s-%s.%s.rpm", pd.Package.Name, packageVersion, pd.FileMetadata.Architecture))), + Href: fmt.Sprintf("package/%s/%s/%s/%s-%s.%s.rpm", pd.Package.Name, packageVersion, pd.FileMetadata.Architecture, pd.Package.Name, packageVersion, pd.FileMetadata.Architecture), }, Format: Format{ License: pd.VersionMetadata.License, From 9d5f409a5a0ac784523cc94a6011f8c8a41d5a95 Mon Sep 17 00:00:00 2001 From: hiifong Date: Wed, 18 Sep 2024 03:02:48 +0800 Subject: [PATCH 03/12] Lazy load avatar images (#32051) (cherry picked from commit f38e1014483b84f4541ffb354cd5dfdd7e000e2c) --- modules/templates/util_avatar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/templates/util_avatar.go b/modules/templates/util_avatar.go index 85832cf264..afc1091516 100644 --- a/modules/templates/util_avatar.go +++ b/modules/templates/util_avatar.go @@ -34,7 +34,7 @@ func AvatarHTML(src string, size int, class, name string) template.HTML { name = "avatar" } - return template.HTML(``) + return template.HTML(``) } // Avatar renders user avatars. args: user, size (int), class (string) From 1bdf334844f398d0a2adafc4499cef15f95df6d8 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 22 Sep 2024 09:28:20 +0200 Subject: [PATCH 04/12] feat: add IfZero utility function (cherry picked from commit 43de021ac1ca017212ec75fd88a8a80a9db27c4c) --- modules/util/util.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/util/util.go b/modules/util/util.go index 0444680228..dcd7cf4f29 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -225,6 +225,15 @@ func Iif[T any](condition bool, trueVal, falseVal T) T { return falseVal } +// IfZero returns "def" if "v" is a zero value, otherwise "v" +func IfZero[T comparable](v, def T) T { + var zero T + if v == zero { + return def + } + return v +} + func ReserveLineBreakForTextarea(input string) string { // Since the content is from a form which is a textarea, the line endings are \r\n. // It's a standard behavior of HTML. From 1ae3b127fc74906f0722caf6486890cf32d23715 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 18 Sep 2024 15:17:25 +0800 Subject: [PATCH 05/12] Refactor CSRF protector (#32057) Remove unused CSRF options, decouple "new csrf protector" and "prepare" logic, do not redirect to home page if CSRF validation falis (it shouldn't happen in daily usage, if it happens, redirecting to home doesn't help either but just makes the problem more complex for "fetch") (cherry picked from commit 1fede04b83288d8a91304a83b7601699bb5cba04) Conflicts: options/locale/locale_en-US.ini tests/integration/repo_branch_test.go trivial context conflicts --- options/locale/locale_en-US.ini | 1 - routers/web/web.go | 2 + services/context/context.go | 6 +- services/context/csrf.go | 192 ++++++++------------------ tests/integration/attachment_test.go | 3 +- tests/integration/csrf_test.go | 26 +--- tests/integration/repo_branch_test.go | 12 +- 7 files changed, 71 insertions(+), 171 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1631c90ba2..2512fe8b55 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -231,7 +231,6 @@ string.desc = Z - A [error] occurred = An error occurred report_message = If you believe that this is a Forgejo bug, please search for issues on Codeberg or open a new issue if necessary. -invalid_csrf = Bad Request: invalid CSRF token not_found = The target couldn't be found. network_error = Network error server_internal = Internal server error diff --git a/routers/web/web.go b/routers/web/web.go index d174b4e251..39116b882d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -132,6 +132,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 c0819ab11e..91e7b1849d 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -127,10 +127,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, @@ -156,7 +154,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) @@ -193,8 +191,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 57c55e6550..5890d53f42 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, reuse 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 95c9c9f753..7cbc2545d5 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -60,7 +60,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 2aa299479a..df9ea9a97c 100644 --- a/tests/integration/repo_branch_test.go +++ b/tests/integration/repo_branch_test.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" repo_service "code.gitea.io/gitea/services/repository" @@ -157,15 +156,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 TestDatabaseMissingABranch(t *testing.T) { From 6275d1bc5000a09793574696cb18cc5b5c4b07d6 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 22 Sep 2024 12:57:03 +0200 Subject: [PATCH 06/12] Refactor CSRF protector (#32057) (fix forgejo tests) Fix the tests unique to Forgejo that are impacted by the refactor. --- services/context/csrf.go | 7 ++-- tests/integration/links_test.go | 24 +++++++++--- tests/integration/oauth_test.go | 66 ++++++++++++++++++--------------- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/services/context/csrf.go b/services/context/csrf.go index 5890d53f42..e0518a499b 100644 --- a/services/context/csrf.go +++ b/services/context/csrf.go @@ -30,8 +30,9 @@ import ( ) const ( - CsrfHeaderName = "X-Csrf-Token" - CsrfFormName = "_csrf" + CsrfHeaderName = "X-Csrf-Token" + CsrfFormName = "_csrf" + CsrfErrorString = "Invalid CSRF 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) // 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) + http.Error(ctx.Resp, CsrfErrorString, http.StatusBadRequest) } } diff --git a/tests/integration/links_test.go b/tests/integration/links_test.go index 68d7008e02..e9ad933b24 100644 --- a/tests/integration/links_test.go +++ b/tests/integration/links_test.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + forgejo_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" "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: "/admin/system-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 { 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) } } + + 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) { diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 0d5e9a0472..f385b99e46 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "net/http" + "net/http/httptest" "net/url" "strings" "testing" @@ -24,6 +25,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/routers/web/auth" + forgejo_context "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" "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) { defer tests.PrepareTestEnv(t)() @@ -840,19 +852,18 @@ func TestOAuth_GrantScopesReadUser(t *testing.T) { authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] - htmlDoc := NewHTMLParser(t, authorizeResp.Body) grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "client_id": app.ClientID, "redirect_uri": "a", "state": "thestate", "granted": "true", }) - grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) - htmlDocGrant := NewHTMLParser(t, grantResp.Body) + grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest) + assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString) accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "_csrf": htmlDocGrant.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "grant_type": "authorization_code", "client_id": app.ClientID, "client_secret": app.ClientSecret, @@ -921,19 +932,18 @@ func TestOAuth_GrantScopesFailReadRepository(t *testing.T) { authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] - htmlDoc := NewHTMLParser(t, authorizeResp.Body) grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "client_id": app.ClientID, "redirect_uri": "a", "state": "thestate", "granted": "true", }) - grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) - htmlDocGrant := NewHTMLParser(t, grantResp.Body) + grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest) + assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString) accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "_csrf": htmlDocGrant.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "grant_type": "authorization_code", "client_id": app.ClientID, "client_secret": app.ClientSecret, @@ -1000,19 +1010,18 @@ func TestOAuth_GrantScopesReadRepository(t *testing.T) { authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] - htmlDoc := NewHTMLParser(t, authorizeResp.Body) grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "client_id": app.ClientID, "redirect_uri": "a", "state": "thestate", "granted": "true", }) - grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) - htmlDocGrant := NewHTMLParser(t, grantResp.Body) + grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest) + assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString) accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "_csrf": htmlDocGrant.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "grant_type": "authorization_code", "client_id": app.ClientID, "client_secret": app.ClientSecret, @@ -1082,19 +1091,18 @@ func TestOAuth_GrantScopesReadPrivateGroups(t *testing.T) { authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] - htmlDoc := NewHTMLParser(t, authorizeResp.Body) grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "client_id": app.ClientID, "redirect_uri": "a", "state": "thestate", "granted": "true", }) - grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) - htmlDocGrant := NewHTMLParser(t, grantResp.Body) + grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest) + assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString) accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "_csrf": htmlDocGrant.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "grant_type": "authorization_code", "client_id": app.ClientID, "client_secret": app.ClientSecret, @@ -1164,19 +1172,18 @@ func TestOAuth_GrantScopesReadOnlyPublicGroups(t *testing.T) { authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] - htmlDoc := NewHTMLParser(t, authorizeResp.Body) grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "client_id": app.ClientID, "redirect_uri": "a", "state": "thestate", "granted": "true", }) - grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) - htmlDocGrant := NewHTMLParser(t, grantResp.Body) + grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest) + assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString) accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "_csrf": htmlDocGrant.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "grant_type": "authorization_code", "client_id": app.ClientID, "client_secret": app.ClientSecret, @@ -1260,19 +1267,18 @@ func TestOAuth_GrantScopesReadPublicGroupsWithTheReadScope(t *testing.T) { authorizeResp := ctx.MakeRequest(t, authorizeReq, http.StatusSeeOther) authcode := strings.Split(strings.Split(authorizeResp.Body.String(), "?code=")[1], "&")[0] - htmlDoc := NewHTMLParser(t, authorizeResp.Body) grantReq := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ - "_csrf": htmlDoc.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "client_id": app.ClientID, "redirect_uri": "a", "state": "thestate", "granted": "true", }) - grantResp := ctx.MakeRequest(t, grantReq, http.StatusSeeOther) - htmlDocGrant := NewHTMLParser(t, grantResp.Body) + grantResp := ctx.MakeRequest(t, grantReq, http.StatusBadRequest) + assert.NotContains(t, grantResp.Body.String(), forgejo_context.CsrfErrorString) accessTokenReq := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ - "_csrf": htmlDocGrant.GetCSRF(), + "_csrf": requireCookieCSRF(t, authorizeResp), "grant_type": "authorization_code", "client_id": app.ClientID, "client_secret": app.ClientSecret, From 526054332acb221e061d3900bba2dc6e012da52d Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 20 Sep 2024 21:00:39 +0200 Subject: [PATCH 07/12] Fix incorrect `/tokens` api (#32085) Fixes #32078 - Add missing scopes output. - Disallow empty scope. --------- Co-authored-by: Lunny Xiao (cherry picked from commit 08adbc468f8875fd4763c3656b334203c11adc0a) --- routers/api/v1/user/app.go | 5 +++++ tests/integration/api_token_test.go | 31 ++++++++++------------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go index b61ebac7d0..d5b20f7703 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -118,6 +118,10 @@ func CreateAccessToken(ctx *context.APIContext) { ctx.Error(http.StatusBadRequest, "AccessTokenScope.Normalize", fmt.Errorf("invalid access token scope provided: %w", err)) return } + if scope == "" { + ctx.Error(http.StatusBadRequest, "AccessTokenScope", "access token must have a scope") + return + } t.Scope = scope if err := auth_model.NewAccessToken(ctx, t); err != nil { @@ -129,6 +133,7 @@ func CreateAccessToken(ctx *context.APIContext) { Token: t.Token, ID: t.ID, TokenLastEight: t.TokenLastEight, + Scopes: t.Scope.StringSlice(), }) } diff --git a/tests/integration/api_token_test.go b/tests/integration/api_token_test.go index 9c7bf37330..01d18ef6f1 100644 --- a/tests/integration/api_token_test.go +++ b/tests/integration/api_token_test.go @@ -23,10 +23,10 @@ func TestAPICreateAndDeleteToken(t *testing.T) { defer tests.PrepareTestEnv(t)() user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - newAccessToken := createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user, nil) + newAccessToken := createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll}) deleteAPIAccessToken(t, newAccessToken, user) - newAccessToken = createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user, nil) + newAccessToken = createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll}) deleteAPIAccessToken(t, newAccessToken, user) } @@ -72,19 +72,19 @@ func TestAPIDeleteTokensPermission(t *testing.T) { user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // admin can delete tokens for other users - createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user2, nil) + createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll}) req := NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-1"). AddBasicAuth(admin.Name) MakeRequest(t, req, http.StatusNoContent) // non-admin can delete tokens for himself - createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user2, nil) + createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll}) req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-2"). AddBasicAuth(user2.Name) MakeRequest(t, req, http.StatusNoContent) // non-admin can't delete tokens for other users - createAPIAccessTokenWithoutCleanUp(t, "test-key-3", user2, nil) + createAPIAccessTokenWithoutCleanUp(t, "test-key-3", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll}) req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-3"). AddBasicAuth(user4.Name) MakeRequest(t, req, http.StatusForbidden) @@ -520,7 +520,7 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model unauthorizedScopes = append(unauthorizedScopes, cateogoryUnauthorizedScopes...) } - accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, &unauthorizedScopes) + accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, unauthorizedScopes) defer deleteAPIAccessToken(t, accessToken, user) // Request the endpoint. Verify that permission is denied. @@ -532,20 +532,12 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model // createAPIAccessTokenWithoutCleanUp Create an API access token and assert that // creation succeeded. The caller is responsible for deleting the token. -func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *user_model.User, scopes *[]auth_model.AccessTokenScope) api.AccessToken { +func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *user_model.User, scopes []auth_model.AccessTokenScope) api.AccessToken { payload := map[string]any{ - "name": tokenName, - } - if scopes != nil { - for _, scope := range *scopes { - scopes, scopesExists := payload["scopes"].([]string) - if !scopesExists { - scopes = make([]string, 0) - } - scopes = append(scopes, string(scope)) - payload["scopes"] = scopes - } + "name": tokenName, + "scopes": scopes, } + log.Debug("Requesting creation of token with scopes: %v", scopes) req := NewRequestWithJSON(t, "POST", "/api/v1/users/"+user.LoginName+"/tokens", payload). AddBasicAuth(user.Name) @@ -563,8 +555,7 @@ func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *us return newAccessToken } -// createAPIAccessTokenWithoutCleanUp Delete an API access token and assert that -// deletion succeeded. +// deleteAPIAccessToken deletes an API access token and assert that deletion succeeded. func deleteAPIAccessToken(t *testing.T, accessToken api.AccessToken, user *user_model.User) { req := NewRequestf(t, "DELETE", "/api/v1/users/"+user.LoginName+"/tokens/%d", accessToken.ID). AddBasicAuth(user.Name) From 2675a24649af2fff34f5c7e416d6ff78591d8d9c Mon Sep 17 00:00:00 2001 From: Timon van der Berg Date: Sat, 21 Sep 2024 20:57:01 +0200 Subject: [PATCH 08/12] Repo Activity: count new issues that were closed (#31776) I'm new to go and contributing to gitea, your guidance is much appreciated. This is meant to solve https://github.com/go-gitea/gitea/issues/13309 Previously, closed issues would not be shown under new issues in the activity tab, even if they were newly created. changes: * Split out newlyCreatedIssues from issuesForActivityStatement to count both currently open and closed issues. * Use a seperate function to count active issues to prevent double-counting issues after the above change. Result is that new issues that have been closed are shown both under "new" and "closed". Signed-off-by: Timon van der Berg (cherry picked from commit ebfde845294cc681de6b1fe1adcf27e35f61b89b) --- models/activities/repo_activity.go | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/models/activities/repo_activity.go b/models/activities/repo_activity.go index ba5e4959f0..3ffad035b7 100644 --- a/models/activities/repo_activity.go +++ b/models/activities/repo_activity.go @@ -34,6 +34,7 @@ type ActivityStats struct { OpenedPRAuthorCount int64 MergedPRs issues_model.PullRequestList MergedPRAuthorCount int64 + ActiveIssues issues_model.IssueList OpenedIssues issues_model.IssueList OpenedIssueAuthorCount int64 ClosedIssues issues_model.IssueList @@ -172,7 +173,7 @@ func (stats *ActivityStats) MergedPRPerc() int { // ActiveIssueCount returns total active issue count func (stats *ActivityStats) ActiveIssueCount() int { - return stats.OpenedIssueCount() + stats.ClosedIssueCount() + return len(stats.ActiveIssues) } // OpenedIssueCount returns open issue count @@ -285,13 +286,21 @@ func (stats *ActivityStats) FillIssues(ctx context.Context, repoID int64, fromTi stats.ClosedIssueAuthorCount = count // New issues - sess = issuesForActivityStatement(ctx, repoID, fromTime, false, false) + sess = newlyCreatedIssues(ctx, repoID, fromTime) sess.OrderBy("issue.created_unix ASC") stats.OpenedIssues = make(issues_model.IssueList, 0) if err = sess.Find(&stats.OpenedIssues); err != nil { return err } + // Active issues + sess = activeIssues(ctx, repoID, fromTime) + sess.OrderBy("issue.created_unix ASC") + stats.ActiveIssues = make(issues_model.IssueList, 0) + if err = sess.Find(&stats.ActiveIssues); err != nil { + return err + } + // Opened issue authors sess = issuesForActivityStatement(ctx, repoID, fromTime, false, false) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("issue").Get(&count); err != nil { @@ -317,6 +326,23 @@ func (stats *ActivityStats) FillUnresolvedIssues(ctx context.Context, repoID int return sess.Find(&stats.UnresolvedIssues) } +func newlyCreatedIssues(ctx context.Context, repoID int64, fromTime time.Time) *xorm.Session { + sess := db.GetEngine(ctx).Where("issue.repo_id = ?", repoID). + And("issue.is_pull = ?", false). // Retain the is_pull check to exclude pull requests + And("issue.created_unix >= ?", fromTime.Unix()) // Include all issues created after fromTime + + return sess +} + +func activeIssues(ctx context.Context, repoID int64, fromTime time.Time) *xorm.Session { + sess := db.GetEngine(ctx).Where("issue.repo_id = ?", repoID). + And("issue.is_pull = ?", false). + And("issue.created_unix >= ?", fromTime.Unix()). + Or("issue.closed_unix >= ?", fromTime.Unix()) + + return sess +} + func issuesForActivityStatement(ctx context.Context, repoID int64, fromTime time.Time, closed, unresolved bool) *xorm.Session { sess := db.GetEngine(ctx).Where("issue.repo_id = ?", repoID). And("issue.is_closed = ?", closed) From f709de24039ab7e605d3e09e3b61240836381603 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 22 Sep 2024 05:56:25 +0800 Subject: [PATCH 09/12] Fix wrong last modify time (#32102) (cherry picked from commit a802508f88e546bf18990559e44bf27a09c869ee) --- modules/httpcache/httpcache.go | 3 ++- modules/httplib/serve.go | 1 + routers/api/packages/maven/maven.go | 4 +++- routers/web/repo/githttp.go | 3 ++- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/httpcache/httpcache.go b/modules/httpcache/httpcache.go index b4af371541..30ce0a4a03 100644 --- a/modules/httpcache/httpcache.go +++ b/modules/httpcache/httpcache.go @@ -76,7 +76,8 @@ func HandleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag s w.Header().Set("Etag", etag) } if lastModified != nil && !lastModified.IsZero() { - w.Header().Set("Last-Modified", lastModified.Format(http.TimeFormat)) + // http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat + w.Header().Set("Last-Modified", lastModified.UTC().Format(http.TimeFormat)) } if len(etag) > 0 { diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index 6e147d76f5..2e3e6a7c42 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -79,6 +79,7 @@ func ServeSetHeaders(w http.ResponseWriter, opts *ServeHeaderOptions) { httpcache.SetCacheControlInHeader(header, duration) if !opts.LastModified.IsZero() { + // http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat header.Set("Last-Modified", opts.LastModified.UTC().Format(http.TimeFormat)) } } diff --git a/routers/api/packages/maven/maven.go b/routers/api/packages/maven/maven.go index 58271e1d43..4181577454 100644 --- a/routers/api/packages/maven/maven.go +++ b/routers/api/packages/maven/maven.go @@ -117,7 +117,9 @@ func serveMavenMetadata(ctx *context.Context, params parameters) { xmlMetadataWithHeader := append([]byte(xml.Header), xmlMetadata...) latest := pds[len(pds)-1] - ctx.Resp.Header().Set("Last-Modified", latest.Version.CreatedUnix.Format(http.TimeFormat)) + // http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat + lastModifed := latest.Version.CreatedUnix.AsTime().UTC().Format(http.TimeFormat) + ctx.Resp.Header().Set("Last-Modified", lastModifed) ext := strings.ToLower(filepath.Ext(params.Filename)) if isChecksumExtension(ext) { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 9f3b63698a..a082498dfd 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -395,7 +395,8 @@ func (h *serviceHandler) sendFile(ctx *context.Context, contentType, file string ctx.Resp.Header().Set("Content-Type", contentType) ctx.Resp.Header().Set("Content-Length", fmt.Sprintf("%d", fi.Size())) - ctx.Resp.Header().Set("Last-Modified", fi.ModTime().Format(http.TimeFormat)) + // http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat + ctx.Resp.Header().Set("Last-Modified", fi.ModTime().UTC().Format(http.TimeFormat)) http.ServeFile(ctx.Resp, ctx.Req, reqFile) } From 9d3473119893ffde0ab36d98e7a0e41c5d0ba9a3 Mon Sep 17 00:00:00 2001 From: Jamie Schouten Date: Sun, 22 Sep 2024 00:42:17 +0200 Subject: [PATCH 10/12] Add bin to Composer Metadata (#32099) This PR addresses the missing `bin` field in Composer metadata, which currently causes vendor-provided binaries to not be symlinked to `vendor/bin` during installation. In the current implementation, running `composer install` does not publish the binaries, leading to issues where expected binaries are not available. By properly declaring the `bin` field, this PR ensures that binaries are correctly symlinked upon installation, as described in the [Composer documentation](https://getcomposer.org/doc/articles/vendor-binaries.md). (cherry picked from commit d351a42494e71b5e2da63302c2f9b46c78e6dbde) --- modules/packages/composer/metadata.go | 1 + tests/integration/api_packages_composer_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/packages/composer/metadata.go b/modules/packages/composer/metadata.go index 2c2e9ebf27..6035eae8ca 100644 --- a/modules/packages/composer/metadata.go +++ b/modules/packages/composer/metadata.go @@ -48,6 +48,7 @@ type Metadata struct { Homepage string `json:"homepage,omitempty"` License Licenses `json:"license,omitempty"` Authors []Author `json:"authors,omitempty"` + Bin []string `json:"bin,omitempty"` Autoload map[string]any `json:"autoload,omitempty"` AutoloadDev map[string]any `json:"autoload-dev,omitempty"` Extra map[string]any `json:"extra,omitempty"` diff --git a/tests/integration/api_packages_composer_test.go b/tests/integration/api_packages_composer_test.go index 9cdcd07e37..9d25cc4d64 100644 --- a/tests/integration/api_packages_composer_test.go +++ b/tests/integration/api_packages_composer_test.go @@ -37,6 +37,7 @@ func TestPackageComposer(t *testing.T) { packageType := "composer-plugin" packageAuthor := "Gitea Authors" packageLicense := "MIT" + packageBin := "./bin/script" var buf bytes.Buffer archive := zip.NewWriter(&buf) @@ -50,6 +51,9 @@ func TestPackageComposer(t *testing.T) { { "name": "` + packageAuthor + `" } + ], + "bin": [ + "` + packageBin + `" ] }`)) archive.Close() @@ -211,6 +215,8 @@ func TestPackageComposer(t *testing.T) { assert.Len(t, pkgs[0].Authors, 1) assert.Equal(t, packageAuthor, pkgs[0].Authors[0].Name) assert.Equal(t, "zip", pkgs[0].Dist.Type) - assert.Equal(t, "7b40bfd6da811b2b78deec1e944f156dbb2c747b", pkgs[0].Dist.Checksum) + assert.Equal(t, "4f5fa464c3cb808a1df191dbf6cb75363f8b7072", pkgs[0].Dist.Checksum) + assert.Len(t, pkgs[0].Bin, 1) + assert.Equal(t, packageBin, pkgs[0].Bin[0]) }) } From 2ffb08bb888bb950b6ed4ff12a4cf4bacc337e18 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 21 Sep 2024 17:50:54 +0800 Subject: [PATCH 11/12] Use camo.Always instead of camo.Allways (#32097) Fix #31575 https://gitea.com/gitea/docs/pulls/73 (cherry picked from commit 8e2dd5d3ddfb442937c79f05df88d18b856952cb) --- custom/conf/app.example.ini | 3 ++- modules/markup/camo.go | 2 +- modules/markup/camo_test.go | 2 +- modules/setting/camo.go | 14 ++++++++++++-- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 9cb5a67172..2eff51fe98 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -529,7 +529,8 @@ INTERNAL_TOKEN = ;; HMAC to encode urls with, it **is required** if camo is enabled. ;HMAC_KEY = ;; Set to true to use camo for https too lese only non https urls are proxyed -;ALLWAYS = false +;; ALLWAYS is deprecated and will be removed in the future +;ALWAYS = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/markup/camo.go b/modules/markup/camo.go index e93797de2b..7e2583469d 100644 --- a/modules/markup/camo.go +++ b/modules/markup/camo.go @@ -38,7 +38,7 @@ func camoHandleLink(link string) string { if setting.Camo.Enabled { lnkURL, err := url.Parse(link) if err == nil && lnkURL.IsAbs() && !strings.HasPrefix(link, setting.AppURL) && - (setting.Camo.Allways || lnkURL.Scheme != "https") { + (setting.Camo.Always || lnkURL.Scheme != "https") { return CamoEncode(link) } } diff --git a/modules/markup/camo_test.go b/modules/markup/camo_test.go index ba58835221..3c5d40afa0 100644 --- a/modules/markup/camo_test.go +++ b/modules/markup/camo_test.go @@ -28,7 +28,7 @@ func TestCamoHandleLink(t *testing.T) { "https://image.proxy/eivin43gJwGVIjR9MiYYtFIk0mw/aHR0cDovL3Rlc3RpbWFnZXMub3JnL2ltZy5qcGc", camoHandleLink("http://testimages.org/img.jpg")) - setting.Camo.Allways = true + setting.Camo.Always = true assert.Equal(t, "https://gitea.com/img.jpg", camoHandleLink("https://gitea.com/img.jpg")) diff --git a/modules/setting/camo.go b/modules/setting/camo.go index 366e9a116c..608ecf8363 100644 --- a/modules/setting/camo.go +++ b/modules/setting/camo.go @@ -3,18 +3,28 @@ package setting -import "code.gitea.io/gitea/modules/log" +import ( + "strconv" + + "code.gitea.io/gitea/modules/log" +) var Camo = struct { Enabled bool ServerURL string `ini:"SERVER_URL"` HMACKey string `ini:"HMAC_KEY"` - Allways bool + Always bool }{} func loadCamoFrom(rootCfg ConfigProvider) { mustMapSetting(rootCfg, "camo", &Camo) if Camo.Enabled { + oldValue := rootCfg.Section("camo").Key("ALLWAYS").MustString("") + if oldValue != "" { + log.Warn("camo.ALLWAYS is deprecated, use camo.ALWAYS instead") + Camo.Always, _ = strconv.ParseBool(oldValue) + } + if Camo.ServerURL == "" || Camo.HMACKey == "" { log.Fatal(`Camo settings require "SERVER_URL" and HMAC_KEY`) } From e3deb88a8d2494e386f221b9dc743a6a1710837d Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 22 Sep 2024 10:01:43 +0200 Subject: [PATCH 12/12] chore(release-notes): weekly cherry-pick week 2024-39 --- release-notes/5372.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 release-notes/5372.md diff --git a/release-notes/5372.md b/release-notes/5372.md new file mode 100644 index 0000000000..fccb305f34 --- /dev/null +++ b/release-notes/5372.md @@ -0,0 +1,5 @@ +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/9d3473119893ffde0ab36d98e7a0e41c5d0ba9a3) Add bin to Composer Metadata. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/f709de24039ab7e605d3e09e3b61240836381603) Fix wrong last modify time. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/2675a24649af2fff34f5c7e416d6ff78591d8d9c) Repo Activity: count new issues that were closed. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/526054332acb221e061d3900bba2dc6e012da52d) Fix incorrect /tokens api. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/0cafec4c7a2faf810953e9d522faf5dc019e1522) Do not escape relative path in RPM primary index.