Fix get reviewers' bug #32415
Fix get reviewers' bug #32415
Conversation
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/M
lunny
added
type/bug
backport/v1.22
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/L
github-actions
bot
added
the
modifies/api
services/pull/reviewer.go
Outdated
| if repo.Owner.Visibility.IsLimited() && doerID == 0 { | ||
| return nil, fmt.Errorf("permission denied") | ||
| } |
There was a problem hiding this comment.
permission check in services? not in router?
There was a problem hiding this comment.
Hm, why not. Because we need to load something and then check.
There was a problem hiding this comment.
First of all, the permission check I said above is the permission of access, not the permission of edit/update/create
For the access permission, if I understand correctly, the access control is in router or the handler, not in service.
The correct logic should be something like this I think:
router -> handler -x> services -> ...
↑ or ↑
access check
There was a problem hiding this comment.
And the permission check can be in another way, instead of finding all possible users and then check whether doer is in the results or not, just check whether user has the permission to access the repo(and PR?) directly.
This PR rewrites
GetReviewerfunction and move it to service layer.Reviewers should not be watchers, so that this PR removed all watchers from reviewers. When the repository is under an organization, the pull request unit read permission will be checked to resolve the bug of #32394
Fix #32394