mirror of
https://github.com/go-gitea/gitea.git
synced 2025-04-13 08:58:12 +00:00
Check user/org repo limit instead of doer (#34147)
This PR tries to finally fix the bug mentioned in #30011 and #15504, where the user repo limit is checked when creating a repo in an organization. Fix #30011 --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: TheFox0x7 <thefox0x7@gmail.com>
This commit is contained in:
parent
a100ac3306
commit
fd7c364ca6
@ -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
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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.
|
||||
|
@ -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}))
|
||||
})
|
||||
}
|
||||
|
@ -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() {
|
||||
|
@ -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
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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
|
||||
|
@ -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,
|
||||
}
|
||||
|
@ -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,
|
||||
}
|
||||
|
@ -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}
|
||||
}
|
||||
|
@ -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)
|
||||
|
@ -75,7 +75,7 @@
|
||||
|
||||
<div class="inline field">
|
||||
<label></label>
|
||||
<button class="ui primary button{{if not .CanForkRepo}} disabled{{end}}">
|
||||
<button class="ui primary button{{if not .CanForkRepoInNewOwner}} disabled{{end}}">
|
||||
{{ctx.Locale.Tr "repo.fork_repo"}}
|
||||
</button>
|
||||
</div>
|
||||
|
@ -802,7 +802,7 @@
|
||||
</div>
|
||||
</div>
|
||||
{{end}}
|
||||
{{if and .Repository.IsFork .Repository.Owner.CanCreateRepo}}
|
||||
{{if .CanConvertFork}}
|
||||
<div class="flex-item">
|
||||
<div class="flex-item-main">
|
||||
<div class="flex-item-title">{{ctx.Locale.Tr "repo.settings.convert_fork"}}</div>
|
||||
@ -916,7 +916,7 @@
|
||||
</div>
|
||||
</div>
|
||||
{{end}}
|
||||
{{if and .Repository.IsFork .Repository.Owner.CanCreateRepo}}
|
||||
{{if .CanConvertFork}}
|
||||
<div class="ui small modal" id="convert-fork-repo-modal">
|
||||
<div class="header">
|
||||
{{ctx.Locale.Tr "repo.settings.convert_fork"}}
|
||||
|
Loading…
x
Reference in New Issue
Block a user