Skip to content

Allow repo admins too to delete the repo#23940

Merged
jolheiser merged 4 commits intogo-gitea:mainfrom
yp05327:fix-incorrect-perm-check-in-CanUserDelete
Apr 7, 2023
Merged

Allow repo admins too to delete the repo#23940
jolheiser merged 4 commits intogo-gitea:mainfrom
yp05327:fix-incorrect-perm-check-in-CanUserDelete

Conversation

@yp05327
Copy link
Contributor

@yp05327 yp05327 commented Apr 6, 2023 •
edited
Loading

Fixes #23934

We need to check AccessModeAdmin in CanUserDelete instead of AccessModeOwner

@yp05327
Copy link
Contributor Author

yp05327 commented Apr 6, 2023 •
edited
Loading

https://github.com/go-gitea/gitea/pull/17811/files#diff-3b0db4078124698cd245ecc5c5396177de97754dde9be5b233230e20a53404c4L242
image
The old design is correct, but since #17811 the logic was changed?
Have no idea about this.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 6, 2023
@lunny
Copy link
Member

lunny commented Apr 6, 2023

https://github.com/go-gitea/gitea/pull/17811/files#diff-3b0db4078124698cd245ecc5c5396177de97754dde9be5b233230e20a53404c4L242 image The old design is correct, but in #17811 the logic was changed? Have no idea about this.

The owner team is created when an org is created. Owners could create an admin team but cannot create another owner team. Admin team have almost permissions as owners except they should not remove owners from the owner team.

@yp05327
Copy link
Contributor Author

yp05327 commented Apr 6, 2023

It seems that we need to upgrade the libraries. 😂
image

@lunny
Copy link
Member

lunny commented Apr 6, 2023

It seems that we need to upgrade the libraries. 😂 image

Another PR should be sent.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 6, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 6, 2023
@lunny lunny added type/bug reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Apr 6, 2023
@delvh delvh changed the title Fix incorrect permission check in CanUserDelete Allow repo admins too to delete the repo Apr 6, 2023
@lunny lunny added this to the 1.20.0 milestone Apr 6, 2023
@jolheiser
Copy link
Member

jolheiser commented Apr 7, 2023

🎺 🤖

@jolheiser jolheiser enabled auto-merge (squash) April 7, 2023 14:40
@jolheiser jolheiser merged commit 26a0cd7 into go-gitea:main Apr 7, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 7, 2023
@yp05327 yp05327 deleted the fix-incorrect-perm-check-in-CanUserDelete branch May 22, 2023 05:06
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lunny lunny lunny approved these changes

@KN4CK3R KN4CK3R KN4CK3R approved these changes

@delvh delvh delvh approved these changes

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Milestone

1.20.0

Development

Successfully merging this pull request may close these issues.

Using the API unable to delete a Repo within an Organisation where user is in the admin team but not the owner

6 participants

@yp05327 @lunny @jolheiser @KN4CK3R @delvh @GiteaBot