diff --git a/models/organization/org.go b/models/organization/org.go index 3e55a36758..dc889ea17f 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 fce357a1ac..15c8c48d5b 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.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 3b268a6f41..5989be74f0 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -247,19 +247,20 @@ 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 +// NOTE: functions calling this assume a failure due to repository count limit; it ONLY checks the repo number LIMIT, if new checks are added, those functions should be revised +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 +273,12 @@ 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 { +// CanForkRepoIn ONLY checks repository count limit +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/models/user/user_test.go b/models/user/user_test.go index 2d5b6a405c..90e8bf13a8 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" @@ -616,3 +617,37 @@ func TestGetInactiveUsers(t *testing.T) { assert.NoError(t, err) assert.Empty(t, users) } + +func TestCanCreateRepo(t *testing.T) { + defer test.MockVariableValue(&setting.Repository.MaxCreationLimit)() + const noLimit = -1 + doerNormal := &user_model.User{} + doerAdmin := &user_model.User{IsAdmin: true} + t.Run("NoGlobalLimit", func(t *testing.T) { + setting.Repository.MaxCreationLimit = noLimit + + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) + assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) + + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) + }) + + t.Run("GlobalLimit50", func(t *testing.T) { + setting.Repository.MaxCreationLimit = 50 + + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) + assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: noLimit})) // limited by global limit + assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: 100})) + + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: noLimit})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: 100})) + }) +} diff --git a/routers/web/repo/fork.go b/routers/web/repo/fork.go index 36e64bfee3..79f033659b 100644 --- a/routers/web/repo/fork.go +++ b/routers/web/repo/fork.go @@ -91,12 +91,17 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { ctx.Data["CanForkToUser"] = canForkToUser ctx.Data["Orgs"] = orgs + // TODO: this message should only be shown for the "current doer" when it is selected, just like the "new repo" page. + // msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", ctx.Doer.MaxCreationLimit()) + if canForkToUser { ctx.Data["ContextUser"] = ctx.Doer + ctx.Data["CanForkRepoInNewOwner"] = true } else if len(orgs) > 0 { ctx.Data["ContextUser"] = orgs[0] + ctx.Data["CanForkRepoInNewOwner"] = true } else { - ctx.Data["CanForkRepo"] = false + ctx.Data["CanForkRepoInNewOwner"] = false ctx.Flash.Error(ctx.Tr("repo.fork_no_valid_owners"), true) return nil } @@ -120,15 +125,6 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { // Fork render repository fork page func Fork(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("new_fork") - - if ctx.Doer.CanForkRepo() { - ctx.Data["CanForkRepo"] = 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) - ctx.Flash.Error(msg, true) - } - getForkRepository(ctx) if ctx.Written() { return @@ -141,7 +137,6 @@ 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 ctxUser := checkContextUser(ctx, form.UID) if ctx.Written() { diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index e260ea36dd..ee112b83f2 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 380fec9d4a..5552a8726c 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 6953970e28..6d5505c42c 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -40,17 +40,17 @@ func deleteFailedAdoptRepository(repoID int64) error { } // 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, @@ -65,7 +65,7 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR // 1 - create the repository database operations first err := db.WithTx(ctx, func(ctx context.Context) error { - return createRepositoryInDB(ctx, doer, u, repo, false) + return createRepositoryInDB(ctx, doer, owner, repo, false) }) if err != nil { return nil, err @@ -104,7 +104,7 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR return nil, fmt.Errorf("UpdateRepositoryCols: %w", 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 22dfce91f0..050544ce11 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -204,10 +204,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, } } @@ -227,9 +227,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, @@ -252,7 +252,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt // 1 - create the repository database operations first err := db.WithTx(ctx, func(ctx context.Context) error { - return createRepositoryInDB(ctx, doer, u, repo, false) + return createRepositoryInDB(ctx, doer, owner, repo, false) }) if err != nil { return nil, err diff --git a/services/repository/fork.go b/services/repository/fork.go index daf6913510..1794cc18ab 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 b3f34e4858..95f585cead 100644 --- a/services/repository/template.go +++ b/services/repository/template.go @@ -68,7 +68,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 4e44b90df2..86917ad285 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 bf71c7ca2e..80a073e9f9 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 9a6c44077f..adf9e250c1 100644 --- a/templates/repo/pulls/fork.tmpl +++ b/templates/repo/pulls/fork.tmpl @@ -75,7 +75,7 @@