From be2f1eeaeb92e552b5defcf8b374ceb4c3a6b1ee Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 8 Jun 2023 13:50:38 +0200 Subject: [PATCH 01/17] [GITEA] silently ignore obsolete sudo scope Fixes: https://codeberg.org/forgejo/forgejo/issues/820 (cherry picked from commit 6a7022ebbb83bda162974028cff01ebcc7c574ec) (cherry picked from commit 764eac47b50688d76fe90aad4819a426444ddb4a) (cherry picked from commit 1141eb7b6f2deeeca0acf1714058823d32097cfd) (cherry picked from commit 826b6509b6405ac0a0731ee0e1477ad2cbac585a) (cherry picked from commit 9990d932b8b72f9a27b6529b350eb09d44b7ef88) (cherry picked from commit 7eca57074385f296427d06c059d331d3704ccf15) (cherry picked from commit 66e1d3f082a99bb0006daf0f337850f251c235dc) (cherry picked from commit 188226a8e6b2926f1f276462741f7cc4d7a050b0) (cherry picked from commit 4cd1bff25c6cafa33464594c99b39326a6dd5740) (cherry picked from commit fad6b6d2c49492297d9d8512afc0369e544a6e75) (cherry picked from commit 5b25c3d8512466fd5fceea86b550bdb35c3aa04b) (cherry picked from commit 4746ece4dd018af781181744fb8743e83b64c6df) (cherry picked from commit 2a6f85afb33a1a0b7424c30de3cdff030f483294) (cherry picked from commit c027d724ee0b694e48d2b7ee1915ba55222a03e0) --- models/auth/token_scope.go | 2 +- models/auth/token_scope_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/auth/token_scope.go b/models/auth/token_scope.go index fe57276700..003ca5c9ab 100644 --- a/models/auth/token_scope.go +++ b/models/auth/token_scope.go @@ -250,7 +250,7 @@ func (s AccessTokenScope) parse() (accessTokenScopeBitmap, error) { remainingScopes = remainingScopes[i+1:] } singleScope := AccessTokenScope(v) - if singleScope == "" { + if singleScope == "" || singleScope == "sudo" { continue } if singleScope == AccessTokenScopeAll { diff --git a/models/auth/token_scope_test.go b/models/auth/token_scope_test.go index a6097e45d7..d11c5e6a3d 100644 --- a/models/auth/token_scope_test.go +++ b/models/auth/token_scope_test.go @@ -20,7 +20,7 @@ func TestAccessTokenScope_Normalize(t *testing.T) { tests := []scopeTestNormalize{ {"", "", nil}, {"write:misc,write:notification,read:package,write:notification,public-only", "public-only,write:misc,write:notification,read:package", nil}, - {"all", "all", nil}, + {"all,sudo", "all", nil}, {"write:activitypub,write:admin,write:misc,write:notification,write:organization,write:package,write:issue,write:repository,write:user", "all", nil}, {"write:activitypub,write:admin,write:misc,write:notification,write:organization,write:package,write:issue,write:repository,write:user,public-only", "public-only,all", nil}, } From 6afe70bbf1290e604fc476ee27901d1722ac1272 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Mon, 12 Jun 2023 13:57:01 +0200 Subject: [PATCH 02/17] [GITEA] add option for banning dots in usernames Refs: https://codeberg.org/forgejo/forgejo/pulls/676 Author: Panagiotis "Ivory" Vasilopoulos Date: Mon Jun 12 13:57:01 2023 +0200 Co-authored-by: Gusted (cherry picked from commit fabdda5c6e84017bf75ab5f9ab6cc0e583b70d09) (cherry picked from commit d2c7f45621028d37944659db096bc92c031dd8e7) (cherry picked from commit dfdbaba3d6b7abf1c542b0ea41b7812b729cc217) (cherry picked from commit a3cda092b8897e4d669cfcf2cb8b16236e3c9b32) (cherry picked from commit f0fdb5905c3b22bec043530da15d2c52f6bc41c9) (cherry picked from commit 9697e48c1f8b23d3dd1da246b525b63c3756353d) (cherry picked from commit 46e31009a86db18a9b5bd8e2f535b198df90c437) (cherry picked from commit 5bb2c54b6f55499937396339bcacd3b4d8fb6b5e) (cherry picked from commit 682f9d24e13b83d89bd6b86324960f1b4fc72eeb) (cherry picked from commit 18634810057ef88fd01b54cec33bd4bd04c53221) (cherry picked from commit 4f1b7c4ddbc4099aa9b6fda1e0145d37f638e567) --- custom/conf/app.example.ini | 5 +++++ modules/setting/service.go | 2 ++ modules/validation/helpers.go | 13 ++++++++++--- modules/validation/helpers_test.go | 31 +++++++++++++++++++++++++++++- modules/web/middleware/binding.go | 7 ++++++- options/locale/locale_en-US.ini | 2 ++ templates/admin/config.tmpl | 2 ++ 7 files changed, 57 insertions(+), 5 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index abf4d459b1..cf3ff6cd43 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -808,6 +808,11 @@ LEVEL = Info ;; Every new user will have restricted permissions depending on this setting ;DEFAULT_USER_IS_RESTRICTED = false ;; +;; Users will be able to use dots when choosing their username. Disabling this is +;; helpful if your usersare having issues with e.g. RSS feeds or advanced third-party +;; extensions that use strange regex patterns. +; ALLOW_DOTS_IN_USERNAMES = true +;; ;; Either "public", "limited" or "private", default is "public" ;; Limited is for users visible only to signed users ;; Private is for users visible only to members of their organizations diff --git a/modules/setting/service.go b/modules/setting/service.go index befb94b61b..afaee18101 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -68,6 +68,7 @@ var Service = struct { DefaultKeepEmailPrivate bool DefaultAllowCreateOrganization bool DefaultUserIsRestricted bool + AllowDotsInUsernames bool EnableTimetracking bool DefaultEnableTimetracking bool DefaultEnableDependencies bool @@ -180,6 +181,7 @@ func loadServiceFrom(rootCfg ConfigProvider) { Service.DefaultKeepEmailPrivate = sec.Key("DEFAULT_KEEP_EMAIL_PRIVATE").MustBool() Service.DefaultAllowCreateOrganization = sec.Key("DEFAULT_ALLOW_CREATE_ORGANIZATION").MustBool(true) Service.DefaultUserIsRestricted = sec.Key("DEFAULT_USER_IS_RESTRICTED").MustBool(false) + Service.AllowDotsInUsernames = sec.Key("ALLOW_DOTS_IN_USERNAMES").MustBool(true) Service.EnableTimetracking = sec.Key("ENABLE_TIMETRACKING").MustBool(true) if Service.EnableTimetracking { Service.DefaultEnableTimetracking = sec.Key("DEFAULT_ENABLE_TIMETRACKING").MustBool(true) diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index f6e00f3887..567ad867fe 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -117,13 +117,20 @@ func IsValidExternalTrackerURLFormat(uri string) bool { } var ( - validUsernamePattern = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) - invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) // No consecutive or trailing non-alphanumeric chars + validUsernamePatternWithDots = regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`) + validUsernamePatternWithoutDots = regexp.MustCompile(`^[\da-zA-Z][-\w]*$`) + + // No consecutive or trailing non-alphanumeric chars, catches both cases + invalidUsernamePattern = regexp.MustCompile(`[-._]{2,}|[-._]$`) ) // IsValidUsername checks if username is valid func IsValidUsername(name string) bool { // It is difficult to find a single pattern that is both readable and effective, // but it's easier to use positive and negative checks. - return validUsernamePattern.MatchString(name) && !invalidUsernamePattern.MatchString(name) + if setting.Service.AllowDotsInUsernames { + return validUsernamePatternWithDots.MatchString(name) && !invalidUsernamePattern.MatchString(name) + } + + return validUsernamePatternWithoutDots.MatchString(name) && !invalidUsernamePattern.MatchString(name) } diff --git a/modules/validation/helpers_test.go b/modules/validation/helpers_test.go index 52f383f698..a1bdf2a29c 100644 --- a/modules/validation/helpers_test.go +++ b/modules/validation/helpers_test.go @@ -155,7 +155,8 @@ func Test_IsValidExternalTrackerURLFormat(t *testing.T) { } } -func TestIsValidUsername(t *testing.T) { +func TestIsValidUsernameAllowDots(t *testing.T) { + setting.Service.AllowDotsInUsernames = true tests := []struct { arg string want bool @@ -185,3 +186,31 @@ func TestIsValidUsername(t *testing.T) { }) } } + +func TestIsValidUsernameBanDots(t *testing.T) { + setting.Service.AllowDotsInUsernames = false + defer func() { + setting.Service.AllowDotsInUsernames = true + }() + + tests := []struct { + arg string + want bool + }{ + {arg: "a", want: true}, + {arg: "abc", want: true}, + {arg: "0.b-c", want: false}, + {arg: "a.b-c_d", want: false}, + {arg: ".abc", want: false}, + {arg: "abc.", want: false}, + {arg: "a..bc", want: false}, + {arg: "a...bc", want: false}, + {arg: "a.-bc", want: false}, + {arg: "a._bc", want: false}, + } + for _, tt := range tests { + t.Run(tt.arg, func(t *testing.T) { + assert.Equalf(t, tt.want, IsValidUsername(tt.arg), "IsValidUsername[AllowDotsInUsernames=false](%v)", tt.arg) + }) + } +} diff --git a/modules/web/middleware/binding.go b/modules/web/middleware/binding.go index d9bcdf3b2a..4e7fca80e2 100644 --- a/modules/web/middleware/binding.go +++ b/modules/web/middleware/binding.go @@ -8,6 +8,7 @@ import ( "reflect" "strings" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/validation" @@ -135,7 +136,11 @@ func Validate(errs binding.Errors, data map[string]any, f Form, l translation.Lo case validation.ErrRegexPattern: data["ErrorMsg"] = trName + l.Tr("form.regex_pattern_error", errs[0].Message) case validation.ErrUsername: - data["ErrorMsg"] = trName + l.Tr("form.username_error") + if setting.Service.AllowDotsInUsernames { + data["ErrorMsg"] = trName + l.Tr("form.username_error") + } else { + data["ErrorMsg"] = trName + l.Tr("form.username_error_no_dots") + } case validation.ErrInvalidGroupTeamMap: data["ErrorMsg"] = trName + l.Tr("form.invalid_group_team_map_error", errs[0].Message) default: diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 932d7311f1..9efe07a3d7 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -291,6 +291,7 @@ default_allow_create_organization = Allow Creation of Organizations by Default default_allow_create_organization_popup = Allow new user accounts to create organizations by default. default_enable_timetracking = Enable Time Tracking by Default default_enable_timetracking_popup = Enable time tracking for new repositories by default. +allow_dots_in_usernames = Allow users to use dots in their usernames. Doesn't affect existing accounts. no_reply_address = Hidden Email Domain no_reply_address_helper = Domain name for users with a hidden email address. For example, the username 'joe' will be logged in Git as 'joe@noreply.example.org' if the hidden email domain is set to 'noreply.example.org'. password_algorithm = Password Hash Algorithm @@ -531,6 +532,7 @@ include_error = ` must contain substring "%s".` glob_pattern_error = ` glob pattern is invalid: %s.` regex_pattern_error = ` regex pattern is invalid: %s.` username_error = ` can only contain alphanumeric chars ('0-9','a-z','A-Z'), dash ('-'), underscore ('_') and dot ('.'). It cannot begin or end with non-alphanumeric chars, and consecutive non-alphanumeric chars are also forbidden.` +username_error_no_dots = ` can only contain alphanumeric chars ('0-9','a-z','A-Z'), dash ('-') and underscore ('_'). It cannot begin or end with non-alphanumeric chars, and consecutive non-alphanumeric chars are also forbidden.` invalid_group_team_map_error = ` mapping is invalid: %s` unknown_error = Unknown error: captcha_incorrect = The CAPTCHA code is incorrect. diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl index 36d9bcb8a5..6947ecfe8e 100644 --- a/templates/admin/config.tmpl +++ b/templates/admin/config.tmpl @@ -159,6 +159,8 @@
{{if .Service.DefaultKeepEmailPrivate}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
{{.locale.Tr "admin.config.default_allow_create_organization"}}
{{if .Service.DefaultAllowCreateOrganization}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
+
{{.locale.Tr "admin.config.allow_dots_in_usernames"}}
+
{{if .Service.AllowDotsInUsernames}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
{{.locale.Tr "admin.config.enable_timetracking"}}
{{if .Service.EnableTimetracking}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}
{{if .Service.EnableTimetracking}} From 039fa20cc8a5b50d5cc37de4503e8a9a80042bcc Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 24 Jun 2023 13:08:52 +0200 Subject: [PATCH 03/17] [GITEA] Add password length check on install page - Resolves #271 - Ensure that the adminstrator password is at least `MIN_PASSWORD_LENGTH`. (cherry picked from commit 28cb04c3f5040980e716ce66cd5906f324257e02) (cherry picked from commit 95371ebd92cd005e2d50a4754e60525cf6135b86) (cherry picked from commit a134288ab6b0291082d913c4e22456b31af58af9) (cherry picked from commit 4202f052cb32aec71a61dd2afd814035a9d85eea) (cherry picked from commit 510b7467d3ee0bf346ad1843775affe1df0675ae) (cherry picked from commit f3a6e1f121e89aaf608fd9890eaf06ed939d1006) (cherry picked from commit f340508819866f355feec6d01b349fa7df29ace9) (cherry picked from commit b891bb176d48c3855cc5b6e4573e7a337af9d382) (cherry picked from commit 1a1bfc38cc7863f5cb3022560cacb2006d08f113) (cherry picked from commit 083d5aefed10e54814c4438eabcd01973d305502) (cherry picked from commit 4586096be9b6214058245da3227541866ea4312f) --- routers/install/install.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/routers/install/install.go b/routers/install/install.go index 6d60dfdca3..88f35cf876 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -358,6 +358,12 @@ func SubmitInstall(ctx *context.Context) { ctx.RenderWithErr(ctx.Tr("form.password_not_match"), tplInstall, form) return } + if len(form.AdminPasswd) < setting.MinPasswordLength { + ctx.Data["Err_Admin"] = true + ctx.Data["Err_AdminPasswd"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplInstall, form) + return + } } // Init the engine with migration From 2f66c1e4ce5f3c6c5555de35a68f7cc9a986b62f Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 21 Jul 2023 15:50:14 +0200 Subject: [PATCH 04/17] [GITEA] Show manual cron run's last time - Currently in the cron tasks, the 'Previous Time' only displays the previous time of when the cron library executes the function, but not any of the manual executions of the task. - Store the last run's time in memory in the Task struct and use that, when that time is later than time that the cron library has executed this task. - This ensures that if an instance admin manually starts a task, there's feedback that this task is/has been run, because the task might be run that quick, that the status icon already has been changed to an checkmark, - Tasks that are executed at startup now reflect this as well, as the time of the execution of that task on startup is now being shown as 'Previous Time'. - Added integration tests for the API part, which is easier to test because querying the HTML table of cron tasks is non-trivial. - Resolves https://codeberg.org/forgejo/forgejo/issues/949 (cherry picked from commit 0475e2048e7641f6ca223d486ffb8e6cecddef87) (cherry picked from commit dcc952f0db883204a1585f3fec0abcacdcab4649) (cherry picked from commit 7168a240e8b5dcba5d6bd6d1395e79eea1e6c5f5) (cherry picked from commit 4bc4cccb1b3836c43fd6f8056fcb3605e7c53bfb) (cherry picked from commit 3fe019ca3c9bbc66ff1ba644c2cb3e118f99948c) [GITEA] Show manual cron run's last time (squash) 26 jobs in cron fixtures (cherry picked from commit 8473030628302f78a954b14d02b423cc180b2751) (cherry picked from commit 871c7297423efe5f2ed33a0dd52070d826f078c8) (cherry picked from commit daefb27d2caaf27ebb8c8142634aec9151515515) --- services/cron/cron.go | 6 ++++ services/cron/tasks.go | 9 +++++ tests/integration/api_admin_test.go | 51 +++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/services/cron/cron.go b/services/cron/cron.go index e3f31d08f0..63db75ab3b 100644 --- a/services/cron/cron.go +++ b/services/cron/cron.go @@ -106,6 +106,12 @@ func ListTasks() TaskTable { next = e.NextRun() prev = e.PreviousRun() } + + // If the manual run is after the cron run, use that instead. + if prev.Before(task.LastRun) { + prev = task.LastRun + } + task.lock.Lock() tTable = append(tTable, &TaskTableRow{ Name: task.Name, diff --git a/services/cron/tasks.go b/services/cron/tasks.go index ea1925c26c..d2c3d1d812 100644 --- a/services/cron/tasks.go +++ b/services/cron/tasks.go @@ -9,6 +9,7 @@ import ( "reflect" "strings" "sync" + "time" "code.gitea.io/gitea/models/db" system_model "code.gitea.io/gitea/models/system" @@ -37,6 +38,8 @@ type Task struct { LastMessage string LastDoer string ExecTimes int64 + // This stores the time of the last manual run of this task. + LastRun time.Time } // DoRunAtStart returns if this task should run at the start @@ -88,6 +91,12 @@ func (t *Task) RunWithUser(doer *user_model.User, config Config) { } }() graceful.GetManager().RunWithShutdownContext(func(baseCtx context.Context) { + // Store the time of this run, before the function is executed, so it + // matches the behavior of what the cron library does. + t.lock.Lock() + t.LastRun = time.Now() + t.lock.Unlock() + pm := process.GetManager() doerName := "" if doer != nil && doer.ID != -1 { diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 6613d4b715..4e222e22e6 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "testing" + "time" asymkey_model "code.gitea.io/gitea/models/asymkey" auth_model "code.gitea.io/gitea/models/auth" @@ -282,3 +283,53 @@ func TestAPIRenameUser(t *testing.T) { }) MakeRequest(t, req, http.StatusOK) } + +func TestAPICron(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // user1 is an admin user + session := loginUser(t, "user1") + + t.Run("List", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadAdmin) + urlStr := fmt.Sprintf("/api/v1/admin/cron?token=%s", token) + req := NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) + + assert.Equal(t, "26", resp.Header().Get("X-Total-Count")) + + var crons []api.Cron + DecodeJSON(t, resp, &crons) + assert.Len(t, crons, 26) + }) + + t.Run("Execute", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + now := time.Now() + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteAdmin) + /// Archive cleanup is harmless, because in the text environment there are none + /// and is thus an NOOP operation and therefore doesn't interfere with any other + /// tests. + urlStr := fmt.Sprintf("/api/v1/admin/cron/archive_cleanup?token=%s", token) + req := NewRequest(t, "POST", urlStr) + MakeRequest(t, req, http.StatusNoContent) + + // Check for the latest run time for this cron, to ensure it + // has been run. + urlStr = fmt.Sprintf("/api/v1/admin/cron?token=%s", token) + req = NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) + + var crons []api.Cron + DecodeJSON(t, resp, &crons) + + for _, cron := range crons { + if cron.Name == "archive_cleanup" { + assert.True(t, now.Before(cron.Prev)) + } + } + }) +} From b13a81ab98a02e30d1b508bb89cdd67a05eae782 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 5 Aug 2023 14:47:09 +0200 Subject: [PATCH 05/17] [GITEA] Allow release creation on commit - The code and tests are already there to allow releases to be created on commits. - This patch modifies the web code to take into account that an commitID could've been passed as target. - Added unit test. - Resolves https://codeberg.org/forgejo/forgejo/issues/1196 (cherry picked from commit 90863e0ab51d1b243f67de266bbeeb7a9031c525) (cherry picked from commit c805aa23b5c6c9a8ab79e2e66786a4ef798e827a) (cherry picked from commit cf45567ca60b2a9411694c8e9b649fd77c64bdae) (cherry picked from commit 672a2b91e5612f438bd7951d173f42c223629fd1) (cherry picked from commit 82c930152cd693f8451e9553504365c724e1fced) (cherry picked from commit 95ac2508b3e8dd9fc2b0168600d989dbce0744ec) --- routers/web/repo/release.go | 4 +++- tests/integration/release_test.go | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 138df45857..a658d7e4cd 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -387,7 +387,9 @@ func NewReleasePost(ctx *context.Context) { return } - if !ctx.Repo.GitRepo.IsBranchExist(form.Target) { + // form.Target can be a branch name or a full commitID. + if !ctx.Repo.GitRepo.IsBranchExist(form.Target) && + len(form.Target) == git.SHAFullLength && !ctx.Repo.GitRepo.IsCommitExist(form.Target) { ctx.RenderWithErr(ctx.Tr("form.target_branch_not_exist"), tplReleaseNew, &form) return } diff --git a/tests/integration/release_test.go b/tests/integration/release_test.go index 8de761ea6c..1c4be6a927 100644 --- a/tests/integration/release_test.go +++ b/tests/integration/release_test.go @@ -21,6 +21,10 @@ import ( ) func createNewRelease(t *testing.T, session *TestSession, repoURL, tag, title string, preRelease, draft bool) { + createNewReleaseTarget(t, session, repoURL, tag, title, "master", preRelease, draft) +} + +func createNewReleaseTarget(t *testing.T, session *TestSession, repoURL, tag, title, target string, preRelease, draft bool) { req := NewRequest(t, "GET", repoURL+"/releases/new") resp := session.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) @@ -31,7 +35,7 @@ func createNewRelease(t *testing.T, session *TestSession, repoURL, tag, title st postData := map[string]string{ "_csrf": htmlDoc.GetCSRF(), "tag_name": tag, - "tag_target": "master", + "tag_target": target, "title": title, "content": "", } @@ -239,3 +243,12 @@ func TestViewTagsList(t *testing.T) { assert.EqualValues(t, []string{"v1.0", "delete-tag", "v1.1"}, tagNames) } + +func TestReleaseOnCommit(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user2") + createNewReleaseTarget(t, session, "/user2/repo1", "v0.0.1", "v0.0.1", "65f1bf27bc3bf70f64657658635e66094edbcb4d", false, false) + + checkLatestReleaseAndCount(t, session, "/user2/repo1", "v0.0.1", translation.NewLocale("en-US").Tr("repo.release.stable"), 4) +} From acce9f9b70053fafb4572bd7fcfc66c3a7ed1f8c Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 8 Aug 2023 15:25:40 +0200 Subject: [PATCH 06/17] [GITEA] Fix media description render for orgmode - In org mode you can specify an description for media via the following syntax `[[description][media link]]`. The description is then used as title or alt. - This patch fixes the rendering of the description by seperating the description and non-description cases and using `org.String()`. - Added unit tests. - Inspired by https://github.com/niklasfasching/go-org/blob/6eb20dbda93cb88c3503f7508dc78cbbc639378f/org/html_writer.go#L406-L427 - Resolves https://codeberg.org/Codeberg/Community/issues/848 (cherry picked from commit ef2456e1b16be50a31c32d69c86f0e07971f5ff2) (cherry picked from commit b6559b4825d90da8a6acdfb266d8aa4b712a8e49) (cherry picked from commit d6dcc34492221bea782ee697bdf3e623ff5927a7) (cherry picked from commit 8b8aab83113b34bade61964e2097ed497abc39e9) (cherry picked from commit b9e0297bc8001b543b6e6c0e6864f9686b345772) --- modules/markup/orgmode/orgmode.go | 28 ++++++++++++++++++-------- modules/markup/orgmode/orgmode_test.go | 14 ++++++++++++- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index a6dac12039..7a95ab518c 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -153,18 +153,30 @@ func (r *Writer) WriteRegularLink(l org.RegularLink) { link = []byte(util.URLJoin(r.URLPrefix, lnk)) } - description := string(link) - if l.Description != nil { - description = r.WriteNodesAsString(l.Description...) - } switch l.Kind() { case "image": - imageSrc := getMediaURL(link) - fmt.Fprintf(r, `%s`, imageSrc, description, description) + if l.Description == nil { + imageSrc := getMediaURL(link) + fmt.Fprintf(r, `%s`, imageSrc, link, link) + } else { + description := strings.TrimPrefix(org.String(l.Description...), "file:") + imageSrc := getMediaURL([]byte(description)) + fmt.Fprintf(r, `%s`, link, imageSrc, imageSrc) + } case "video": - videoSrc := getMediaURL(link) - fmt.Fprintf(r, ``, videoSrc, description, description) + if l.Description == nil { + imageSrc := getMediaURL(link) + fmt.Fprintf(r, ``, imageSrc, link, link) + } else { + description := strings.TrimPrefix(org.String(l.Description...), "file:") + videoSrc := getMediaURL([]byte(description)) + fmt.Fprintf(r, ``, link, videoSrc, videoSrc) + } default: + description := string(link) + if l.Description != nil { + description = r.WriteNodesAsString(l.Description...) + } fmt.Fprintf(r, `%s`, link, description, description) } } diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index d6467c36f7..8f454e9955 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -42,7 +42,7 @@ func TestRender_StandardLinks(t *testing.T) { "

WikiPage

") } -func TestRender_Images(t *testing.T) { +func TestRender_Media(t *testing.T) { setting.AppURL = AppURL setting.AppSubURL = AppSubURL @@ -60,6 +60,18 @@ func TestRender_Images(t *testing.T) { test("[[file:"+url+"]]", "

\""+result+"\"

") + + // With description. + test("[[https://example.com][https://example.com/example.svg]]", + `

https://example.com/example.svg

`) + test("[[https://example.com][https://example.com/example.mp4]]", + `

`) + + // Without description. + test("[[https://example.com/example.svg]]", + `

https://example.com/example.svg

`) + test("[[https://example.com/example.mp4]]", + `

`) } func TestRender_Source(t *testing.T) { From 0c11587582b8837778ee85f4e3b04241e5d71760 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 18 Aug 2023 11:21:24 +0200 Subject: [PATCH 07/17] [GITEA] Improve HTML title on repositories - The `` element that lives inside the `<head>` element is an important element that gives browsers and search engine crawlers the title of the webpage, hence the element name. It's therefor important that this title is accurate. - Currently there are three issues with titles on repositories. It doesn't use the `FullName` and instead only uses the repository name, this doesn't distinguish which user or organisation the repository is on. It doesn't show the full treepath in the title when visiting an file inside a directory and instead only uses the latest path in treepath. It can show the repository name twice if the `.Title` variable also included the repository name such as on the repository homepage. - Use the repository's fullname (which include which user the repository is on) instead of just their name. - Display the repository's fullname if it isn't already in `.Title`. - Use the full treepath in the repository code view instead of just the last path. - Adds integration tests. - Adds a new repository (`repo59`) that has 3 depths for folders, which wasn't in any other fixture repository yet, so the full treepath for could be properly tested. - Resolves https://codeberg.org/forgejo/forgejo/issues/1276 (cherry picked from commit ff9a6a2cda34cf2b2e392cc47125ed0f619b287b) (cherry picked from commit 76dffc862103eb23d51445ef9d611296308c8413) (cherry picked from commit ff0615b9d0f3ea4bd86a28c4ac5b0c4740230c81) (cherry picked from commit 8712eaa394053a8c8f1f4cb17307e094c65c7059) --- models/fixtures/release.yml | 14 +++ models/fixtures/repo_unit.yml | 32 ++++++ models/fixtures/repository.yml | 14 +++ models/fixtures/user.yml | 2 +- models/repo/repo_list_test.go | 6 +- routers/web/repo/view.go | 4 +- templates/base/head.tmpl | 3 +- .../user2/repo59.git/HEAD | 1 + .../user2/repo59.git/config | 4 + .../user2/repo59.git/description | 1 + .../user2/repo59.git/info/exclude | 6 + .../40/8bbd3bd1f96950f8cf2f98c479557f6b18817a | Bin 0 -> 63 bytes .../5d/5c87a90af64cc67f22d60a942d5efaef8bc96b | Bin 0 -> 50 bytes .../88/3e2970ed6937cbb63311e941adb97df0ae3a52 | Bin 0 -> 49 bytes .../8c/ac7a8f434451410cc91ab9c04d07baff974ad8 | Bin 0 -> 120 bytes .../a0/ccafed39086ef520be6886d9395eb2100d317e | Bin 0 -> 102 bytes .../ab/e2a9ddfd7f542ff89bc13960a929dc8ca86c99 | Bin 0 -> 36 bytes .../cd/879fb6cf5b7bbe0fbc3a0ef44c8695fde89a56 | Bin 0 -> 108 bytes .../d8/f53dfb33f6ccf4169c34970b5e747511c18beb | Bin 0 -> 784 bytes .../f3/c1ec36c0e7605be54e71f24035caa675b7ba41 | Bin 0 -> 48 bytes .../user2/repo59.git/packed-refs | 3 + .../user2/repo59.git/refs/heads/cake-recipe | 1 + tests/integration/api_repo_test.go | 6 +- tests/integration/integration_test.go | 15 +++ tests/integration/repo_test.go | 104 ++++++++++++++++++ 25 files changed, 206 insertions(+), 10 deletions(-) create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/HEAD create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/config create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/description create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/info/exclude create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/40/8bbd3bd1f96950f8cf2f98c479557f6b18817a create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/5d/5c87a90af64cc67f22d60a942d5efaef8bc96b create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/88/3e2970ed6937cbb63311e941adb97df0ae3a52 create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/8c/ac7a8f434451410cc91ab9c04d07baff974ad8 create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/a0/ccafed39086ef520be6886d9395eb2100d317e create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/ab/e2a9ddfd7f542ff89bc13960a929dc8ca86c99 create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/cd/879fb6cf5b7bbe0fbc3a0ef44c8695fde89a56 create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/d8/f53dfb33f6ccf4169c34970b5e747511c18beb create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/objects/f3/c1ec36c0e7605be54e71f24035caa675b7ba41 create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/packed-refs create mode 100644 tests/gitea-repositories-meta/user2/repo59.git/refs/heads/cake-recipe diff --git a/models/fixtures/release.yml b/models/fixtures/release.yml index 4ed7df440d..844deb3a7b 100644 --- a/models/fixtures/release.yml +++ b/models/fixtures/release.yml @@ -136,3 +136,17 @@ is_prerelease: false is_tag: false created_unix: 946684803 + +- id: 11 + repo_id: 59 + publisher_id: 2 + tag_name: "v1.0" + lower_tag_name: "v1.0" + target: "main" + title: "v1.0" + sha1: "d8f53dfb33f6ccf4169c34970b5e747511c18beb" + num_commits: 1 + is_draft: false + is_prerelease: false + is_tag: false + created_unix: 946684803 diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index c22eb8c2a2..6afef2a432 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -608,6 +608,38 @@ type: 1 created_unix: 946684810 +# BEGIN Forgejo [GITEA] Improve HTML title on repositories +- + id: 1093 + repo_id: 59 + type: 1 + created_unix: 946684810 + +- + id: 1094 + repo_id: 59 + type: 2 + created_unix: 946684810 + +- + id: 1095 + repo_id: 59 + type: 3 + created_unix: 946684810 + +- + id: 1096 + repo_id: 59 + type: 4 + created_unix: 946684810 + +- + id: 1097 + repo_id: 59 + type: 5 + created_unix: 946684810 +# END Forgejo [GITEA] Improve HTML title on repositories + - id: 91 repo_id: 58 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 15668e6cae..5dd3e8cc90 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -1467,6 +1467,7 @@ owner_name: user27 lower_name: repo49 name: repo49 + description: A wonderful repository with more than just a README.md default_branch: master num_watches: 0 num_stars: 0 @@ -1693,3 +1694,16 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + +- + id: 59 + owner_id: 2 + owner_name: user2 + lower_name: repo59 + name: repo59 + default_branch: master + is_empty: false + is_archived: false + is_private: false + status: 0 + num_issues: 0 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index f24d098a7e..c0d3051079 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -66,7 +66,7 @@ num_followers: 2 num_following: 1 num_stars: 2 - num_repos: 14 + num_repos: 15 num_teams: 0 num_members: 0 visibility: 0 diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index 7097b6ea14..3be1ebb3c9 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -138,12 +138,12 @@ func getTestCases() []struct { { name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse}, - count: 31, + count: 32, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse}, - count: 36, + count: 37, }, { name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", @@ -158,7 +158,7 @@ func getTestCases() []struct { { name: "AllPublic/PublicRepositoriesOfOrganization", opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse}, - count: 31, + count: 32, }, { name: "AllTemplates", diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index f0fe6140df..1495e126bc 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -165,7 +165,7 @@ func renderDirectory(ctx *context.Context, treeLink string) { if ctx.Repo.TreePath != "" { ctx.Data["HideRepoInfo"] = true - ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+path.Base(ctx.Repo.TreePath), ctx.Repo.RefName) + ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+util.PathEscapeSegments(ctx.Repo.TreePath), ctx.Repo.RefName) } subfolder, readmeFile, err := findReadmeFileInEntries(ctx, entries, true) @@ -344,7 +344,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st } defer dataRc.Close() - ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+path.Base(ctx.Repo.TreePath), ctx.Repo.RefName) + ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+util.PathEscapeSegments(ctx.Repo.TreePath), ctx.Repo.RefName) ctx.Data["FileIsSymlink"] = entry.IsLink() ctx.Data["FileName"] = blob.Name() ctx.Data["RawFileLink"] = rawLink + "/" + util.PathEscapeSegments(ctx.Repo.TreePath) diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl index c3645209cd..08c68752e2 100644 --- a/templates/base/head.tmpl +++ b/templates/base/head.tmpl @@ -2,7 +2,8 @@ <html lang="{{ctx.Locale.Lang}}" class="theme-{{if .SignedUser.Theme}}{{.SignedUser.Theme}}{{else}}{{DefaultTheme}}{{end}}"> <head> <meta name="viewport" content="width=device-width, initial-scale=1"> - <title>{{if .Title}}{{.Title | RenderEmojiPlain}} - {{end}}{{if .Repository.Name}}{{.Repository.Name}} - {{end}}{{AppName}} + {{/* Display `- .Repsository.FullName` only if `.Title` does not already start with that. */}} + {{if .Title}}{{.Title | RenderEmojiPlain}} - {{end}}{{if and (.Repository.Name) (not (StringUtils.HasPrefix .Title .Repository.FullName))}}{{.Repository.FullName}} - {{end}}{{AppName}} {{if .ManifestData}}{{end}} diff --git a/tests/gitea-repositories-meta/user2/repo59.git/HEAD b/tests/gitea-repositories-meta/user2/repo59.git/HEAD new file mode 100644 index 0000000000..cb089cd89a --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo59.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests/gitea-repositories-meta/user2/repo59.git/config b/tests/gitea-repositories-meta/user2/repo59.git/config new file mode 100644 index 0000000000..07d359d07c --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo59.git/config @@ -0,0 +1,4 @@ +[core] + repositoryformatversion = 0 + filemode = true + bare = true diff --git a/tests/gitea-repositories-meta/user2/repo59.git/description b/tests/gitea-repositories-meta/user2/repo59.git/description new file mode 100644 index 0000000000..498b267a8c --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo59.git/description @@ -0,0 +1 @@ +Unnamed repository; edit this file 'description' to name the repository. diff --git a/tests/gitea-repositories-meta/user2/repo59.git/info/exclude b/tests/gitea-repositories-meta/user2/repo59.git/info/exclude new file mode 100644 index 0000000000..a5196d1be8 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo59.git/info/exclude @@ -0,0 +1,6 @@ +# git ls-files --others --exclude-from=.git/info/exclude +# Lines that start with '#' are comments. +# For a project mostly in C, the following would be a good set of +# exclude patterns (uncomment them if you want to use them): +# *.[oa] +# *~ diff --git a/tests/gitea-repositories-meta/user2/repo59.git/objects/40/8bbd3bd1f96950f8cf2f98c479557f6b18817a b/tests/gitea-repositories-meta/user2/repo59.git/objects/40/8bbd3bd1f96950f8cf2f98c479557f6b18817a new file mode 100644 index 0000000000000000000000000000000000000000..567284ef1c21f472841f3d729cbfe024d78c448c GIT binary patch literal 63 zcmV-F0Korv0ZYosPf{>3XHZrMN-fAQ&Me6)6NNXyJgE!N}W0ssfX5``$?8bAO5 literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo59.git/objects/5d/5c87a90af64cc67f22d60a942d5efaef8bc96b b/tests/gitea-repositories-meta/user2/repo59.git/objects/5d/5c87a90af64cc67f22d60a942d5efaef8bc96b new file mode 100644 index 0000000000000000000000000000000000000000..f23960f4cc8c09af29fc4765f683b697a4547d74 GIT binary patch literal 50 zcmV-20L}k+0V^p=O;s>9VK6ZO0)@QP;*!j~bcW9d-8WL<+jltv I07;Az9utceGXMYp literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo59.git/objects/88/3e2970ed6937cbb63311e941adb97df0ae3a52 b/tests/gitea-repositories-meta/user2/repo59.git/objects/88/3e2970ed6937cbb63311e941adb97df0ae3a52 new file mode 100644 index 0000000000000000000000000000000000000000..46cc9e3e5ec14999b5424f35c1c548db9e9e33c1 GIT binary patch literal 49 zcmb7FlR6{FfcPQQ3!H%bn$i7%S~Z$=-z96@n>ehkMsI7j#P%$ zXG=6znHT_pLP~0C0Yhv|`%12FKF8{nu5nG#jr;Y!`(!rMjI`9mlG377y^@L&h7LQ; ag14FGr?(jkzI0r>v-ZO}s~`Z9QY~(zy*Y~j literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo59.git/objects/a0/ccafed39086ef520be6886d9395eb2100d317e b/tests/gitea-repositories-meta/user2/repo59.git/objects/a0/ccafed39086ef520be6886d9395eb2100d317e new file mode 100644 index 0000000000000000000000000000000000000000..3b71228a7ec7b91f1066cda387ef6efa28c2ae68 GIT binary patch literal 102 zcmV-s0Ga=I0V^p=O;xb4U@$Z=Ff%bx2y%6F@paY9O<`F5Xyx6%^&$E{W*@XnSgCoZ zXGP9TsG{Q3M6>2AdT%A&7_dRgi_+j`HiK;pmmM?MU`7Mx>)%bH?6OHM zk5sBnouqA=Y63LcbH7YOmH|GAl4Hc@EW@%K&C)1I1Uia^1hFYP#!;RNM>a}%Dtb?4 zJAn6?4K(;YO4A`5NBYlfjhe2`eoNZs4?rJ;J;Mr|fWQvz5u(27_uQ2I-(JxbV^x4( z|B6Ty%>s_%fUBlh_~u>6-<$#zs9bFmF%~6^Q@J9f=}|(LabtH3q_NxV$YbR02+h-Wy>8SN zL-n>NnOHxD1_ktBKMTFMF~Oeo44c$Nx1zjPg-n1^s{~0y49o8@?{IqT_1l9FDBH+tz~X}{gyB7!qm$KrCW*^)AW)NzRtpG4}}%%=|SNs z@Nz?3mnX@zAyYKZV*tH#nR{bxS}d9i?M(85HRz`|j6_MUVShjg2W|Ppe8h&7*MdL7JHfOBCJr|RNl?>o$nk$Jg%Kk7Ay z>ilZ^!I!SZjq{9}32tH%t54`QEQTUns*)fL4hA0LId@WGsHUmmoZfJs-`Nc O!MB&yMEwL3??r>^yMtc< literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo59.git/objects/f3/c1ec36c0e7605be54e71f24035caa675b7ba41 b/tests/gitea-repositories-meta/user2/repo59.git/objects/f3/c1ec36c0e7605be54e71f24035caa675b7ba41 new file mode 100644 index 0000000000000000000000000000000000000000..b6803eb5a271b9d33c981d1ba24fa4126ae409db GIT binary patch literal 48 zcmV-00MGw;0V^p=O;s>9W-u`T0)@2voRrieh6QKVzqRDZ`>L=nqwS_;+$I5D!#V&X Gkq<4~#T1zU literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo59.git/packed-refs b/tests/gitea-repositories-meta/user2/repo59.git/packed-refs new file mode 100644 index 0000000000..114c84d2aa --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo59.git/packed-refs @@ -0,0 +1,3 @@ +# pack-refs with: peeled fully-peeled sorted +d8f53dfb33f6ccf4169c34970b5e747511c18beb refs/heads/master +d8f53dfb33f6ccf4169c34970b5e747511c18beb refs/tags/v1.0 diff --git a/tests/gitea-repositories-meta/user2/repo59.git/refs/heads/cake-recipe b/tests/gitea-repositories-meta/user2/repo59.git/refs/heads/cake-recipe new file mode 100644 index 0000000000..63bbea6692 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo59.git/refs/heads/cake-recipe @@ -0,0 +1 @@ +d8f53dfb33f6ccf4169c34970b5e747511c18beb diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index be4135b050..bad1b05905 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -93,9 +93,9 @@ func TestAPISearchRepo(t *testing.T) { }{ { name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{ - nil: {count: 33}, - user: {count: 33}, - user2: {count: 33}, + nil: {count: 34}, + user: {count: 34}, + user2: {count: 34}, }, }, { diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 49a714c343..a3d0eb1019 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -531,3 +531,18 @@ func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { doc := NewHTMLParser(t, resp.Body) return doc.GetCSRF() } + +func GetHTMLTitle(t testing.TB, session *TestSession, urlStr string) string { + t.Helper() + + req := NewRequest(t, "GET", urlStr) + var resp *httptest.ResponseRecorder + if session == nil { + resp = MakeRequest(t, req, http.StatusOK) + } else { + resp = session.MakeRequest(t, req, http.StatusOK) + } + + doc := NewHTMLParser(t, resp.Body) + return doc.Find("head title").Text() +} diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index 9ace3ca30c..cccfd4b7bb 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -444,3 +444,107 @@ func TestGeneratedSourceLink(t *testing.T) { assert.Equal(t, "/user27/repo49/src/commit/aacbdfe9e1c4b47f60abe81849045fa4e96f1d75/test/test.txt", dataURL) }) } + +func TestRepoHTMLTitle(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + t.Run("Repository homepage", func(t *testing.T) { + t.Run("Without description", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1") + assert.EqualValues(t, "user2/repo1 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("With description", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user27/repo49") + assert.EqualValues(t, "user27/repo49: A wonderful repository with more than just a README.md - Gitea: Git with a cup of tea", htmlTitle) + }) + }) + + t.Run("Code view", func(t *testing.T) { + t.Run("Directory", func(t *testing.T) { + t.Run("Default branch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/master/deep/nesting") + assert.EqualValues(t, "repo59/deep/nesting at master - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("Non-default branch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/cake-recipe/deep/nesting") + assert.EqualValues(t, "repo59/deep/nesting at cake-recipe - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("Commit", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/commit/d8f53dfb33f6ccf4169c34970b5e747511c18beb/deep/nesting/") + assert.EqualValues(t, "repo59/deep/nesting at d8f53dfb33f6ccf4169c34970b5e747511c18beb - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("Tag", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/tag/v1.0/deep/nesting/") + assert.EqualValues(t, "repo59/deep/nesting at v1.0 - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + }) + t.Run("File", func(t *testing.T) { + t.Run("Default branch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/master/deep/nesting/folder/secret_sauce_recipe.txt") + assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at master - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("Non-default branch", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/cake-recipe/deep/nesting/folder/secret_sauce_recipe.txt") + assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at cake-recipe - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("Commit", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/commit/d8f53dfb33f6ccf4169c34970b5e747511c18beb/deep/nesting/folder/secret_sauce_recipe.txt") + assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at d8f53dfb33f6ccf4169c34970b5e747511c18beb - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("Tag", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/tag/v1.0/deep/nesting/folder/secret_sauce_recipe.txt") + assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at v1.0 - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle) + }) + }) + }) + + t.Run("Issues view", func(t *testing.T) { + t.Run("Overview page", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/issues") + assert.EqualValues(t, "Issues - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("View issue page", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/issues/1") + assert.EqualValues(t, "#1 - issue1 - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle) + }) + }) + + t.Run("Pull requests view", func(t *testing.T) { + t.Run("Overview page", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/pulls") + assert.EqualValues(t, "Pull Requests - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle) + }) + t.Run("View pull request", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/pulls/2") + assert.EqualValues(t, "#2 - issue2 - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle) + }) + }) +} From a64426907de788cc0937a7a2b16af4d2f26f7fe6 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 18 Aug 2023 04:39:23 +0200 Subject: [PATCH 08/17] [GITEA] Add slow SQL query warning - Databases are one of the most important parts of Forgejo, every interaction with Forgejo uses the database in one way or another. Therefore, it is important to maintain the database and recognize when Forgejo is not doing well with the database. Forgejo already has the option to log *every* SQL query along with its execution time, but monitoring becomes impractical for larger instances and takes up unnecessary storage in the logs. - Add a QoL enhancement that allows instance administrators to specify a threshold value beyond which query execution time is logged as a warning in the xorm logger. The default value is a conservative five seconds to avoid this becoming a source of spam in the logs. - The use case for this patch is that with an instance the size of Codeberg, monitoring SQL logs is not very fruitful and most of them are uninteresting. Recently, in the context of persistent deadlock issues (https://codeberg.org/forgejo/forgejo/issues/220), I have noticed that certain queries hold locks on tables like comment and issue for several seconds. This patch helps to identify which queries these are and when they happen. - Added unit test. (cherry picked from commit 24bbe7886fb4cb9a38c8dab8c44f4c9cbfa25481) (cherry picked from commit 6e29145b3c1455498531593d38e6a914941a12cb) (cherry picked from commit 63731e30712872bd2395eb3cf36d9996e5793645) (cherry picked from commit 3ce1a097369c132654de70df707b867e47bd1c40) --- models/db/engine.go | 28 +++++++++++++++++++++++++++ models/db/engine_test.go | 38 +++++++++++++++++++++++++++++++++++++ modules/setting/database.go | 2 ++ 3 files changed, 68 insertions(+) diff --git a/models/db/engine.go b/models/db/engine.go index b5a41f93e3..0fcd4849bf 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -11,10 +11,13 @@ import ( "io" "reflect" "strings" + "time" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "xorm.io/xorm" + "xorm.io/xorm/contexts" "xorm.io/xorm/names" "xorm.io/xorm/schemas" @@ -147,6 +150,13 @@ func InitEngine(ctx context.Context) error { xormEngine.SetConnMaxLifetime(setting.Database.ConnMaxLifetime) xormEngine.SetDefaultContext(ctx) + if setting.Database.SlowQueryTreshold > 0 { + xormEngine.AddHook(&SlowQueryHook{ + Treshold: setting.Database.SlowQueryTreshold, + Logger: log.GetLogger("xorm"), + }) + } + SetDefaultEngine(ctx, xormEngine) return nil } @@ -300,3 +310,21 @@ func SetLogSQL(ctx context.Context, on bool) { sess.Engine().ShowSQL(on) } } + +type SlowQueryHook struct { + Treshold time.Duration + Logger log.Logger +} + +var _ contexts.Hook = &SlowQueryHook{} + +func (SlowQueryHook) BeforeProcess(c *contexts.ContextHook) (context.Context, error) { + return c.Ctx, nil +} + +func (h *SlowQueryHook) AfterProcess(c *contexts.ContextHook) error { + if c.ExecuteTime >= h.Treshold { + h.Logger.Log(8, log.WARN, "[Slow SQL Query] %s %v - %v", c.SQL, c.Args, c.ExecuteTime) + } + return nil +} diff --git a/models/db/engine_test.go b/models/db/engine_test.go index c9ae5f1542..ba922821b0 100644 --- a/models/db/engine_test.go +++ b/models/db/engine_test.go @@ -6,15 +6,19 @@ package db_test import ( "path/filepath" "testing" + "time" "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" _ "code.gitea.io/gitea/cmd" // for TestPrimaryKeys "github.com/stretchr/testify/assert" + "xorm.io/xorm" ) func TestDumpDatabase(t *testing.T) { @@ -85,3 +89,37 @@ func TestPrimaryKeys(t *testing.T) { } } } + +func TestSlowQuery(t *testing.T) { + lc, cleanup := test.NewLogChecker("slow-query") + lc.StopMark("[Slow SQL Query]") + defer cleanup() + + e := db.GetEngine(db.DefaultContext) + engine, ok := e.(*xorm.Engine) + assert.True(t, ok) + + // It's not possible to clean this up with XORM, but it's luckily not harmful + // to leave around. + engine.AddHook(&db.SlowQueryHook{ + Treshold: time.Second * 10, + Logger: log.GetLogger("slow-query"), + }) + + // NOOP query. + e.Exec("SELECT 1 WHERE false;") + + _, stopped := lc.Check(100 * time.Millisecond) + assert.False(t, stopped) + + engine.AddHook(&db.SlowQueryHook{ + Treshold: 0, // Every query should be logged. + Logger: log.GetLogger("slow-query"), + }) + + // NOOP query. + e.Exec("SELECT 1 WHERE false;") + + _, stopped = lc.Check(100 * time.Millisecond) + assert.True(t, stopped) +} diff --git a/modules/setting/database.go b/modules/setting/database.go index 709655368c..219c96e4d7 100644 --- a/modules/setting/database.go +++ b/modules/setting/database.go @@ -44,6 +44,7 @@ var ( ConnMaxLifetime time.Duration IterateBufferSize int AutoMigration bool + SlowQueryTreshold time.Duration }{ Timeout: 500, IterateBufferSize: 50, @@ -86,6 +87,7 @@ func loadDBSetting(rootCfg ConfigProvider) { Database.DBConnectRetries = sec.Key("DB_RETRIES").MustInt(10) Database.DBConnectBackoff = sec.Key("DB_RETRY_BACKOFF").MustDuration(3 * time.Second) Database.AutoMigration = sec.Key("AUTO_MIGRATION").MustBool(true) + Database.SlowQueryTreshold = sec.Key("SLOW_QUERY_TRESHOLD").MustDuration(5 * time.Second) } // DBConnStr returns database connection string From 9d0bee784d387fac026d3dcc09f10e496a99a7c5 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 18 Aug 2023 15:18:23 +0200 Subject: [PATCH 09/17] [GITEA] Use vertical tabs on issue filters - This is actually https://github.com/go-gitea/gitea/pull/19978 & https://github.com/go-gitea/gitea/pull/19486 but was removed in one of the UI refactors of v1.20 - This is a very technical fix and is best explained in the CSS comments. But the short version: When there's an overflow being set, but you want an element to 'break out' of that overflow with `position: absolute`, it sometimes doesn't work! You need to set some CSS to let the browser know that the element needs to use an element outside of that overflow as 'clip parent'. - Resolves my internal frustration with the mobile UI constantly getting broken. (cherry picked from commit 879f842bed999190506e1d60508e7aede1a4be21) (cherry picked from commit 6099c9b41b9a135fb14b41304459056050abbbe2) (cherry picked from commit 0749d00b160033de0457e17baa1e000e68810d94) (cherry picked from commit ec6a5428a7f05d8e6d2e8a6c476b2b9d35656b0f) --- web_src/css/repo/issue-list.css | 35 +++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/web_src/css/repo/issue-list.css b/web_src/css/repo/issue-list.css index 17ae6ea38f..6afdc79bd4 100644 --- a/web_src/css/repo/issue-list.css +++ b/web_src/css/repo/issue-list.css @@ -17,10 +17,6 @@ } @media (max-width: 767.98px) { - .issue-list-toolbar-right .dropdown .menu { - left: auto !important; - right: auto !important; - } .issue-list-navbar { order: 0; } @@ -31,6 +27,37 @@ .issue-list-search { order: 2 !important; } + /* Don't use flex wrap on mobile as it takes too much vertical space. + * Only set overflow properties on mobile screens, because while the + * CSS trick to pop out from overflowing works on desktop screen, it + * has a massive flaw that it cannot inherited any max width from it's 'real' + * parent and therefor ends up taking more vertical space than is desired. + **/ + .issue-list-toolbar-right .filter.menu { + flex-wrap: nowrap; + overflow-x: auto; + overflow-y: hidden; + } + + /* The following few CSS was created with care and built with the information + * from CSS-Tricks: https://css-tricks.com/popping-hidden-overflow/ + */ + + /* It's important that every element up to .issue-list-toolbar-right doesn't + * have a position set, such that element that wants to pop out will use + * .issue-list-toolbar-right as 'clip parent' and thereby avoids the + * overflow-y: hidden. + */ + .issue-list-toolbar-right .filter.menu > .dropdown.item { + position: initial; + } + /* It's important that this element and not an child has `position` set. + * Set width so that overflow-x knows where to stop overflowing. + */ + .issue-list-toolbar-right { + position: relative; + width: 100%; + } } #issue-list .flex-item-title .labels-list { From 601f00e5306133fc49488f664ded1b346b5894ff Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 19 Aug 2023 12:58:17 +0200 Subject: [PATCH 10/17] [GITEA] Add anchor to review types - The review type '22' is a general comment type that is attached to single codecomments, reviews with multiple comments or to simple approve and request changes comment. This comment can be used to create a link towards this action on an pull request. - Adds an anchor to the review comment type, so that when its getting linked to it, it actually jumps towards that event. - This also now fixes the behavior that after you created a review you will be redirected to that review and because this is an general comment type other mails will also be 'fixed' such as the approved or request changes. - Resolves https://codeberg.org/forgejo/forgejo/issues/1248 (cherry picked from commit b0c3075a794be3ccc768b5018f56ace6f12c11e1) (cherry picked from commit f61505281ced3b19fb76267b81aef0939629bfb4) (cherry picked from commit 1741a5f1fe6adc68bb5f87bdd1c5bdc5bfaa45c7) (cherry picked from commit 5ee4cf2ed952f043cf2d9d5546d7a6c24c6c1ee7) --- templates/repo/issue/view_content/comments.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index d43979ff59..0f77e0254a 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -364,7 +364,7 @@ {{end}} {{else if eq .Type 22}} -
+
{{if .OriginalAuthor}} {{else}} From 155cc19f75662998fcb2a1a08e345e0724437a58 Mon Sep 17 00:00:00 2001 From: zareck Date: Sun, 20 Aug 2023 11:16:30 -0300 Subject: [PATCH 11/17] [GITEA] add GitHub repo migration test Signed-off-by: zareck (cherry picked from commit f48e3ff0db027c6420446c0bab3089d9a46194a8) Removing comments and make command (cherry picked from commit 7664a423a5abf051383374b4156857e83faee7c0) (cherry picked from commit b2fb43536424f90373fdc177bd2c79c374efd2be) (cherry picked from commit 0a24a819a9561c8355adb00b7b202438c5c1bc1a) --- tests/integration/repo_migrate_test.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/integration/repo_migrate_test.go b/tests/integration/repo_migrate_test.go index 91e2961d6d..9fb7a73379 100644 --- a/tests/integration/repo_migrate_test.go +++ b/tests/integration/repo_migrate_test.go @@ -15,8 +15,8 @@ import ( "github.com/stretchr/testify/assert" ) -func testRepoMigrate(t testing.TB, session *TestSession, cloneAddr, repoName string) *httptest.ResponseRecorder { - req := NewRequest(t, "GET", fmt.Sprintf("/repo/migrate?service_type=%d", structs.PlainGitService)) // render plain git migration page +func testRepoMigrate(t testing.TB, session *TestSession, cloneAddr, repoName string, service structs.GitServiceType) *httptest.ResponseRecorder { + req := NewRequest(t, "GET", fmt.Sprintf("/repo/migrate?service_type=%d", service)) // render plain git migration page resp := session.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) @@ -31,7 +31,7 @@ func testRepoMigrate(t testing.TB, session *TestSession, cloneAddr, repoName str "clone_addr": cloneAddr, "uid": uid, "repo_name": repoName, - "service": fmt.Sprintf("%d", structs.PlainGitService), + "service": fmt.Sprintf("%d", service), }) resp = session.MakeRequest(t, req, http.StatusSeeOther) @@ -41,5 +41,17 @@ func testRepoMigrate(t testing.TB, session *TestSession, cloneAddr, repoName str func TestRepoMigrate(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") - testRepoMigrate(t, session, "https://github.com/go-gitea/test_repo.git", "git") + for _, s := range []struct { + testName string + cloneAddr string + repoName string + service structs.GitServiceType + }{ + {"TestMigrateGithub", "https://github.com/go-gitea/test_repo.git", "git", structs.PlainGitService}, + {"TestMigrateGithub", "https://github.com/go-gitea/test_repo.git", "github", structs.GithubService}, + } { + t.Run(s.testName, func(t *testing.T) { + testRepoMigrate(t, session, s.cloneAddr, s.repoName, s.service) + }) + } } From f3c615e7d254e259357ff0b75873cb1551d8ccf5 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 29 Aug 2023 10:33:02 +0200 Subject: [PATCH 12/17] [GITEA] Show manual cron run's last time (squash) 27 jobs in cron fixtures (cherry picked from commit bb71759454071a3f555afdb33702e2af6037fc9b) (cherry picked from commit cc258d342e80dd40d8107b6581c452d2b71c2598) --- tests/integration/api_admin_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 4e222e22e6..85b9103e53 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -298,11 +298,11 @@ func TestAPICron(t *testing.T) { req := NewRequest(t, "GET", urlStr) resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "26", resp.Header().Get("X-Total-Count")) + assert.Equal(t, "27", resp.Header().Get("X-Total-Count")) var crons []api.Cron DecodeJSON(t, resp, &crons) - assert.Len(t, crons, 26) + assert.Len(t, crons, 27) }) t.Run("Execute", func(t *testing.T) { From f35c31b9beef33e0aafc4e62e5b8fa38b3623494 Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 29 Aug 2023 22:00:16 +0200 Subject: [PATCH 13/17] [GITEA] Simplify cron list test (squash) - As per https://codeberg.org/forgejo/forgejo/pulls/1352#issuecomment-1076074 - Only test if the returned stuff is correct, not necessarily accurate. (cherry picked from commit 55bcaf60ec21aec000ff79635981a16566c77d43) (cherry picked from commit 4f03e4810619455e7dca65741cb53a9fd2d98bfe) --- tests/integration/api_admin_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 85b9103e53..423a27eb52 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -298,11 +298,12 @@ func TestAPICron(t *testing.T) { req := NewRequest(t, "GET", urlStr) resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "27", resp.Header().Get("X-Total-Count")) + assert.NotEmpty(t, resp.Header().Get("X-Total-Count")) var crons []api.Cron DecodeJSON(t, resp, &crons) - assert.Len(t, crons, 27) + + assert.NotEmpty(t, crons) }) t.Run("Execute", func(t *testing.T) { From 077b1c52b6e330a66aa55c4e29562278e94026d1 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 30 Aug 2023 15:21:18 +0200 Subject: [PATCH 14/17] [GITEA] [picture].*AVATAR_UPLOAD_PATH is legacy (cherry picked from commit cb4cc01825458752efe01628f705b4f8676e49a2) (cherry picked from commit bef11d61318a462e34202f78fad7f883b0756a88) --- custom/conf/app.example.ini | 3 --- 1 file changed, 3 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index cf3ff6cd43..cc02cbda3b 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1769,9 +1769,6 @@ LEVEL = Info ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; -;AVATAR_UPLOAD_PATH = data/avatars -;REPOSITORY_AVATAR_UPLOAD_PATH = data/repo-avatars -;; ;; How Gitea deals with missing repository avatars ;; none = no avatar will be displayed; random = random avatar will be displayed; image = default image will be used ;REPOSITORY_AVATAR_FALLBACK = none From a0c26f687087e0d146d1298f51dc198d90946393 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 31 Aug 2023 22:59:20 +0200 Subject: [PATCH 15/17] [GITEA] S3: log human readable error on connection failure Should BucketExists (HeadBucket) fail because of an error related to the connection rather than the existence of the bucket, no information is available and the admin is left guessing. https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadBucket.html > This action is useful to determine if a bucket exists and you have > permission to access it. The action returns a 200 OK if the bucket > exists and you have permission to access it. > > If the bucket does not exist or you do not have permission to access > it, the HEAD request returns a generic 400 Bad Request, 403 > Forbidden or 404 Not Found code. A message body is not included, so > you cannot determine the exception beyond these error codes. GetBucketVersioning is used instead and exclusively dedicated to asserting if using the connection does not return a BadRequest. If it does the NewMinioStorage logs an error and returns. Otherwise it keeps going knowing that BucketExists is not going to fail for reasons unrelated to the existence of the bucket and the permissions to access it. (cherry picked from commit de599246059bf94c2a2ce89fdb753b6d105d83ea) --- modules/storage/minio.go | 18 ++++++++++++++++ modules/storage/minio_test.go | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 3246993bb1..26f947fdfe 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -71,6 +71,11 @@ func convertMinioErr(err error) error { return err } +var getBucketVersioning = func(ctx context.Context, minioClient *minio.Client, bucket string) error { + _, err := minioClient.GetBucketVersioning(ctx, bucket) + return err +} + // NewMinioStorage returns a minio storage func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, error) { config := cfg.MinioConfig @@ -90,6 +95,19 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, return nil, convertMinioErr(err) } + // Check if the connection works + err = getBucketVersioning(ctx, minioClient, config.Bucket) + if err != nil { + errResp, ok := err.(minio.ErrorResponse) + if !ok { + return nil, err + } + if errResp.StatusCode == http.StatusBadRequest { + log.Error("S3 storage connection failure at %s:%s with base path %s and region: %s", config.Endpoint, config.Bucket, config.Location, errResp.Message) + return nil, err + } + } + // Check to see if we already own this bucket exists, err := minioClient.BucketExists(ctx, config.Bucket) if err != nil { diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 8fdf31e6cf..bb1d0e954a 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -4,10 +4,17 @@ package storage import ( + "context" + "net/http" "os" "testing" + "time" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + + "github.com/minio/minio-go/v7" + "github.com/stretchr/testify/assert" ) func TestMinioStorageIterator(t *testing.T) { @@ -25,3 +32,35 @@ func TestMinioStorageIterator(t *testing.T) { }, }) } + +func TestS3StorageBadRequest(t *testing.T) { + if os.Getenv("CI") == "" { + t.Skip("S3Storage not present outside of CI") + return + } + lc, cleanup := test.NewLogChecker("bad-request") + lc.StopMark("S3 storage connection failure") + defer cleanup() + cfg := &setting.Storage{ + MinioConfig: setting.MinioStorageConfig{ + Endpoint: "minio:9000", + AccessKeyID: "123456", + SecretAccessKey: "12345678", + Bucket: "bucket", + Location: "us-east-1", + }, + } + message := "ERROR" + defer test.MockVariableValue(&getBucketVersioning, func(ctx context.Context, minioClient *minio.Client, bucket string) error { + return minio.ErrorResponse{ + StatusCode: http.StatusBadRequest, + Code: "FixtureError", + Message: message, + } + })() + _, err := NewStorage(setting.MinioStorageType, cfg) + assert.ErrorContains(t, err, message) + + _, stopped := lc.Check(100 * time.Millisecond) + assert.False(t, stopped) +} From de7e33e7a5b6c63b68d829e724639bd80811cce5 Mon Sep 17 00:00:00 2001 From: Lars Lehtonen Date: Wed, 6 Sep 2023 09:08:28 -0700 Subject: [PATCH 16/17] [GITEA] services/wiki: Close() after error handling Refs: https://codeberg.org/forgejo/forgejo/pulls/1385 Signed-off-by: Lars Lehtonen (cherry picked from commit c6a85d760693bcecb0a5df24bfe0ee662c593725) (cherry picked from commit fc065c8294322b2d8bd3dbff524bf3f0669bbaec) --- services/wiki/wiki_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go index 0621456f3e..6f05dedab6 100644 --- a/services/wiki/wiki_test.go +++ b/services/wiki/wiki_test.go @@ -251,8 +251,8 @@ func TestPrepareWikiFileName(t *testing.T) { unittest.PrepareTestEnv(t) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) gitRepo, err := git.OpenRepository(git.DefaultContext, repo.WikiPath()) - defer gitRepo.Close() assert.NoError(t, err) + defer gitRepo.Close() tests := []struct { name string @@ -303,8 +303,8 @@ func TestPrepareWikiFileName_FirstPage(t *testing.T) { assert.NoError(t, err) gitRepo, err := git.OpenRepository(git.DefaultContext, tmpDir) - defer gitRepo.Close() assert.NoError(t, err) + defer gitRepo.Close() existence, newWikiPath, err := prepareGitPath(gitRepo, "Home") assert.False(t, existence) From 7ea66ee1c5dd21d9e6a43f961e8adc71ec79b806 Mon Sep 17 00:00:00 2001 From: Aravinth Manivannan Date: Thu, 7 Sep 2023 07:11:29 +0000 Subject: [PATCH 17/17] [GITEA] notifies admins on new user registration Sends email with information on the new user (time of creation and time of last sign-in) and a link to manage the new user from the admin panel closes: https://codeberg.org/forgejo/forgejo/issues/480 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1371 Co-authored-by: Aravinth Manivannan Co-committed-by: Aravinth Manivannan (cherry picked from commit c721aa828ba6aec5ef95459cfc632a0a1f7463e9) (cherry picked from commit 6487efcb9da61be1f802f1cd8007330153322770) Conflicts: modules/notification/base/notifier.go modules/notification/base/null.go modules/notification/notification.go https://codeberg.org/forgejo/forgejo/pulls/1422 --- custom/conf/app.example.ini | 2 + modules/setting/admin.go | 1 + options/locale/locale_en-US.ini | 4 + routers/web/auth/auth.go | 3 +- services/mailer/mail_admin_new_user.go | 82 +++++++++++++++++++ services/mailer/mail_admin_new_user_test.go | 88 +++++++++++++++++++++ services/mailer/notify.go | 4 + services/notify/notifier.go | 1 + services/notify/notify.go | 7 ++ services/notify/null.go | 4 + templates/mail/admin_new_user.tmpl | 22 ++++++ 11 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 services/mailer/mail_admin_new_user.go create mode 100644 services/mailer/mail_admin_new_user_test.go create mode 100644 templates/mail/admin_new_user.tmpl diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index cc02cbda3b..6b603f9d42 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1455,6 +1455,8 @@ LEVEL = Info ;; ;; Default configuration for email notifications for users (user configurable). Options: enabled, onmention, disabled ;DEFAULT_EMAIL_NOTIFICATIONS = enabled +;; Send email notifications to all instance admins on new user sign-ups. Options: enabled, true, false +;NOTIFY_NEW_SIGN_UPS = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/setting/admin.go b/modules/setting/admin.go index 2d2dd26de9..4e2f343715 100644 --- a/modules/setting/admin.go +++ b/modules/setting/admin.go @@ -7,6 +7,7 @@ package setting var Admin struct { DisableRegularOrgCreation bool DefaultEmailNotification string + NotifyNewSignUps bool } func loadAdminFrom(rootCfg ConfigProvider) { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9efe07a3d7..71b2a45107 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -438,6 +438,10 @@ activate_email = Verify your email address activate_email.title = %s, please verify your email address activate_email.text = Please click the following link to verify your email address within %s: +admin.new_user.subject = New user %s +admin.new_user.user_info = User Information +admin.new_user.text = Please click here to manage the user from the admin panel. + register_notify = Welcome to Gitea register_notify.title = %[1]s, welcome to %[2]s register_notify.text_1 = this is your registration confirmation email for %s! diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index c20a45ebc9..3f1d7881fe 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -30,6 +30,7 @@ import ( "code.gitea.io/gitea/services/externalaccount" "code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/mailer" + notify_service "code.gitea.io/gitea/services/notify" "github.com/markbates/goth" ) @@ -568,6 +569,7 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. } } + notify_service.NewUserSignUp(ctx, u) // update external user information if gothUser != nil { if err := externalaccount.UpdateExternalUser(u, *gothUser); err != nil { @@ -591,7 +593,6 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. ctx.Data["Email"] = u.Email ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale) ctx.HTML(http.StatusOK, TplActivate) - if setting.CacheService.Enabled { if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil { log.Error("Set cache(MailResendLimit) fail: %v", err) diff --git a/services/mailer/mail_admin_new_user.go b/services/mailer/mail_admin_new_user.go new file mode 100644 index 0000000000..332afa5e49 --- /dev/null +++ b/services/mailer/mail_admin_new_user.go @@ -0,0 +1,82 @@ +// Copyright 2023 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package mailer + +import ( + "bytes" + "context" + "strconv" + + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/translation" +) + +const ( + tplNewUserMail base.TplName = "admin_new_user" +) + +var sa = SendAsyncs + +// MailNewUser sends notification emails on new user registrations to all admins +func MailNewUser(ctx context.Context, u *user_model.User) { + if !setting.Admin.NotifyNewSignUps { + return + } + + if setting.MailService == nil { + // No mail service configured + return + } + + recipients, err := user_model.GetAllUsers() + if err != nil { + log.Error("user_model.GetAllUsers: %v", err) + return + } + + langMap := make(map[string][]string) + for _, r := range recipients { + if r.IsAdmin { + langMap[r.Language] = append(langMap[r.Language], r.Email) + } + } + + for lang, tos := range langMap { + mailNewUser(ctx, u, lang, tos) + } +} + +func mailNewUser(ctx context.Context, u *user_model.User, lang string, tos []string) { + locale := translation.NewLocale(lang) + + subject := locale.Tr("mail.admin.new_user.subject", u.Name) + manageUserURL := setting.AppSubURL + "/admin/users/" + strconv.FormatInt(u.ID, 10) + body := locale.Tr("mail.admin.new_user.text", manageUserURL) + mailMeta := map[string]any{ + "NewUser": u, + "Subject": subject, + "Body": body, + "Language": locale.Language(), + "locale": locale, + "Str2html": templates.Str2html, + } + + var mailBody bytes.Buffer + + if err := bodyTemplates.ExecuteTemplate(&mailBody, string(tplNewUserMail), mailMeta); err != nil { + log.Error("ExecuteTemplate [%s]: %v", string(tplNewUserMail)+"/body", err) + return + } + + msgs := make([]*Message, 0, len(tos)) + for _, to := range tos { + msg := NewMessage(to, subject, mailBody.String()) + msg.Info = subject + msgs = append(msgs, msg) + } + sa(msgs) +} diff --git a/services/mailer/mail_admin_new_user_test.go b/services/mailer/mail_admin_new_user_test.go new file mode 100644 index 0000000000..0772bb0da5 --- /dev/null +++ b/services/mailer/mail_admin_new_user_test.go @@ -0,0 +1,88 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package mailer + +import ( + "context" + "strconv" + "strings" + "testing" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func getTestUsers() []*user_model.User { + admin := new(user_model.User) + admin.Name = "admin" + admin.IsAdmin = true + admin.Language = "en_US" + admin.Email = "admin@forgejo.org" + + newUser := new(user_model.User) + newUser.Name = "new_user" + newUser.Language = "en_US" + newUser.IsAdmin = false + newUser.Email = "new_user@forgejo.org" + newUser.LastLoginUnix = 1693648327 + newUser.CreatedUnix = 1693648027 + + user_model.CreateUser(admin) + user_model.CreateUser(newUser) + + users := make([]*user_model.User, 0) + users = append(users, admin) + users = append(users, newUser) + + return users +} + +func cleanUpUsers(ctx context.Context, users []*user_model.User) { + for _, u := range users { + db.DeleteByID(ctx, u.ID, new(user_model.User)) + } +} + +func TestAdminNotificationMail_test(t *testing.T) { + mailService := setting.Mailer{ + From: "test@forgejo.org", + Protocol: "dummy", + } + + setting.MailService = &mailService + setting.Domain = "localhost" + setting.AppSubURL = "http://localhost" + + // test with NOTIFY_NEW_SIGNUPS enabled + setting.Admin.NotifyNewSignUps = true + + ctx := context.Background() + NewContext(ctx) + + users := getTestUsers() + oldSendAsyncs := sa + defer func() { + sa = oldSendAsyncs + cleanUpUsers(ctx, users) + }() + + sa = func(msgs []*Message) { + assert.Equal(t, len(msgs), 1, "Test provides only one admin user, so only one email must be sent") + assert.Equal(t, msgs[0].To, users[0].Email, "checks if the recipient is the admin of the instance") + manageUserURL := "/admin/users/" + strconv.FormatInt(users[1].ID, 10) + assert.True(t, strings.ContainsAny(msgs[0].Body, manageUserURL), "checks if the message contains the link to manage the newly created user from the admin panel") + } + MailNewUser(ctx, users[1]) + + // test with NOTIFY_NEW_SIGNUPS disabled; emails shouldn't be sent + setting.Admin.NotifyNewSignUps = false + sa = func(msgs []*Message) { + assert.Equal(t, 1, 0, "this shouldn't execute. MailNewUser must exit early since NOTIFY_NEW_SIGNUPS is disabled") + } + + MailNewUser(ctx, users[1]) +} diff --git a/services/mailer/notify.go b/services/mailer/notify.go index f0419e2cbb..0b7e8178e6 100644 --- a/services/mailer/notify.go +++ b/services/mailer/notify.go @@ -199,3 +199,7 @@ func (m *mailNotifier) RepoPendingTransfer(ctx context.Context, doer, newOwner * log.Error("SendRepoTransferNotifyMail: %v", err) } } + +func (m *mailNotifier) NewUserSignUp(ctx context.Context, newUser *user_model.User) { + MailNewUser(ctx, newUser) +} diff --git a/services/notify/notifier.go b/services/notify/notifier.go index d1dbe44c11..a65dc22dd6 100644 --- a/services/notify/notifier.go +++ b/services/notify/notifier.go @@ -72,4 +72,5 @@ type Notifier interface { PackageCreate(ctx context.Context, doer *user_model.User, pd *packages_model.PackageDescriptor) PackageDelete(ctx context.Context, doer *user_model.User, pd *packages_model.PackageDescriptor) + NewUserSignUp(ctx context.Context, newUser *user_model.User) } diff --git a/services/notify/notify.go b/services/notify/notify.go index 71bc1c7d58..6410e15a5b 100644 --- a/services/notify/notify.go +++ b/services/notify/notify.go @@ -360,3 +360,10 @@ func PackageDelete(ctx context.Context, doer *user_model.User, pd *packages_mode notifier.PackageDelete(ctx, doer, pd) } } + +// NewUserSignUp notifies deletion of a package to notifiers +func NewUserSignUp(ctx context.Context, newUser *user_model.User) { + for _, notifier := range notifiers { + notifier.NewUserSignUp(ctx, newUser) + } +} diff --git a/services/notify/null.go b/services/notify/null.go index c5b31f83d6..3535e014c8 100644 --- a/services/notify/null.go +++ b/services/notify/null.go @@ -204,3 +204,7 @@ func (*NullNotifier) PackageCreate(ctx context.Context, doer *user_model.User, p // PackageDelete places a place holder function func (*NullNotifier) PackageDelete(ctx context.Context, doer *user_model.User, pd *packages_model.PackageDescriptor) { } + +// NotifyNewUserSignUp notifies deletion of a package to notifiers +func (*NullNotifier) NewUserSignUp(ctx context.Context, newUser *user_model.User) { +} diff --git a/templates/mail/admin_new_user.tmpl b/templates/mail/admin_new_user.tmpl new file mode 100644 index 0000000000..58b5c264e7 --- /dev/null +++ b/templates/mail/admin_new_user.tmpl @@ -0,0 +1,22 @@ + + + + + {{.Subject}} + + + + + + +
    +

    {{.locale.Tr "mail.admin.new_user.user_info"}}

    +
  • {{.locale.Tr "admin.users.created"}}: {{DateTime "full" .NewUser.LastLoginUnix}}
    • +
  • {{.locale.Tr "admin.users.last_login"}}: {{DateTime "full" .NewUser.CreatedUnix}}
    • +
+

{{.Body | Str2html}}

+ +