Skip to content

Commit 4d23231

Browse files
authored
fix users being able bypass limits with repo transfers (#34031)
prevent user from being able to transfer repo to user who cannot have more repositories
1 parent a2e8a28 commit 4d23231

File tree

6 files changed

+91
-15
lines changed

6 files changed

+91
-15
lines changed

models/fixtures/repo_transfer.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,11 @@
2121
repo_id: 32
2222
created_unix: 1553610671
2323
updated_unix: 1553610671
24+
25+
-
26+
id: 4
27+
doer_id: 3
28+
recipient_id: 1
29+
repo_id: 5
30+
created_unix: 1553610671
31+
updated_unix: 1553610671

routers/api/v1/repo/transfer.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,19 @@ func Transfer(ctx *context.APIContext) {
108108
oldFullname := ctx.Repo.Repository.FullName()
109109

110110
if err := repo_service.StartRepositoryTransfer(ctx, ctx.Doer, newOwner, ctx.Repo.Repository, teams); err != nil {
111-
if repo_model.IsErrRepoTransferInProgress(err) {
111+
switch {
112+
case repo_model.IsErrRepoTransferInProgress(err):
112113
ctx.APIError(http.StatusConflict, err)
113-
return
114-
}
115-
116-
if repo_model.IsErrRepoAlreadyExist(err) {
114+
case repo_model.IsErrRepoAlreadyExist(err):
117115
ctx.APIError(http.StatusUnprocessableEntity, err)
118-
return
119-
}
120-
121-
if errors.Is(err, user_model.ErrBlockedUser) {
116+
case repo_service.IsRepositoryLimitReached(err):
117+
ctx.APIError(http.StatusForbidden, err)
118+
case errors.Is(err, user_model.ErrBlockedUser):
122119
ctx.APIError(http.StatusForbidden, err)
123-
} else {
120+
default:
124121
ctx.APIErrorInternal(err)
122+
return
125123
}
126-
return
127124
}
128125

129126
if ctx.Repo.Repository.Status == repo_model.RepositoryPendingTransfer {
@@ -169,6 +166,8 @@ func AcceptTransfer(ctx *context.APIContext) {
169166
ctx.APIError(http.StatusNotFound, err)
170167
case errors.Is(err, util.ErrPermissionDenied):
171168
ctx.APIError(http.StatusForbidden, err)
169+
case repo_service.IsRepositoryLimitReached(err):
170+
ctx.APIError(http.StatusForbidden, err)
172171
default:
173172
ctx.APIErrorInternal(err)
174173
}

routers/web/repo/repo.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,15 @@ func CreatePost(ctx *context.Context) {
305305
}
306306

307307
func handleActionError(ctx *context.Context, err error) {
308-
if errors.Is(err, user_model.ErrBlockedUser) {
308+
switch {
309+
case errors.Is(err, user_model.ErrBlockedUser):
309310
ctx.Flash.Error(ctx.Tr("repo.action.blocked_user"))
310-
} else if errors.Is(err, util.ErrPermissionDenied) {
311+
case repo_service.IsRepositoryLimitReached(err):
312+
limit := err.(repo_service.LimitReachedError).Limit
313+
ctx.Flash.Error(ctx.TrN(limit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", limit))
314+
case errors.Is(err, util.ErrPermissionDenied):
311315
ctx.HTTPError(http.StatusNotFound)
312-
} else {
316+
default:
313317
ctx.ServerError(fmt.Sprintf("Action (%s)", ctx.PathParam("action")), err)
314318
}
315319
}

routers/web/repo/setting/setting.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,9 @@ func handleSettingsPostTransfer(ctx *context.Context) {
848848
ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil)
849849
} else if repo_model.IsErrRepoTransferInProgress(err) {
850850
ctx.RenderWithErr(ctx.Tr("repo.settings.transfer_in_progress"), tplSettingsOptions, nil)
851+
} else if repo_service.IsRepositoryLimitReached(err) {
852+
limit := err.(repo_service.LimitReachedError).Limit
853+
ctx.RenderWithErr(ctx.TrN(limit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", limit), tplSettingsOptions, nil)
851854
} else if errors.Is(err, user_model.ErrBlockedUser) {
852855
ctx.RenderWithErr(ctx.Tr("repo.settings.transfer.blocked_user"), tplSettingsOptions, nil)
853856
} else {

services/repository/transfer.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,22 @@ import (
2020
"code.gitea.io/gitea/modules/gitrepo"
2121
"code.gitea.io/gitea/modules/globallock"
2222
"code.gitea.io/gitea/modules/log"
23+
"code.gitea.io/gitea/modules/setting"
2324
"code.gitea.io/gitea/modules/util"
2425
notify_service "code.gitea.io/gitea/services/notify"
2526
)
2627

28+
type LimitReachedError struct{ Limit int }
29+
30+
func (LimitReachedError) Error() string {
31+
return "Repository limit has been reached"
32+
}
33+
34+
func IsRepositoryLimitReached(err error) bool {
35+
_, ok := err.(LimitReachedError)
36+
return ok
37+
}
38+
2739
func getRepoWorkingLockKey(repoID int64) string {
2840
return fmt.Sprintf("repo_working_%d", repoID)
2941
}
@@ -49,6 +61,11 @@ func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, d
4961
return err
5062
}
5163

64+
if !doer.IsAdmin && !repoTransfer.Recipient.CanCreateRepo() {
65+
limit := util.Iif(repoTransfer.Recipient.MaxRepoCreation >= 0, repoTransfer.Recipient.MaxRepoCreation, setting.Repository.MaxCreationLimit)
66+
return LimitReachedError{Limit: limit}
67+
}
68+
5269
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
5370
return util.ErrPermissionDenied
5471
}
@@ -399,6 +416,11 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
399416
return err
400417
}
401418

419+
if !doer.IsAdmin && !newOwner.CanCreateRepo() {
420+
limit := util.Iif(newOwner.MaxRepoCreation >= 0, newOwner.MaxRepoCreation, setting.Repository.MaxCreationLimit)
421+
return LimitReachedError{Limit: limit}
422+
}
423+
402424
var isDirectTransfer bool
403425
oldOwnerName := repo.OwnerName
404426

services/repository/transfer_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2019 The Gitea Authors. All rights reserved.
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
22
// SPDX-License-Identifier: MIT
33

44
package repository
@@ -14,11 +14,14 @@ import (
1414
repo_model "code.gitea.io/gitea/models/repo"
1515
"code.gitea.io/gitea/models/unittest"
1616
user_model "code.gitea.io/gitea/models/user"
17+
"code.gitea.io/gitea/modules/setting"
18+
"code.gitea.io/gitea/modules/test"
1719
"code.gitea.io/gitea/modules/util"
1820
"code.gitea.io/gitea/services/feed"
1921
notify_service "code.gitea.io/gitea/services/notify"
2022

2123
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
2225
)
2326

2427
var notifySync sync.Once
@@ -125,3 +128,40 @@ func TestRepositoryTransfer(t *testing.T) {
125128
err = RejectRepositoryTransfer(db.DefaultContext, repo2, doer)
126129
assert.True(t, repo_model.IsErrNoPendingTransfer(err))
127130
}
131+
132+
// Test transfer rejections
133+
func TestRepositoryTransferRejection(t *testing.T) {
134+
require.NoError(t, unittest.PrepareTestDatabase())
135+
// Set limit to 0 repositories so no repositories can be transferred
136+
defer test.MockVariableValue(&setting.Repository.MaxCreationLimit, 0)()
137+
138+
// Admin case
139+
doerAdmin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
140+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 5})
141+
142+
transfer, err := repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo)
143+
require.NoError(t, err)
144+
require.NotNil(t, transfer)
145+
require.NoError(t, transfer.LoadRecipient(db.DefaultContext))
146+
147+
require.True(t, transfer.Recipient.CanCreateRepo()) // admin is not subject to limits
148+
149+
// Administrator should not be affected by the limits so transfer should be successful
150+
assert.NoError(t, AcceptTransferOwnership(db.DefaultContext, repo, doerAdmin))
151+
152+
// Non admin user case
153+
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10})
154+
repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21})
155+
156+
transfer, err = repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo)
157+
require.NoError(t, err)
158+
require.NotNil(t, transfer)
159+
require.NoError(t, transfer.LoadRecipient(db.DefaultContext))
160+
161+
require.False(t, transfer.Recipient.CanCreateRepo()) // regular user is subject to limits
162+
163+
// Cannot accept because of the limit
164+
err = AcceptTransferOwnership(db.DefaultContext, repo, doer)
165+
assert.Error(t, err)
166+
assert.True(t, IsRepositoryLimitReached(err))
167+
}

0 commit comments

Comments
 (0)