From fb3ee17108ec0339115ea7da8fa4609c1f961e84 Mon Sep 17 00:00:00 2001 From: DrMaxNix Date: Mon, 7 Apr 2025 22:37:45 +0200 Subject: [PATCH 1/9] Check user/org repo limit instead of doer (30011) --- models/repo/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo/update.go b/models/repo/update.go index fce357a1acf35..01e62126786f2 100644 --- a/models/repo/update.go +++ b/models/repo/update.go @@ -113,7 +113,7 @@ func (err ErrRepoFilesAlreadyExist) Unwrap() error { // CheckCreateRepository check if could created a repository func CheckCreateRepository(ctx context.Context, doer, u *user_model.User, name string, overwriteOrAdopt bool) error { - if !doer.CanCreateRepo() { + if !u.CanCreateRepo() { return ErrReachLimitOfRepo{u.MaxRepoCreation} } From d072f83a5621552ca5b83e7e57c1c2b480662029 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 8 Apr 2025 09:41:09 +0800 Subject: [PATCH 2/9] Update models/repo/update.go Co-authored-by: TheFox0x7 --- models/repo/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo/update.go b/models/repo/update.go index 01e62126786f2..4a1068c6410eb 100644 --- a/models/repo/update.go +++ b/models/repo/update.go @@ -113,7 +113,7 @@ func (err ErrRepoFilesAlreadyExist) Unwrap() error { // CheckCreateRepository check if could created a repository func CheckCreateRepository(ctx context.Context, doer, u *user_model.User, name string, overwriteOrAdopt bool) error { - if !u.CanCreateRepo() { + if !doer.IsAdmin && !u.CanCreateRepo() { return ErrReachLimitOfRepo{u.MaxRepoCreation} } From 8b48d863364d41f74c51b6035adbdaec51252673 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 8 Apr 2025 10:01:32 +0800 Subject: [PATCH 3/9] fix --- models/organization/org.go | 6 ------ models/repo/update.go | 16 ++++++++-------- models/user/user.go | 20 +++++++++----------- routers/web/repo/fork.go | 8 ++++---- routers/web/repo/repo.go | 16 ++++++---------- routers/web/repo/setting/setting.go | 3 ++- services/repository/adopt.go | 18 +++++++++--------- services/repository/create.go | 24 ++++++++++++------------ services/repository/fork.go | 2 +- services/repository/template.go | 2 +- services/repository/transfer.go | 4 ++-- services/repository/transfer_test.go | 4 ++-- templates/repo/pulls/fork.tmpl | 2 +- templates/repo/settings/options.tmpl | 4 ++-- 14 files changed, 59 insertions(+), 70 deletions(-) diff --git a/models/organization/org.go b/models/organization/org.go index 3e55a36758963..dc889ea17fa8e 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -178,12 +178,6 @@ func (org *Organization) HomeLink() string { return org.AsUser().HomeLink() } -// CanCreateRepo returns if user login can create a repository -// NOTE: functions calling this assume a failure due to repository count limit; if new checks are added, those functions should be revised -func (org *Organization) CanCreateRepo() bool { - return org.AsUser().CanCreateRepo() -} - // FindOrgMembersOpts represensts find org members conditions type FindOrgMembersOpts struct { db.ListOptions diff --git a/models/repo/update.go b/models/repo/update.go index 4a1068c6410eb..15c8c48d5bbe3 100644 --- a/models/repo/update.go +++ b/models/repo/update.go @@ -111,31 +111,31 @@ func (err ErrRepoFilesAlreadyExist) Unwrap() error { return util.ErrAlreadyExist } -// CheckCreateRepository check if could created a repository -func CheckCreateRepository(ctx context.Context, doer, u *user_model.User, name string, overwriteOrAdopt bool) error { - if !doer.IsAdmin && !u.CanCreateRepo() { - return ErrReachLimitOfRepo{u.MaxRepoCreation} +// CheckCreateRepository check if doer could create a repository in new owner +func CheckCreateRepository(ctx context.Context, doer, owner *user_model.User, name string, overwriteOrAdopt bool) error { + if !doer.CanCreateRepoIn(owner) { + return ErrReachLimitOfRepo{owner.MaxRepoCreation} } if err := IsUsableRepoName(name); err != nil { return err } - has, err := IsRepositoryModelOrDirExist(ctx, u, name) + has, err := IsRepositoryModelOrDirExist(ctx, owner, name) if err != nil { return fmt.Errorf("IsRepositoryExist: %w", err) } else if has { - return ErrRepoAlreadyExist{u.Name, name} + return ErrRepoAlreadyExist{owner.Name, name} } - repoPath := RepoPath(u.Name, name) + repoPath := RepoPath(owner.Name, name) isExist, err := util.IsExist(repoPath) if err != nil { log.Error("Unable to check if %s exists. Error: %v", repoPath, err) return err } if !overwriteOrAdopt && isExist { - return ErrRepoFilesAlreadyExist{u.Name, name} + return ErrRepoFilesAlreadyExist{owner.Name, name} } return nil } diff --git a/models/user/user.go b/models/user/user.go index 3b268a6f41e52..baaa806ba0bd4 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -247,19 +247,19 @@ func (u *User) MaxCreationLimit() int { return u.MaxRepoCreation } -// CanCreateRepo returns if user login can create a repository -// NOTE: functions calling this assume a failure due to repository count limit; if new checks are added, those functions should be revised -func (u *User) CanCreateRepo() bool { +// CanCreateRepoIn checks whether the doer(u) can create a repository in the owner +func (u *User) CanCreateRepoIn(owner *User) bool { if u.IsAdmin { return true } - if u.MaxRepoCreation <= -1 { - if setting.Repository.MaxCreationLimit <= -1 { + const noLimit = -1 + if owner.MaxRepoCreation == noLimit { + if setting.Repository.MaxCreationLimit == noLimit { return true } - return u.NumRepos < setting.Repository.MaxCreationLimit + return owner.NumRepos < setting.Repository.MaxCreationLimit } - return u.NumRepos < u.MaxRepoCreation + return owner.NumRepos < owner.MaxRepoCreation } // CanCreateOrganization returns true if user can create organisation. @@ -272,13 +272,11 @@ func (u *User) CanEditGitHook() bool { return !setting.DisableGitHooks && (u.IsAdmin || u.AllowGitHook) } -// CanForkRepo returns if user login can fork a repository -// It checks especially that the user can create repos, and potentially more -func (u *User) CanForkRepo() bool { +func (u *User) CanForkRepoIn(owner *User) bool { if setting.Repository.AllowForkWithoutMaximumLimit { return true } - return u.CanCreateRepo() + return u.CanCreateRepoIn(owner) } // CanImportLocal returns true if user can migrate repository by local path. diff --git a/routers/web/repo/fork.go b/routers/web/repo/fork.go index 36e64bfee31ae..0700172b5d412 100644 --- a/routers/web/repo/fork.go +++ b/routers/web/repo/fork.go @@ -96,7 +96,7 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { } else if len(orgs) > 0 { ctx.Data["ContextUser"] = orgs[0] } else { - ctx.Data["CanForkRepo"] = false + ctx.Data["CanForkRepoInDoer"] = false ctx.Flash.Error(ctx.Tr("repo.fork_no_valid_owners"), true) return nil } @@ -121,8 +121,8 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { func Fork(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("new_fork") - if ctx.Doer.CanForkRepo() { - ctx.Data["CanForkRepo"] = true + if ctx.Doer.CanForkRepoIn(ctx.Doer) { + ctx.Data["CanForkRepoInDoer"] = true } else { maxCreationLimit := ctx.Doer.MaxCreationLimit() msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) @@ -141,7 +141,7 @@ func Fork(ctx *context.Context) { func ForkPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CreateRepoForm) ctx.Data["Title"] = ctx.Tr("new_fork") - ctx.Data["CanForkRepo"] = true + ctx.Data["CanForkRepoInDoer"] = true ctxUser := checkContextUser(ctx, form.UID) if ctx.Written() { diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index e260ea36ddd76..ee112b83f261b 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -87,17 +87,13 @@ func checkContextUser(ctx *context.Context, uid int64) *user_model.User { return nil } - if !ctx.Doer.IsAdmin { - orgsAvailable := []*organization.Organization{} - for i := 0; i < len(orgs); i++ { - if orgs[i].CanCreateRepo() { - orgsAvailable = append(orgsAvailable, orgs[i]) - } + var orgsAvailable []*organization.Organization + for i := 0; i < len(orgs); i++ { + if ctx.Doer.CanCreateRepoIn(orgs[i].AsUser()) { + orgsAvailable = append(orgsAvailable, orgs[i]) } - ctx.Data["Orgs"] = orgsAvailable - } else { - ctx.Data["Orgs"] = orgs } + ctx.Data["Orgs"] = orgsAvailable // Not equal means current user is an organization. if uid == ctx.Doer.ID || uid == 0 { @@ -154,7 +150,7 @@ func createCommon(ctx *context.Context) { ctx.Data["Licenses"] = repo_module.Licenses ctx.Data["Readmes"] = repo_module.Readmes ctx.Data["IsForcedPrivate"] = setting.Repository.ForcePrivate - ctx.Data["CanCreateRepoInDoer"] = ctx.Doer.CanCreateRepo() + ctx.Data["CanCreateRepoInDoer"] = ctx.Doer.CanCreateRepoIn(ctx.Doer) ctx.Data["MaxCreationLimitOfDoer"] = ctx.Doer.MaxCreationLimit() ctx.Data["SupportedObjectFormats"] = git.DefaultFeatures().SupportedObjectFormats ctx.Data["DefaultObjectFormat"] = git.Sha1ObjectFormat diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 380fec9d4a7d6..5552a8726cc89 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -59,6 +59,7 @@ func SettingsCtxData(ctx *context.Context) { ctx.Data["DisableNewPushMirrors"] = setting.Mirror.DisableNewPush ctx.Data["DefaultMirrorInterval"] = setting.Mirror.DefaultInterval ctx.Data["MinimumMirrorInterval"] = setting.Mirror.MinInterval + ctx.Data["CanConvertFork"] = ctx.Repo.Repository.IsFork && ctx.Doer.CanCreateRepoIn(ctx.Repo.Repository.Owner) signing, _ := asymkey_service.SigningKey(ctx, ctx.Repo.Repository.RepoPath()) ctx.Data["SigningKeyAvailable"] = len(signing) > 0 @@ -786,7 +787,7 @@ func handleSettingsPostConvertFork(ctx *context.Context) { return } - if !ctx.Repo.Owner.CanCreateRepo() { + if !ctx.Doer.CanForkRepoIn(ctx.Repo.Owner) { maxCreationLimit := ctx.Repo.Owner.MaxCreationLimit() msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) ctx.Flash.Error(msg) diff --git a/services/repository/adopt.go b/services/repository/adopt.go index b7321156d9da8..a8e609788ec75 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -29,17 +29,17 @@ import ( ) // AdoptRepository adopts pre-existing repository files for the user/organization. -func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { - if !doer.IsAdmin && !u.CanCreateRepo() { +func AdoptRepository(ctx context.Context, doer, owner *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { + if !doer.CanCreateRepoIn(owner) { return nil, repo_model.ErrReachLimitOfRepo{ - Limit: u.MaxRepoCreation, + Limit: owner.MaxRepoCreation, } } repo := &repo_model.Repository{ - OwnerID: u.ID, - Owner: u, - OwnerName: u.Name, + OwnerID: owner.ID, + Owner: owner, + OwnerName: owner.Name, Name: opts.Name, LowerName: strings.ToLower(opts.Name), Description: opts.Description, @@ -60,12 +60,12 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR } if !isExist { return repo_model.ErrRepoNotExist{ - OwnerName: u.Name, + OwnerName: owner.Name, Name: repo.Name, } } - if err := CreateRepositoryByExample(ctx, doer, u, repo, true, false); err != nil { + if err := CreateRepositoryByExample(ctx, doer, owner, repo, true, false); err != nil { return err } @@ -100,7 +100,7 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR } return nil, err } - notify_service.AdoptRepository(ctx, doer, u, repo) + notify_service.AdoptRepository(ctx, doer, owner, repo) return repo, nil } diff --git a/services/repository/create.go b/services/repository/create.go index af4e897151454..d564985c9d862 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -200,10 +200,10 @@ func initRepository(ctx context.Context, u *user_model.User, repo *repo_model.Re } // CreateRepositoryDirectly creates a repository for the user/organization. -func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { - if !doer.IsAdmin && !u.CanCreateRepo() { +func CreateRepositoryDirectly(ctx context.Context, doer, owner *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { + if !doer.CanCreateRepoIn(owner) { return nil, repo_model.ErrReachLimitOfRepo{ - Limit: u.MaxRepoCreation, + Limit: owner.MaxRepoCreation, } } @@ -223,9 +223,9 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt } repo := &repo_model.Repository{ - OwnerID: u.ID, - Owner: u, - OwnerName: u.Name, + OwnerID: owner.ID, + Owner: owner, + OwnerName: owner.Name, Name: opts.Name, LowerName: strings.ToLower(opts.Name), Description: opts.Description, @@ -247,7 +247,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt var rollbackRepo *repo_model.Repository if err := db.WithTx(ctx, func(ctx context.Context) error { - if err := CreateRepositoryByExample(ctx, doer, u, repo, false, false); err != nil { + if err := CreateRepositoryByExample(ctx, doer, owner, repo, false, false); err != nil { return err } @@ -271,7 +271,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt // So we will now fail and delegate to other functionality to adopt or delete log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName()) return repo_model.ErrRepoFilesAlreadyExist{ - Uname: u.Name, + Uname: owner.Name, Name: repo.Name, } } @@ -280,7 +280,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt if err2 := gitrepo.DeleteRepository(ctx, repo); err2 != nil { log.Error("initRepository: %v", err) return fmt.Errorf( - "delete repo directory %s/%s failed(2): %v", u.Name, repo.Name, err2) + "delete repo directory %s/%s failed(2): %v", owner.Name, repo.Name, err2) } return fmt.Errorf("initRepository: %w", err) } @@ -289,7 +289,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt if len(opts.IssueLabels) > 0 { if err = repo_module.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { rollbackRepo = repo - rollbackRepo.OwnerID = u.ID + rollbackRepo.OwnerID = owner.ID return fmt.Errorf("InitializeLabels: %w", err) } } @@ -302,7 +302,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil { log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) rollbackRepo = repo - rollbackRepo.OwnerID = u.ID + rollbackRepo.OwnerID = owner.ID return fmt.Errorf("CreateRepository(git update-server-info): %w", err) } @@ -315,7 +315,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt if err != nil { log.Error("CreateRepository(git rev-parse HEAD) in %v: Stdout: %s\nError: %v", repo, stdout, err) rollbackRepo = repo - rollbackRepo.OwnerID = u.ID + rollbackRepo.OwnerID = owner.ID return fmt.Errorf("CreateRepository(git rev-parse HEAD): %w", err) } if err := repo_model.UpdateRepoLicenses(ctx, repo, stdout, licenses); err != nil { diff --git a/services/repository/fork.go b/services/repository/fork.go index 5b1ba7a418232..52e2f9e25b562 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -65,7 +65,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork } // Fork is prohibited, if user has reached maximum limit of repositories - if !owner.CanForkRepo() { + if !doer.CanForkRepoIn(owner) { return nil, repo_model.ErrReachLimitOfRepo{ Limit: owner.MaxRepoCreation, } diff --git a/services/repository/template.go b/services/repository/template.go index 36a680c8e2009..840538b4dd158 100644 --- a/services/repository/template.go +++ b/services/repository/template.go @@ -63,7 +63,7 @@ func GenerateProtectedBranch(ctx context.Context, templateRepo, generateRepo *re // GenerateRepository generates a repository from a template func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templateRepo *repo_model.Repository, opts GenerateRepoOptions) (_ *repo_model.Repository, err error) { - if !doer.IsAdmin && !owner.CanCreateRepo() { + if !doer.CanCreateRepoIn(owner) { return nil, repo_model.ErrReachLimitOfRepo{ Limit: owner.MaxRepoCreation, } diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 4e44b90df205c..86917ad2851bb 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -61,7 +61,7 @@ func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, d return err } - if !doer.IsAdmin && !repoTransfer.Recipient.CanCreateRepo() { + if !doer.CanCreateRepoIn(repoTransfer.Recipient) { limit := util.Iif(repoTransfer.Recipient.MaxRepoCreation >= 0, repoTransfer.Recipient.MaxRepoCreation, setting.Repository.MaxCreationLimit) return LimitReachedError{Limit: limit} } @@ -416,7 +416,7 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use return err } - if !doer.IsAdmin && !newOwner.CanCreateRepo() { + if !doer.CanForkRepoIn(newOwner) { limit := util.Iif(newOwner.MaxRepoCreation >= 0, newOwner.MaxRepoCreation, setting.Repository.MaxCreationLimit) return LimitReachedError{Limit: limit} } diff --git a/services/repository/transfer_test.go b/services/repository/transfer_test.go index bf71c7ca2e30e..80a073e9f912d 100644 --- a/services/repository/transfer_test.go +++ b/services/repository/transfer_test.go @@ -144,7 +144,7 @@ func TestRepositoryTransferRejection(t *testing.T) { require.NotNil(t, transfer) require.NoError(t, transfer.LoadRecipient(db.DefaultContext)) - require.True(t, transfer.Recipient.CanCreateRepo()) // admin is not subject to limits + require.True(t, doerAdmin.CanCreateRepoIn(transfer.Recipient)) // admin is not subject to limits // Administrator should not be affected by the limits so transfer should be successful assert.NoError(t, AcceptTransferOwnership(db.DefaultContext, repo, doerAdmin)) @@ -158,7 +158,7 @@ func TestRepositoryTransferRejection(t *testing.T) { require.NotNil(t, transfer) require.NoError(t, transfer.LoadRecipient(db.DefaultContext)) - require.False(t, transfer.Recipient.CanCreateRepo()) // regular user is subject to limits + require.False(t, doer.CanCreateRepoIn(transfer.Recipient)) // regular user is subject to limits // Cannot accept because of the limit err = AcceptTransferOwnership(db.DefaultContext, repo, doer) diff --git a/templates/repo/pulls/fork.tmpl b/templates/repo/pulls/fork.tmpl index 9a6c44077f172..fe6ecdbf15d95 100644 --- a/templates/repo/pulls/fork.tmpl +++ b/templates/repo/pulls/fork.tmpl @@ -75,7 +75,7 @@
-
diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 202be3fce7c68..4d61604612389 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -802,7 +802,7 @@ {{end}} - {{if and .Repository.IsFork .Repository.Owner.CanCreateRepo}} + {{if .CanConvertFork}}
{{ctx.Locale.Tr "repo.settings.convert_fork"}}
@@ -916,7 +916,7 @@
{{end}} - {{if and .Repository.IsFork .Repository.Owner.CanCreateRepo}} + {{if .CanConvertFork}}