-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adjust permissions check on creating PR to fix #6302 #6607
Adjust permissions check on creating PR to fix #6302 #6607
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6607 +/- ##
=========================================
Coverage ? 40.46%
=========================================
Files ? 405
Lines ? 54388
Branches ? 0
=========================================
Hits ? 22006
Misses ? 29356
Partials ? 3026
Continue to review full report at Codecov.
|
@@ -671,8 +671,8 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) | |||
ctx.ServerError("GetUserRepoPermission", err) | |||
return nil, nil, nil, nil, "", "" | |||
} | |||
if !perm.CanReadIssuesOrPulls(true) { | |||
log.Trace("ParseCompareInfo[%d]: cannot create/read pull requests", baseRepo.ID) | |||
if !perm.CanReadIssuesOrPulls(true) && !perm.CanWrite(models.UnitTypeCode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems it's not reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. Are you saying that this is wrong?
What should the permissions test be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note: For a parseCompare
we should have two permissions, one is for base repo, another is to head repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK So I'm not sure that's what's currently being tested.
This is unnecessary the likely issue is something to do with a migration. |
Some users in #6302 appear to be having difficulty creating pull requests since #6098.
I'm uncertain as to why this additional permissions check is required perhaps these users have a different permissions model but I'm raising this PR as possible fix.
Fixes #6302