fix users being able bypass limits with repo transfers #34031
Conversation
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/S
github-actions
bot
added
modifies/api
techknowlogick
added
backport/v1.23
|
Is there still an edge case? Suppose the limit is 2, now OrgA has 1 repo. Then a user could create 100 repositories and just start the "transfer", then the checks all pass, then OrgA keep "accepting" the transfers, then OrgA will get 100 new repositories? If it is a real case, it's better to document it if it's difficult to fix, |
Completely forgot about that one, I'll take a look at it later. Of the top of my head there would be two potential solutions:
|
|
Another choice is: just document it and leave it there. I guess we do not need to make too many limits to end users ......... until somebody really really really needs to "fix" it. |
GiteaBot
added
lgtm/need 1
add admin exemption and custom error for the case
TheFox0x7
force-pushed
the
patch-repo-transfer
branch
from
08d3cca to
1517217
Compare
pull-request-size
bot
added
size/M
|
I've added a test (which will fail right now) and I have few questions - most obvious one being what doesn't reset the state? |
|
It's |
pull-request-size
bot
added
size/L
|
The name of the setting is creating limitation not owned limitation. So that I don't know whether we should do like this. |
|
I'd argue that transferring is an implicit creation - what's the difference between user creating a repo and user creating an org/second user and transferring it to bypass creation limit? Some users use this setting to prevent spam and I think this would be considered a bug by them, if even though a limit of one created repository is set, a user could create and transfer one after another with no limit. On the other hand you do bring a good point - it's probably a misusage of the setting in the first place. |
GiteaBot
added
lgtm/done
lunny
added
the
reviewed/wait-merge
GiteaBot
removed
the
reviewed/wait-merge
| switch { | ||
| case repo_model.IsErrRepoTransferInProgress(err): | ||
| ctx.APIError(http.StatusConflict, err) | ||
| return | ||
| } | ||
|
|
||
| if repo_model.IsErrRepoAlreadyExist(err) { | ||
| case repo_model.IsErrRepoAlreadyExist(err): | ||
| ctx.APIError(http.StatusUnprocessableEntity, err) | ||
| return | ||
| } | ||
|
|
||
| if errors.Is(err, user_model.ErrBlockedUser) { | ||
| case repo_service.IsRepositoryLimitReached(err): | ||
| ctx.APIError(http.StatusForbidden, err) | ||
| case errors.Is(err, user_model.ErrBlockedUser): | ||
| ctx.APIError(http.StatusForbidden, err) | ||
| } else { | ||
| default: | ||
| ctx.APIErrorInternal(err) | ||
| return |
There was a problem hiding this comment.
These case branches lose return?
This comment was marked as outdated.
This comment was marked as outdated.
GiteaBot
added
the
backport/manual
wxiaoguang
removed
backport/manual
|
Maybe not need to backport since there are too many conflicts |
* giteaofficial/main: [skip ci] Updated translations via Crowdin fix users being able bypass limits with repo transfers (go-gitea#34031) Improve pull request list api (go-gitea#34052) fix(go-gitea#34076):replace assgniee translation key (go-gitea#34077) [Fix] Resolve the problem of commit_statuses not being loaded at the top - right when switching files from the file tree (go-gitea#34079) Enable testifylint rules (go-gitea#34075) Fix markup content overflow (go-gitea#34072)
prevent user from being able to transfer repo to user who cannot have more repositories
@techknowlogick