From dd3c4d7096cff91854bcc6641f55d9d093e5c86e Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Mon, 11 Nov 2024 07:37:24 +0800 Subject: [PATCH 01/11] Add a doctor check to disable the "Actions" unit for mirrors (#32424) Resolve #32232 Users can disable the "Actions" unit for all mirror repos by running ``` gitea doctor check --run disable-mirror-actions-unit --fix ``` (cherry picked from commit a910abbb451ea89b8279b43bd818a140fe0f3b51) --- services/doctor/actions.go | 70 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 services/doctor/actions.go diff --git a/services/doctor/actions.go b/services/doctor/actions.go new file mode 100644 index 0000000000..7c44fb8392 --- /dev/null +++ b/services/doctor/actions.go @@ -0,0 +1,70 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package doctor + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + unit_model "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" + repo_service "code.gitea.io/gitea/services/repository" +) + +func disableMirrorActionsUnit(ctx context.Context, logger log.Logger, autofix bool) error { + var reposToFix []*repo_model.Repository + + for page := 1; ; page++ { + repos, _, err := repo_model.SearchRepository(ctx, &repo_model.SearchRepoOptions{ + ListOptions: db.ListOptions{ + PageSize: repo_model.RepositoryListDefaultPageSize, + Page: page, + }, + Mirror: optional.Some(true), + }) + if err != nil { + return fmt.Errorf("SearchRepository: %w", err) + } + if len(repos) == 0 { + break + } + + for _, repo := range repos { + if repo.UnitEnabled(ctx, unit_model.TypeActions) { + reposToFix = append(reposToFix, repo) + } + } + } + + if len(reposToFix) == 0 { + logger.Info("Found no mirror with actions unit enabled") + } else { + logger.Warn("Found %d mirrors with actions unit enabled", len(reposToFix)) + } + if !autofix || len(reposToFix) == 0 { + return nil + } + + for _, repo := range reposToFix { + if err := repo_service.UpdateRepositoryUnits(ctx, repo, nil, []unit_model.Type{unit_model.TypeActions}); err != nil { + return err + } + } + logger.Info("Fixed %d mirrors with actions unit enabled", len(reposToFix)) + + return nil +} + +func init() { + Register(&Check{ + Title: "Disable the actions unit for all mirrors", + Name: "disable-mirror-actions-unit", + IsDefault: false, + Run: disableMirrorActionsUnit, + Priority: 9, + }) +} From 7f51210672031aee7a790455d51a17ce11a70559 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Mon, 11 Nov 2024 05:58:37 +0100 Subject: [PATCH 02/11] Harden runner updateTask and updateLog api (#32462) Per proposal https://github.com/go-gitea/gitea/issues/32461 (cherry picked from commit f888e45432ccb86b18e6709fbd25223e07f2c422) --- models/actions/task.go | 4 +++- routers/api/actions/runner/runner.go | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/models/actions/task.go b/models/actions/task.go index 8d41a631aa..8bd139a2d6 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -341,7 +341,7 @@ func UpdateTask(ctx context.Context, task *ActionTask, cols ...string) error { // UpdateTaskByState updates the task by the state. // It will always update the task if the state is not final, even there is no change. // So it will update ActionTask.Updated to avoid the task being judged as a zombie task. -func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionTask, error) { +func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.TaskState) (*ActionTask, error) { stepStates := map[int64]*runnerv1.StepState{} for _, v := range state.Steps { stepStates[v.Id] = v @@ -360,6 +360,8 @@ func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionT return nil, err } else if !has { return nil, util.ErrNotExist + } else if runnerID != task.RunnerID { + return nil, fmt.Errorf("invalid runner for task") } if task.Status.IsDone() { diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index 017bdf6324..cb47074e98 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -177,7 +177,9 @@ func (s *Service) UpdateTask( ctx context.Context, req *connect.Request[runnerv1.UpdateTaskRequest], ) (*connect.Response[runnerv1.UpdateTaskResponse], error) { - task, err := actions_model.UpdateTaskByState(ctx, req.Msg.State) + runner := GetRunner(ctx) + + task, err := actions_model.UpdateTaskByState(ctx, runner.ID, req.Msg.State) if err != nil { return nil, status.Errorf(codes.Internal, "update task: %v", err) } @@ -239,11 +241,15 @@ func (s *Service) UpdateLog( ctx context.Context, req *connect.Request[runnerv1.UpdateLogRequest], ) (*connect.Response[runnerv1.UpdateLogResponse], error) { + runner := GetRunner(ctx) + res := connect.NewResponse(&runnerv1.UpdateLogResponse{}) task, err := actions_model.GetTaskByID(ctx, req.Msg.TaskId) if err != nil { return nil, status.Errorf(codes.Internal, "get task: %v", err) + } else if runner.ID != task.RunnerID { + return nil, status.Errorf(codes.Internal, "invalid runner for task") } ack := task.LogLength From 56971f9ed90a01fd74a634b7496593e6f62ac260 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Nov 2024 13:33:35 -0800 Subject: [PATCH 03/11] Disable Oauth check if oauth disabled (#32368) Fix #32367 --------- Co-authored-by: Giteabot Co-authored-by: wxiaoguang (cherry picked from commit 840ad7eefe2b49ab453b9a89b153a264a8c9f8a2) Conflicts: services/auth/oauth2.go trivial context conflict --- routers/web/web.go | 68 ++++++++++++++++++++++------------------- services/auth/oauth2.go | 3 ++ 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index ecdd5d8d92..e44716236c 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -327,6 +327,13 @@ func registerRoutes(m *web.Route) { } } + oauth2Enabled := func(ctx *context.Context) { + if !setting.OAuth2.Enabled { + ctx.Error(http.StatusForbidden) + return + } + } + reqMilestonesDashboardPageEnabled := func(ctx *context.Context) { if !setting.Service.ShowMilestonesDashboardPage { ctx.Error(http.StatusForbidden) @@ -516,16 +523,18 @@ func registerRoutes(m *web.Route) { m.Any("/user/events", routing.MarkLongPolling, events.Events) m.Group("/login/oauth", func() { - m.Get("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) - m.Post("/grant", web.Bind(forms.GrantApplicationForm{}), auth.GrantApplicationOAuth) - // TODO manage redirection - m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) - }, ignSignInAndCsrf, reqSignIn) + m.Group("", func() { + m.Get("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) + m.Post("/grant", web.Bind(forms.GrantApplicationForm{}), auth.GrantApplicationOAuth) + // TODO manage redirection + m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) + }, ignSignInAndCsrf, reqSignIn) - m.Methods("GET, OPTIONS", "/login/oauth/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth) - m.Methods("POST, OPTIONS", "/login/oauth/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth) - m.Methods("GET, OPTIONS", "/login/oauth/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys) - m.Methods("POST, OPTIONS", "/login/oauth/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth) + m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth) + m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth) + m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys) + m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth) + }, oauth2Enabled) m.Group("/user/settings", func() { m.Get("", user_setting.Profile) @@ -567,17 +576,24 @@ func registerRoutes(m *web.Route) { }, openIDSignInEnabled) m.Post("/account_link", linkAccountEnabled, security.DeleteAccountLink) }) - m.Group("/applications/oauth2", func() { - m.Get("/{id}", user_setting.OAuth2ApplicationShow) - m.Post("/{id}", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsEdit) - m.Post("/{id}/regenerate_secret", user_setting.OAuthApplicationsRegenerateSecret) - m.Post("", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsPost) - m.Post("/{id}/delete", user_setting.DeleteOAuth2Application) - m.Post("/{id}/revoke/{grantId}", user_setting.RevokeOAuth2Grant) + + m.Group("/applications", func() { + // oauth2 applications + m.Group("/oauth2", func() { + m.Get("/{id}", user_setting.OAuth2ApplicationShow) + m.Post("/{id}", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsEdit) + m.Post("/{id}/regenerate_secret", user_setting.OAuthApplicationsRegenerateSecret) + m.Post("", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsPost) + m.Post("/{id}/delete", user_setting.DeleteOAuth2Application) + m.Post("/{id}/revoke/{grantId}", user_setting.RevokeOAuth2Grant) + }, oauth2Enabled) + + // access token applications + m.Combo("").Get(user_setting.Applications). + Post(web.Bind(forms.NewAccessTokenForm{}), user_setting.ApplicationsPost) + m.Post("/delete", user_setting.DeleteApplication) }) - m.Combo("/applications").Get(user_setting.Applications). - Post(web.Bind(forms.NewAccessTokenForm{}), user_setting.ApplicationsPost) - m.Post("/applications/delete", user_setting.DeleteApplication) + m.Combo("/keys").Get(user_setting.Keys). Post(web.Bind(forms.AddKeyForm{}), user_setting.KeysPost) m.Post("/keys/delete", user_setting.DeleteKey) @@ -755,12 +771,7 @@ func registerRoutes(m *web.Route) { m.Post("/regenerate_secret", admin.ApplicationsRegenerateSecret) m.Post("/delete", admin.DeleteApplication) }) - }, func(ctx *context.Context) { - if !setting.OAuth2.Enabled { - ctx.Error(http.StatusForbidden) - return - } - }) + }, oauth2Enabled) m.Group("/actions", func() { m.Get("", admin.RedirectToDefaultSetting) @@ -883,12 +894,7 @@ func registerRoutes(m *web.Route) { m.Post("/regenerate_secret", org.OAuthApplicationsRegenerateSecret) m.Post("/delete", org.DeleteOAuth2Application) }) - }, func(ctx *context.Context) { - if !setting.OAuth2.Enabled { - ctx.Error(http.StatusForbidden) - return - } - }) + }, oauth2Enabled) m.Group("/hooks", func() { m.Get("", org.Webhooks) diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 6a63c62796..8b625a193e 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -68,6 +68,9 @@ func grantAdditionalScopes(grantScopes string) string { // CheckOAuthAccessToken returns uid of user from oauth token // + non default openid scopes requested func CheckOAuthAccessToken(ctx context.Context, accessToken string) (int64, string) { + if !setting.OAuth2.Enabled { + return 0, "" + } // JWT tokens require a "." if !strings.Contains(accessToken, ".") { return 0, "" From 03ab73d92eabaf774278effe3332623b1dc3580a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Nov 2024 12:17:58 +0800 Subject: [PATCH 04/11] Fix nil panic if repo doesn't exist (#32501) fix #32496 (cherry picked from commit 985e2a8af3d6468bac3ab178148c38bdbd8414f5) --- models/activities/action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/activities/action.go b/models/activities/action.go index b6c816f096..dd67b98242 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -250,6 +250,9 @@ func (a *Action) GetActDisplayNameTitle(ctx context.Context) string { // GetRepoUserName returns the name of the action repository owner. func (a *Action) GetRepoUserName(ctx context.Context) string { a.loadRepo(ctx) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.OwnerName } @@ -262,6 +265,9 @@ func (a *Action) ShortRepoUserName(ctx context.Context) string { // GetRepoName returns the name of the action repository. func (a *Action) GetRepoName(ctx context.Context) string { a.loadRepo(ctx) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.Name } From c2e8790df37a14b4d2f72c7377db75309e0ebf1d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 13 Nov 2024 23:19:14 -0800 Subject: [PATCH 05/11] Trim title before insert/update to database to match the size requirements of database (#32498) Fix #32489 (cherry picked from commit 98d9a71ffe510da0e10d042d8f87a348022aca87) --- models/actions/run.go | 3 +++ models/actions/runner.go | 2 ++ models/actions/schedule.go | 2 ++ models/issues/issue_update.go | 4 ++++ models/issues/pull.go | 1 + models/project/project.go | 4 ++++ models/repo/release.go | 1 + services/release/release.go | 1 + 8 files changed, 18 insertions(+) diff --git a/models/actions/run.go b/models/actions/run.go index f637634575..06a1290d5d 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -254,6 +254,7 @@ func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID strin } // InsertRun inserts a run +// The title will be cut off at 255 characters if it's longer than 255 characters. func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWorkflow) error { ctx, commiter, err := db.TxContext(ctx) if err != nil { @@ -266,6 +267,7 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork return err } run.Index = index + run.Title, _ = util.SplitStringAtByteN(run.Title, 255) if err := db.Insert(ctx, run); err != nil { return err @@ -391,6 +393,7 @@ func UpdateRun(ctx context.Context, run *ActionRun, cols ...string) error { if len(cols) > 0 { sess.Cols(cols...) } + run.Title, _ = util.SplitStringAtByteN(run.Title, 255) affected, err := sess.Update(run) if err != nil { return err diff --git a/models/actions/runner.go b/models/actions/runner.go index 175f211c72..a679d7d989 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -271,6 +271,7 @@ func GetRunnerByID(ctx context.Context, id int64) (*ActionRunner, error) { // UpdateRunner updates runner's information. func UpdateRunner(ctx context.Context, r *ActionRunner, cols ...string) error { e := db.GetEngine(ctx) + r.Name, _ = util.SplitStringAtByteN(r.Name, 255) var err error if len(cols) == 0 { _, err = e.ID(r.ID).AllCols().Update(r) @@ -312,6 +313,7 @@ func CreateRunner(ctx context.Context, t *ActionRunner) error { // Remove OwnerID to avoid confusion; it's not worth returning an error here. t.OwnerID = 0 } + t.Name, _ = util.SplitStringAtByteN(t.Name, 255) return db.Insert(ctx, t) } diff --git a/models/actions/schedule.go b/models/actions/schedule.go index acb9961bf6..8b7e6ec44f 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -12,6 +12,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" webhook_module "code.gitea.io/gitea/modules/webhook" ) @@ -67,6 +68,7 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { // Loop through each schedule row for _, row := range rows { + row.Title, _ = util.SplitStringAtByteN(row.Title, 255) // Create new schedule row if err = db.Insert(ctx, row); err != nil { return err diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index dbfd2fc91b..31c8bdc17b 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/references" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -154,6 +155,7 @@ func ChangeIssueTitle(ctx context.Context, issue *Issue, doer *user_model.User, } defer committer.Close() + issue.Title, _ = util.SplitStringAtByteN(issue.Title, 255) if err = UpdateIssueCols(ctx, issue, "name"); err != nil { return fmt.Errorf("updateIssueCols: %w", err) } @@ -409,6 +411,7 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue } // NewIssue creates new issue with labels for repository. +// The title will be cut off at 255 characters if it's longer than 255 characters. func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string) (err error) { ctx, committer, err := db.TxContext(ctx) if err != nil { @@ -422,6 +425,7 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, la } issue.Index = idx + issue.Title, _ = util.SplitStringAtByteN(issue.Title, 255) if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ Repo: repo, diff --git a/models/issues/pull.go b/models/issues/pull.go index e7663ea350..13eafccdc7 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -566,6 +566,7 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Iss } issue.Index = idx + issue.Title, _ = util.SplitStringAtByteN(issue.Title, 255) if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ Repo: repo, diff --git a/models/project/project.go b/models/project/project.go index 8cebf34b5e..245838abb5 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -242,6 +242,7 @@ func GetSearchOrderByBySortType(sortType string) db.SearchOrderBy { } // NewProject creates a new Project +// The title will be cut off at 255 characters if it's longer than 255 characters. func NewProject(ctx context.Context, p *Project) error { if !IsTemplateTypeValid(p.TemplateType) { p.TemplateType = TemplateTypeNone @@ -255,6 +256,8 @@ func NewProject(ctx context.Context, p *Project) error { return util.NewInvalidArgumentErrorf("project type is not valid") } + p.Title, _ = util.SplitStringAtByteN(p.Title, 255) + return db.WithTx(ctx, func(ctx context.Context) error { if err := db.Insert(ctx, p); err != nil { return err @@ -302,6 +305,7 @@ func UpdateProject(ctx context.Context, p *Project) error { p.CardType = CardTypeTextOnly } + p.Title, _ = util.SplitStringAtByteN(p.Title, 255) _, err := db.GetEngine(ctx).ID(p.ID).Cols( "title", "description", diff --git a/models/repo/release.go b/models/repo/release.go index d96eac0d19..38e38c6572 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -171,6 +171,7 @@ func IsReleaseExist(ctx context.Context, repoID int64, tagName string) (bool, er // UpdateRelease updates all columns of a release func UpdateRelease(ctx context.Context, rel *Release) error { + rel.Title, _ = util.SplitStringAtByteN(rel.Title, 255) _, err := db.GetEngine(ctx).ID(rel.ID).AllCols().Update(rel) return err } diff --git a/services/release/release.go b/services/release/release.go index 99851ed1b7..876514beec 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -151,6 +151,7 @@ func CreateRelease(gitRepo *git.Repository, rel *repo_model.Release, msg string, return err } + rel.Title, _ = util.SplitStringAtByteN(rel.Title, 255) rel.LowerTagName = strings.ToLower(rel.TagName) if err = db.Insert(gitRepo.Ctx, rel); err != nil { return err From 96ee0f56475204b2bbdc7f2aeb35b1c32eac469c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Nov 2024 18:13:01 -0800 Subject: [PATCH 06/11] Fix oauth2 error handle not return immediately (#32514) (cherry picked from commit 4121f952d18a4c3a3c08ae645af3458ef08b439d) --- routers/web/auth/oauth.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index a9cbce9a9d..bf197bd14d 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1013,6 +1013,8 @@ func SignInOAuthCallback(ctx *context.Context) { } if err, ok := err.(*go_oauth2.RetrieveError); ok { ctx.Flash.Error("OAuth2 RetrieveError: "+err.Error(), true) + ctx.Redirect(setting.AppSubURL + "/user/login") + return } ctx.ServerError("UserSignIn", err) return From a8f2002a9b061ec1092df67c6f05e30aa7d2e2d2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Nov 2024 20:04:20 -0800 Subject: [PATCH 07/11] Remove transaction for archive download (#32186) Since there is a status column in the database, the transaction is unnecessary when downloading an archive. The transaction is blocking database operations, especially with SQLite. Replace #27563 (cherry picked from commit e1b269e956e955dd1dfb012f40270d73f8329092) --- .deadcode-out | 3 -- services/repository/archiver/archiver.go | 33 ++++++++----------- services/repository/archiver/archiver_test.go | 12 +++---- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/.deadcode-out b/.deadcode-out index c82a229317..2c54b37490 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -289,9 +289,6 @@ code.gitea.io/gitea/services/pull code.gitea.io/gitea/services/repository IsErrForkAlreadyExist -code.gitea.io/gitea/services/repository/archiver - ArchiveRepository - code.gitea.io/gitea/services/repository/files ContentType.String GetFileResponseFromCommit diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index c74712b4ba..73dcb0795f 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -69,7 +69,7 @@ func (e RepoRefNotFoundError) Is(err error) bool { } // NewRequest creates an archival request, based on the URI. The -// resulting ArchiveRequest is suitable for being passed to ArchiveRepository() +// resulting ArchiveRequest is suitable for being passed to Await() // if it's determined that the request still needs to be satisfied. func NewRequest(ctx context.Context, repoID int64, repo *git.Repository, uri string) (*ArchiveRequest, error) { r := &ArchiveRequest{ @@ -168,13 +168,14 @@ func (aReq *ArchiveRequest) Await(ctx context.Context) (*repo_model.RepoArchiver } } +// doArchive satisfies the ArchiveRequest being passed in. Processing +// will occur in a separate goroutine, as this phase may take a while to +// complete. If the archive already exists, doArchive will not do +// anything. In all cases, the caller should be examining the *ArchiveRequest +// being returned for completion, as it may be different than the one they passed +// in. func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver, error) { - txCtx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - ctx, _, finished := process.GetManager().AddContext(txCtx, fmt.Sprintf("ArchiveRequest[%d]: %s", r.RepoID, r.GetArchiveName())) + ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("ArchiveRequest[%d]: %s", r.RepoID, r.GetArchiveName())) defer finished() archiver, err := repo_model.GetRepoArchiver(ctx, r.RepoID, r.Type, r.CommitID) @@ -209,7 +210,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver return nil, err } } - return archiver, committer.Commit() + return archiver, nil } if !errors.Is(err, os.ErrNotExist) { @@ -278,17 +279,7 @@ func doArchive(ctx context.Context, r *ArchiveRequest) (*repo_model.RepoArchiver } } - return archiver, committer.Commit() -} - -// ArchiveRepository satisfies the ArchiveRequest being passed in. Processing -// will occur in a separate goroutine, as this phase may take a while to -// complete. If the archive already exists, ArchiveRepository will not do -// anything. In all cases, the caller should be examining the *ArchiveRequest -// being returned for completion, as it may be different than the one they passed -// in. -func ArchiveRepository(ctx context.Context, request *ArchiveRequest) (*repo_model.RepoArchiver, error) { - return doArchive(ctx, request) + return archiver, nil } var archiverQueue *queue.WorkerPoolQueue[*ArchiveRequest] @@ -298,8 +289,10 @@ func Init(ctx context.Context) error { handler := func(items ...*ArchiveRequest) []*ArchiveRequest { for _, archiveReq := range items { log.Trace("ArchiverData Process: %#v", archiveReq) - if _, err := doArchive(ctx, archiveReq); err != nil { + if archiver, err := doArchive(ctx, archiveReq); err != nil { log.Error("Archive %v failed: %v", archiveReq, err) + } else { + log.Trace("ArchiverData Success: %#v", archiver) } } return nil diff --git a/services/repository/archiver/archiver_test.go b/services/repository/archiver/archiver_test.go index a90bfa7bef..6d9224da9f 100644 --- a/services/repository/archiver/archiver_test.go +++ b/services/repository/archiver/archiver_test.go @@ -81,13 +81,13 @@ func TestArchive_Basic(t *testing.T) { inFlight[1] = tgzReq inFlight[2] = secondReq - ArchiveRepository(db.DefaultContext, zipReq) - ArchiveRepository(db.DefaultContext, tgzReq) - ArchiveRepository(db.DefaultContext, secondReq) + doArchive(db.DefaultContext, zipReq) + doArchive(db.DefaultContext, tgzReq) + doArchive(db.DefaultContext, secondReq) // Make sure sending an unprocessed request through doesn't affect the queue // count. - ArchiveRepository(db.DefaultContext, zipReq) + doArchive(db.DefaultContext, zipReq) // Sleep two seconds to make sure the queue doesn't change. time.Sleep(2 * time.Second) @@ -102,7 +102,7 @@ func TestArchive_Basic(t *testing.T) { // We still have the other three stalled at completion, waiting to remove // from archiveInProgress. Try to submit this new one before its // predecessor has cleared out of the queue. - ArchiveRepository(db.DefaultContext, zipReq2) + doArchive(db.DefaultContext, zipReq2) // Now we'll submit a request and TimedWaitForCompletion twice, before and // after we release it. We should trigger both the timeout and non-timeout @@ -110,7 +110,7 @@ func TestArchive_Basic(t *testing.T) { timedReq, err := NewRequest(ctx, ctx.Repo.Repository.ID, ctx.Repo.GitRepo, secondCommit+".tar.gz") require.NoError(t, err) assert.NotNil(t, timedReq) - ArchiveRepository(db.DefaultContext, timedReq) + doArchive(db.DefaultContext, timedReq) zipReq2, err = NewRequest(ctx, ctx.Repo.Repository.ID, ctx.Repo.GitRepo, firstCommit+".zip") require.NoError(t, err) From 45435a8789f8ff69603799a9031246d2d621d139 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 16 Nov 2024 16:41:44 +0800 Subject: [PATCH 08/11] Fix and refactor markdown rendering (#32522) (cherry picked from commit 5eebe1dc5fb29a162c51d050396fce7b14e47f4e) Conflicts: models/repo/repo.go models/repo/repo_test.go modules/markup/html.go modules/markup/html_commit.go modules/markup/html_email.go modules/markup/html_emoji.go modules/markup/html_internal_test.go modules/markup/html_issue.go modules/markup/html_link.go modules/markup/html_node.go modules/markup/html_test.go modules/markup/markdown/goldmark.go modules/markup/markdown/markdown_test.go modules/markup/markdown/transform_image.go modules/markup/orgmode/orgmode.go modules/markup/orgmode/orgmode_test.go modules/markup/render.go modules/markup/render_links.go modules/templates/util_render.go modules/templates/util_render_test.go routers/common/markup.go routers/web/feed/convert.go routers/web/repo/wiki.go but a few lines survived and are useful --- modules/markup/asciicast/asciicast.go | 2 +- modules/markup/csv/csv.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/markup/asciicast/asciicast.go b/modules/markup/asciicast/asciicast.go index 0678062340..873029c1bd 100644 --- a/modules/markup/asciicast/asciicast.go +++ b/modules/markup/asciicast/asciicast.go @@ -39,7 +39,7 @@ const ( // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{ - {Element: "div", AllowAttr: "class", Regexp: regexp.MustCompile(playerClassName)}, + {Element: "div", AllowAttr: "class", Regexp: regexp.MustCompile("^" + playerClassName + "$")}, {Element: "div", AllowAttr: playerSrcAttr}, } } diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 3d952b0de4..092eec7098 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -37,9 +37,9 @@ func (Renderer) Extensions() []string { // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{ - {Element: "table", AllowAttr: "class", Regexp: regexp.MustCompile(`data-table`)}, - {Element: "th", AllowAttr: "class", Regexp: regexp.MustCompile(`line-num`)}, - {Element: "td", AllowAttr: "class", Regexp: regexp.MustCompile(`line-num`)}, + {Element: "table", AllowAttr: "class", Regexp: regexp.MustCompile(`^data-table$`)}, + {Element: "th", AllowAttr: "class", Regexp: regexp.MustCompile(`^line-num$`)}, + {Element: "td", AllowAttr: "class", Regexp: regexp.MustCompile(`^line-num$`)}, } } From 7751bb64cb20ca65c62466771f393f623eb37a0d Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 11 Nov 2024 01:38:30 +0100 Subject: [PATCH 09/11] Calculate `PublicOnly` for org membership only once (#32234) Refactoring of #32211 this move the PublicOnly() filter calcuation next to the DB querys and let it be decided by the Doer --- *Sponsored by Kithara Software GmbH* (cherry picked from commit 43c252dfeaf9ab03c4db3e7ac5169bc0d69901ac) Conflicts: models/organization/org_test.go models/organization/org_user_test.go routers/web/org/home.go rather simple conflict resolution but not trivial tests/integration/user_count_test.go had to be adapted (simple) because it does not exist in Gitea and uses the modified model --- models/organization/org.go | 18 ++++++--- models/organization/org_test.go | 60 +++++++++++++++------------- models/organization/org_user_test.go | 4 +- routers/api/v1/org/member.go | 22 +++++----- routers/web/org/home.go | 8 ++-- routers/web/org/members.go | 8 ++-- services/context/org.go | 7 ++-- tests/integration/user_count_test.go | 5 ++- 8 files changed, 76 insertions(+), 56 deletions(-) diff --git a/models/organization/org.go b/models/organization/org.go index 45f19c7696..e686b6f304 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -141,8 +141,9 @@ func (org *Organization) LoadTeams(ctx context.Context) ([]*Team, error) { } // GetMembers returns all members of organization. -func (org *Organization) GetMembers(ctx context.Context) (user_model.UserList, map[int64]bool, error) { +func (org *Organization) GetMembers(ctx context.Context, doer *user_model.User) (user_model.UserList, map[int64]bool, error) { return FindOrgMembers(ctx, &FindOrgMembersOpts{ + Doer: doer, OrgID: org.ID, }) } @@ -195,16 +196,22 @@ func (org *Organization) CanCreateRepo() bool { // FindOrgMembersOpts represensts find org members conditions type FindOrgMembersOpts struct { db.ListOptions - OrgID int64 - PublicOnly bool + Doer *user_model.User + IsDoerMember bool + OrgID int64 +} + +func (opts FindOrgMembersOpts) PublicOnly() bool { + return opts.Doer == nil || !(opts.IsDoerMember || opts.Doer.IsAdmin) } // CountOrgMembers counts the organization's members func CountOrgMembers(ctx context.Context, opts *FindOrgMembersOpts) (int64, error) { sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID) - if opts.PublicOnly { + if opts.PublicOnly() { sess.And("is_public = ?", true) } + return sess.Count(new(OrgUser)) } @@ -524,9 +531,10 @@ func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organiz // GetOrgUsersByOrgID returns all organization-user relations by organization ID. func GetOrgUsersByOrgID(ctx context.Context, opts *FindOrgMembersOpts) ([]*OrgUser, error) { sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID) - if opts.PublicOnly { + if opts.PublicOnly() { sess.And("is_public = ?", true) } + if opts.ListOptions.PageSize > 0 { sess = db.SetSessionPagination(sess, opts) diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 39ccc2d466..a81da28808 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -4,6 +4,7 @@ package organization_test import ( + "sort" "testing" "code.gitea.io/gitea/models/db" @@ -104,7 +105,7 @@ func TestUser_GetTeams(t *testing.T) { func TestUser_GetMembers(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) - members, _, err := org.GetMembers(db.DefaultContext) + members, _, err := org.GetMembers(db.DefaultContext, &user_model.User{IsAdmin: true}) require.NoError(t, err) if assert.Len(t, members, 3) { assert.Equal(t, int64(2), members[0].ID) @@ -211,37 +212,42 @@ func TestFindOrgs(t *testing.T) { func TestGetOrgUsersByOrgID(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) - orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ - ListOptions: db.ListOptions{}, - OrgID: 3, - PublicOnly: false, - }) - require.NoError(t, err) - if assert.Len(t, orgUsers, 3) { - assert.Equal(t, organization.OrgUser{ - ID: orgUsers[0].ID, - OrgID: 3, - UID: 2, - IsPublic: true, - }, *orgUsers[0]) - assert.Equal(t, organization.OrgUser{ - ID: orgUsers[1].ID, - OrgID: 3, - UID: 4, - IsPublic: false, - }, *orgUsers[1]) - assert.Equal(t, organization.OrgUser{ - ID: orgUsers[2].ID, - OrgID: 3, - UID: 28, - IsPublic: true, - }, *orgUsers[2]) + opts := &organization.FindOrgMembersOpts{ + Doer: &user_model.User{IsAdmin: true}, + OrgID: 3, } + assert.False(t, opts.PublicOnly()) + orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, opts) + require.NoError(t, err) + sort.Slice(orgUsers, func(i, j int) bool { + return orgUsers[i].ID < orgUsers[j].ID + }) + assert.EqualValues(t, []*organization.OrgUser{{ + ID: 1, + OrgID: 3, + UID: 2, + IsPublic: true, + }, { + ID: 2, + OrgID: 3, + UID: 4, + IsPublic: false, + }, { + ID: 9, + OrgID: 3, + UID: 28, + IsPublic: true, + }}, orgUsers) + + opts = &organization.FindOrgMembersOpts{OrgID: 3} + assert.True(t, opts.PublicOnly()) + orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, opts) + require.NoError(t, err) + assert.Len(t, orgUsers, 2) orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{ ListOptions: db.ListOptions{}, OrgID: unittest.NonexistentID, - PublicOnly: false, }) require.NoError(t, err) assert.Empty(t, orgUsers) diff --git a/models/organization/org_user_test.go b/models/organization/org_user_test.go index 07d07ce3b8..336bf1a5a4 100644 --- a/models/organization/org_user_test.go +++ b/models/organization/org_user_test.go @@ -95,7 +95,7 @@ func TestUserListIsPublicMember(t *testing.T) { func testUserListIsPublicMember(t *testing.T, orgID int64, expected map[int64]bool) { org, err := organization.GetOrgByID(db.DefaultContext, orgID) require.NoError(t, err) - _, membersIsPublic, err := org.GetMembers(db.DefaultContext) + _, membersIsPublic, err := org.GetMembers(db.DefaultContext, &user_model.User{IsAdmin: true}) require.NoError(t, err) assert.Equal(t, expected, membersIsPublic) } @@ -122,7 +122,7 @@ func TestUserListIsUserOrgOwner(t *testing.T) { func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected map[int64]bool) { org, err := organization.GetOrgByID(db.DefaultContext, orgID) require.NoError(t, err) - members, _, err := org.GetMembers(db.DefaultContext) + members, _, err := org.GetMembers(db.DefaultContext, &user_model.User{IsAdmin: true}) require.NoError(t, err) assert.Equal(t, expected, organization.IsUserOrgOwner(db.DefaultContext, members, orgID)) } diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go index fb66d4c3f5..0895c53328 100644 --- a/routers/api/v1/org/member.go +++ b/routers/api/v1/org/member.go @@ -18,11 +18,12 @@ import ( ) // listMembers list an organization's members -func listMembers(ctx *context.APIContext, publicOnly bool) { +func listMembers(ctx *context.APIContext, isMember bool) { opts := &organization.FindOrgMembersOpts{ - OrgID: ctx.Org.Organization.ID, - PublicOnly: publicOnly, - ListOptions: utils.GetListOptions(ctx), + Doer: ctx.Doer, + IsDoerMember: isMember, + OrgID: ctx.Org.Organization.ID, + ListOptions: utils.GetListOptions(ctx), } count, err := organization.CountOrgMembers(ctx, opts) @@ -73,16 +74,19 @@ func ListMembers(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - publicOnly := true + var ( + isMember bool + err error + ) + if ctx.Doer != nil { - isMember, err := ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID) + isMember, err = ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID) if err != nil { ctx.Error(http.StatusInternalServerError, "IsOrgMember", err) return } - publicOnly = !isMember && !ctx.Doer.IsAdmin } - listMembers(ctx, publicOnly) + listMembers(ctx, isMember) } // ListPublicMembers list an organization's public members @@ -112,7 +116,7 @@ func ListPublicMembers(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - listMembers(ctx, true) + listMembers(ctx, false) } // IsMember check if a user is a member of an organization diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 92793d95a4..9ebefa334c 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -110,10 +110,12 @@ func Home(ctx *context.Context) { } opts := &organization.FindOrgMembersOpts{ - OrgID: org.ID, - PublicOnly: ctx.Org.PublicMemberOnly, - ListOptions: db.ListOptions{Page: 1, PageSize: 25}, + Doer: ctx.Doer, + OrgID: org.ID, + IsDoerMember: ctx.Org.IsMember, + ListOptions: db.ListOptions{Page: 1, PageSize: 25}, } + members, _, err := organization.FindOrgMembers(ctx, opts) if err != nil { ctx.ServerError("FindOrgMembers", err) diff --git a/routers/web/org/members.go b/routers/web/org/members.go index 9a3d60e122..3a5509f911 100644 --- a/routers/web/org/members.go +++ b/routers/web/org/members.go @@ -33,8 +33,8 @@ func Members(ctx *context.Context) { } opts := &organization.FindOrgMembersOpts{ - OrgID: org.ID, - PublicOnly: true, + Doer: ctx.Doer, + OrgID: org.ID, } if ctx.Doer != nil { @@ -43,9 +43,9 @@ func Members(ctx *context.Context) { ctx.Error(http.StatusInternalServerError, "IsOrgMember") return } - opts.PublicOnly = !isMember && !ctx.Doer.IsAdmin + opts.IsDoerMember = isMember } - ctx.Data["PublicOnly"] = opts.PublicOnly + ctx.Data["PublicOnly"] = opts.PublicOnly() total, err := organization.CountOrgMembers(ctx, opts) if err != nil { diff --git a/services/context/org.go b/services/context/org.go index 018b76de43..9673f2f5a9 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -26,7 +26,6 @@ type Organization struct { Organization *organization.Organization OrgLink string CanCreateOrgRepo bool - PublicMemberOnly bool // Only display public members Team *organization.Team Teams []*organization.Team @@ -176,10 +175,10 @@ func HandleOrgAssignment(ctx *Context, args ...bool) { ctx.Data["OrgLink"] = ctx.Org.OrgLink // Member - ctx.Org.PublicMemberOnly = ctx.Doer == nil || !ctx.Org.IsMember && !ctx.Doer.IsAdmin opts := &organization.FindOrgMembersOpts{ - OrgID: org.ID, - PublicOnly: ctx.Org.PublicMemberOnly, + Doer: ctx.Doer, + OrgID: org.ID, + IsDoerMember: ctx.Org.IsMember, } ctx.Data["NumMembers"], err = organization.CountOrgMembers(ctx, opts) if err != nil { diff --git a/tests/integration/user_count_test.go b/tests/integration/user_count_test.go index db3a9b11a7..8fb78a61ee 100644 --- a/tests/integration/user_count_test.go +++ b/tests/integration/user_count_test.go @@ -75,8 +75,9 @@ func (countTest *userCountTest) Init(t *testing.T, doerID, userID int64) { require.NoError(t, err) countTest.memberCount, err = organization.CountOrgMembers(db.DefaultContext, &organization.FindOrgMembersOpts{ - OrgID: org.ID, - PublicOnly: !isMember, + Doer: countTest.doer, + OrgID: org.ID, + IsDoerMember: isMember, }) require.NoError(t, err) From f6a46055aac2917af7f0a46dfd106c03bea733bc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 13 Nov 2024 21:31:47 -0800 Subject: [PATCH 10/11] Reimplement GetUserOrgsList to make it simple and clear (#32486) Reimplement GetUserOrgsList and also move some functions and test to org_list file. --------- Co-authored-by: Zettat123 (cherry picked from commit b4abb6deff14b741c7666d7579e0eea68443306c) Conflicts: models/organization/org_test.go services/oauth2_provider/access_token.go trivial conflicts due to codeblocks moving to different files --- models/organization/mini_org.go | 78 --------------- models/organization/org.go | 57 ----------- models/organization/org_list.go | 138 +++++++++++++++++++++++++++ models/organization/org_list_test.go | 63 ++++++++++++ models/organization/org_test.go | 36 ------- 5 files changed, 201 insertions(+), 171 deletions(-) delete mode 100644 models/organization/mini_org.go create mode 100644 models/organization/org_list.go create mode 100644 models/organization/org_list_test.go diff --git a/models/organization/mini_org.go b/models/organization/mini_org.go deleted file mode 100644 index b1b24624c5..0000000000 --- a/models/organization/mini_org.go +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package organization - -import ( - "context" - "fmt" - "strings" - - "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" - user_model "code.gitea.io/gitea/models/user" - - "xorm.io/builder" -) - -// MinimalOrg represents a simple organization with only the needed columns -type MinimalOrg = Organization - -// GetUserOrgsList returns all organizations the given user has access to -func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, error) { - schema, err := db.TableInfo(new(user_model.User)) - if err != nil { - return nil, err - } - - outputCols := []string{ - "id", - "name", - "full_name", - "visibility", - "avatar", - "avatar_email", - "use_custom_avatar", - } - - groupByCols := &strings.Builder{} - for _, col := range outputCols { - fmt.Fprintf(groupByCols, "`%s`.%s,", schema.Name, col) - } - groupByStr := groupByCols.String() - groupByStr = groupByStr[0 : len(groupByStr)-1] - - sess := db.GetEngine(ctx) - sess = sess.Select(groupByStr+", count(distinct repo_id) as org_count"). - Table("user"). - Join("INNER", "team", "`team`.org_id = `user`.id"). - Join("INNER", "team_user", "`team`.id = `team_user`.team_id"). - Join("LEFT", builder. - Select("id as repo_id, owner_id as repo_owner_id"). - From("repository"). - Where(repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid)), "`repository`.repo_owner_id = `team`.org_id"). - Where("`team_user`.uid = ?", user.ID). - GroupBy(groupByStr) - - type OrgCount struct { - Organization `xorm:"extends"` - OrgCount int - } - - orgCounts := make([]*OrgCount, 0, 10) - - if err := sess. - Asc("`user`.name"). - Find(&orgCounts); err != nil { - return nil, err - } - - orgs := make([]*MinimalOrg, len(orgCounts)) - for i, orgCount := range orgCounts { - orgCount.Organization.NumRepos = orgCount.OrgCount - orgs[i] = &orgCount.Organization - } - - return orgs, nil -} diff --git a/models/organization/org.go b/models/organization/org.go index e686b6f304..0dc42e1c4f 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -24,13 +24,6 @@ import ( "xorm.io/builder" ) -// ________ .__ __ .__ -// \_____ \_______ _________ ____ |__|____________ _/ |_|__| ____ ____ -// / | \_ __ \/ ___\__ \ / \| \___ /\__ \\ __\ |/ _ \ / \ -// / | \ | \/ /_/ > __ \| | \ |/ / / __ \| | | ( <_> ) | \ -// \_______ /__| \___ (____ /___| /__/_____ \(____ /__| |__|\____/|___| / -// \/ /_____/ \/ \/ \/ \/ \/ - // ErrOrgNotExist represents a "OrgNotExist" kind of error. type ErrOrgNotExist struct { ID int64 @@ -446,42 +439,6 @@ func GetUsersWhoCanCreateOrgRepo(ctx context.Context, orgID int64) (map[int64]*u And("team_user.org_id = ?", orgID).Find(&users) } -// SearchOrganizationsOptions options to filter organizations -type SearchOrganizationsOptions struct { - db.ListOptions - All bool -} - -// FindOrgOptions finds orgs options -type FindOrgOptions struct { - db.ListOptions - UserID int64 - IncludePrivate bool -} - -func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder { - cond := builder.Eq{"uid": userID} - if !includePrivate { - cond["is_public"] = true - } - return builder.Select("org_id").From("org_user").Where(cond) -} - -func (opts FindOrgOptions) ToConds() builder.Cond { - var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization} - if opts.UserID > 0 { - cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludePrivate))) - } - if !opts.IncludePrivate { - cond = cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic}) - } - return cond -} - -func (opts FindOrgOptions) ToOrders() string { - return "`user`.name ASC" -} - // HasOrgOrUserVisible tells if the given user can see the given org or user func HasOrgOrUserVisible(ctx context.Context, orgOrUser, user *user_model.User) bool { // If user is nil, it's an anonymous user/request. @@ -514,20 +471,6 @@ func HasOrgsVisible(ctx context.Context, orgs []*Organization, user *user_model. return false } -// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID -// are allowed to create repos. -func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organization, error) { - orgs := make([]*Organization, 0, 10) - - return orgs, db.GetEngine(ctx).Where(builder.In("id", builder.Select("`user`.id").From("`user`"). - Join("INNER", "`team_user`", "`team_user`.org_id = `user`.id"). - Join("INNER", "`team`", "`team`.id = `team_user`.team_id"). - Where(builder.Eq{"`team_user`.uid": userID}). - And(builder.Eq{"`team`.authorize": perm.AccessModeOwner}.Or(builder.Eq{"`team`.can_create_org_repo": true})))). - Asc("`user`.name"). - Find(&orgs) -} - // GetOrgUsersByOrgID returns all organization-user relations by organization ID. func GetOrgUsersByOrgID(ctx context.Context, opts *FindOrgMembersOpts) ([]*OrgUser, error) { sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID) diff --git a/models/organization/org_list.go b/models/organization/org_list.go new file mode 100644 index 0000000000..72ebf6f178 --- /dev/null +++ b/models/organization/org_list.go @@ -0,0 +1,138 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package organization + +import ( + "context" + "fmt" + "strings" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/structs" + + "xorm.io/builder" +) + +// SearchOrganizationsOptions options to filter organizations +type SearchOrganizationsOptions struct { + db.ListOptions + All bool +} + +// FindOrgOptions finds orgs options +type FindOrgOptions struct { + db.ListOptions + UserID int64 + IncludePrivate bool +} + +func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder { + cond := builder.Eq{"uid": userID} + if !includePrivate { + cond["is_public"] = true + } + return builder.Select("org_id").From("org_user").Where(cond) +} + +func (opts FindOrgOptions) ToConds() builder.Cond { + var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization} + if opts.UserID > 0 { + cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludePrivate))) + } + if !opts.IncludePrivate { + cond = cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic}) + } + return cond +} + +func (opts FindOrgOptions) ToOrders() string { + return "`user`.lower_name ASC" +} + +// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID +// are allowed to create repos. +func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organization, error) { + orgs := make([]*Organization, 0, 10) + + return orgs, db.GetEngine(ctx).Where(builder.In("id", builder.Select("`user`.id").From("`user`"). + Join("INNER", "`team_user`", "`team_user`.org_id = `user`.id"). + Join("INNER", "`team`", "`team`.id = `team_user`.team_id"). + Where(builder.Eq{"`team_user`.uid": userID}). + And(builder.Eq{"`team`.authorize": perm.AccessModeOwner}.Or(builder.Eq{"`team`.can_create_org_repo": true})))). + Asc("`user`.name"). + Find(&orgs) +} + +// MinimalOrg represents a simple organization with only the needed columns +type MinimalOrg = Organization + +// GetUserOrgsList returns all organizations the given user has access to +func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, error) { + schema, err := db.TableInfo(new(user_model.User)) + if err != nil { + return nil, err + } + + outputCols := []string{ + "id", + "name", + "full_name", + "visibility", + "avatar", + "avatar_email", + "use_custom_avatar", + } + + selectColumns := &strings.Builder{} + for i, col := range outputCols { + fmt.Fprintf(selectColumns, "`%s`.%s", schema.Name, col) + if i < len(outputCols)-1 { + selectColumns.WriteString(", ") + } + } + columnsStr := selectColumns.String() + + var orgs []*MinimalOrg + if err := db.GetEngine(ctx).Select(columnsStr). + Table("user"). + Where(builder.In("`user`.`id`", queryUserOrgIDs(user.ID, true))). + Find(&orgs); err != nil { + return nil, err + } + + type orgCount struct { + OrgID int64 + RepoCount int + } + var orgCounts []orgCount + if err := db.GetEngine(ctx). + Select("owner_id AS org_id, COUNT(DISTINCT(repository.id)) as repo_count"). + Table("repository"). + Join("INNER", "org_user", "owner_id = org_user.org_id"). + Where("org_user.uid = ?", user.ID). + And(builder.Or( + builder.Eq{"repository.is_private": false}, + builder.In("repository.id", builder.Select("repo_id").From("team_repo"). + InnerJoin("team_user", "team_user.team_id = team_repo.team_id"). + Where(builder.Eq{"team_user.uid": user.ID})), + builder.In("repository.id", builder.Select("repo_id").From("collaboration"). + Where(builder.Eq{"user_id": user.ID})), + )). + GroupBy("owner_id").Find(&orgCounts); err != nil { + return nil, err + } + + orgCountMap := make(map[int64]int, len(orgCounts)) + for _, orgCount := range orgCounts { + orgCountMap[orgCount.OrgID] = orgCount.RepoCount + } + + for _, org := range orgs { + org.NumRepos = orgCountMap[org.ID] + } + + return orgs, nil +} diff --git a/models/organization/org_list_test.go b/models/organization/org_list_test.go new file mode 100644 index 0000000000..7d02c6c73f --- /dev/null +++ b/models/organization/org_list_test.go @@ -0,0 +1,63 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package organization_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCountOrganizations(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + expected, err := db.GetEngine(db.DefaultContext).Where("type=?", user_model.UserTypeOrganization).Count(&organization.Organization{}) + require.NoError(t, err) + cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludePrivate: true}) + require.NoError(t, err) + assert.Equal(t, expected, cnt) +} + +func TestFindOrgs(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ + UserID: 4, + IncludePrivate: true, + }) + require.NoError(t, err) + if assert.Len(t, orgs, 1) { + assert.EqualValues(t, 3, orgs[0].ID) + } + + orgs, err = db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ + UserID: 4, + IncludePrivate: false, + }) + require.NoError(t, err) + assert.Empty(t, orgs) + + total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ + UserID: 4, + IncludePrivate: true, + }) + require.NoError(t, err) + assert.EqualValues(t, 1, total) +} + +func TestGetUserOrgsList(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + orgs, err := organization.GetUserOrgsList(db.DefaultContext, &user_model.User{ID: 4}) + require.NoError(t, err) + if assert.Len(t, orgs, 1) { + assert.EqualValues(t, 3, orgs[0].ID) + // repo_id: 3 is in the team, 32 is public, 5 is private with no team + assert.EqualValues(t, 2, orgs[0].NumRepos) + } +} diff --git a/models/organization/org_test.go b/models/organization/org_test.go index a81da28808..681eeb8a34 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -129,15 +129,6 @@ func TestGetOrgByName(t *testing.T) { assert.True(t, organization.IsErrOrgNotExist(err)) } -func TestCountOrganizations(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - expected, err := db.GetEngine(db.DefaultContext).Where("type=?", user_model.UserTypeOrganization).Count(&organization.Organization{}) - require.NoError(t, err) - cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludePrivate: true}) - require.NoError(t, err) - assert.Equal(t, expected, cnt) -} - func TestIsOrganizationOwner(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) test := func(orgID, userID int64, expected bool) { @@ -182,33 +173,6 @@ func TestIsPublicMembership(t *testing.T) { test(unittest.NonexistentID, unittest.NonexistentID, false) } -func TestFindOrgs(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ - UserID: 4, - IncludePrivate: true, - }) - require.NoError(t, err) - if assert.Len(t, orgs, 1) { - assert.EqualValues(t, 3, orgs[0].ID) - } - - orgs, err = db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ - UserID: 4, - IncludePrivate: false, - }) - require.NoError(t, err) - assert.Empty(t, orgs) - - total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ - UserID: 4, - IncludePrivate: true, - }) - require.NoError(t, err) - assert.EqualValues(t, 1, total) -} - func TestGetOrgUsersByOrgID(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) From cc6321baf7575fe31dbc089c8eedcf834e1b7897 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 17 Nov 2024 20:41:05 +0100 Subject: [PATCH 11/11] chore(release-notes): notes for the week 2024-47 weekly cherry pick --- release-notes/5997.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 release-notes/5997.md diff --git a/release-notes/5997.md b/release-notes/5997.md new file mode 100644 index 0000000000..9597da9e79 --- /dev/null +++ b/release-notes/5997.md @@ -0,0 +1,8 @@ +fix(security): [commit](https://codeberg.org/forgejo/forgejo/commit/45435a8789f8ff69603799a9031246d2d621d139) Fix and refactor markdown rendering +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/a8f2002a9b061ec1092df67c6f05e30aa7d2e2d2) Remove transaction for archive download +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/96ee0f56475204b2bbdc7f2aeb35b1c32eac469c) Fix oauth2 error handle not return immediately +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/c2e8790df37a14b4d2f72c7377db75309e0ebf1d) Trim title before insert/update to database to match the size requirements of database +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/03ab73d92eabaf774278effe3332623b1dc3580a) Fix nil panic if repo doesn't exist +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/56971f9ed90a01fd74a634b7496593e6f62ac260) Disable Oauth check if oauth disabled +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/7f51210672031aee7a790455d51a17ce11a70559) Harden runner updateTask and updateLog api +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/dd3c4d7096cff91854bcc6641f55d9d093e5c86e) Add a doctor check to disable the "Actions" unit for mirrors