API pull's head/base have correct permission (#17214)
close #17181 * for all pull requests API return permissions of caller * for all webhook return empty permissions Signed-off-by: Danila Kryukov <pricly_yellow@dismail.de> Co-authored-by: delvh <dev.lh@web.de> Co-authored-by: 6543 <6543@obermui.de>
This commit is contained in:
pricly-yellow
GitHub
parent
67bc04fe21
commit
4afdb1eb78
|
|
@ -17,7 +17,7 @@ import (
|
|||
// ToAPIPullRequest assumes following fields have been assigned with valid values:
|
||||
// Required - Issue
|
||||
// Optional - Merger
|
||||
func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
|
||||
func ToAPIPullRequest(pr *models.PullRequest, doer *models.User) *api.PullRequest {
|
||||
var (
|
||||
baseBranch *git.Branch
|
||||
headBranch *git.Branch
|
||||
|
|
@ -41,6 +41,12 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
|
|||
return nil
|
||||
}
|
||||
|
||||
perm, err := models.GetUserRepoPermission(pr.BaseRepo, doer)
|
||||
if err != nil {
|
||||
log.Error("GetUserRepoPermission[%d]: %v", pr.BaseRepoID, err)
|
||||
perm.AccessMode = models.AccessModeNone
|
||||
}
|
||||
|
||||
apiPullRequest := &api.PullRequest{
|
||||
ID: pr.ID,
|
||||
URL: pr.Issue.HTMLURL(),
|
||||
|
|
@ -68,7 +74,7 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
|
|||
Name: pr.BaseBranch,
|
||||
Ref: pr.BaseBranch,
|
||||
RepoID: pr.BaseRepoID,
|
||||
Repository: ToRepo(pr.BaseRepo, models.AccessModeNone),
|
||||
Repository: ToRepo(pr.BaseRepo, perm.AccessMode),
|
||||
},
|
||||
Head: &api.PRBranchInfo{
|
||||
Name: pr.HeadBranch,
|
||||
|
|
@ -114,8 +120,14 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
|
|||
}
|
||||
|
||||
if pr.HeadRepo != nil && pr.Flow == models.PullRequestFlowGithub {
|
||||
perm, err := models.GetUserRepoPermission(pr.HeadRepo, doer)
|
||||
if err != nil {
|
||||
log.Error("GetUserRepoPermission[%d]: %v", pr.HeadRepoID, err)
|
||||
perm.AccessMode = models.AccessModeNone
|
||||
}
|
||||
|
||||
apiPullRequest.Head.RepoID = pr.HeadRepo.ID
|
||||
apiPullRequest.Head.Repository = ToRepo(pr.HeadRepo, models.AccessModeNone)
|
||||
apiPullRequest.Head.Repository = ToRepo(pr.HeadRepo, perm.AccessMode)
|
||||
|
||||
headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -21,14 +21,14 @@ func TestPullRequest_APIFormat(t *testing.T) {
|
|||
pr := db.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest)
|
||||
assert.NoError(t, pr.LoadAttributes())
|
||||
assert.NoError(t, pr.LoadIssue())
|
||||
apiPullRequest := ToAPIPullRequest(pr)
|
||||
apiPullRequest := ToAPIPullRequest(pr, nil)
|
||||
assert.NotNil(t, apiPullRequest)
|
||||
assert.EqualValues(t, &structs.PRBranchInfo{
|
||||
Name: "branch1",
|
||||
Ref: "refs/pull/2/head",
|
||||
Sha: "4a357436d925b5c974181ff12a994538ddc5a269",
|
||||
RepoID: 1,
|
||||
Repository: ToRepo(headRepo, models.AccessModeNone),
|
||||
Repository: ToRepo(headRepo, models.AccessModeRead),
|
||||
}, apiPullRequest.Head)
|
||||
|
||||
//withOut HeadRepo
|
||||
|
|
@ -38,7 +38,7 @@ func TestPullRequest_APIFormat(t *testing.T) {
|
|||
// simulate fork deletion
|
||||
pr.HeadRepo = nil
|
||||
pr.HeadRepoID = 100000
|
||||
apiPullRequest = ToAPIPullRequest(pr)
|
||||
apiPullRequest = ToAPIPullRequest(pr, nil)
|
||||
assert.NotNil(t, apiPullRequest)
|
||||
assert.Nil(t, apiPullRequest.Head.Repository)
|
||||
assert.EqualValues(t, -1, apiPullRequest.Head.RepoID)
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *models.User, issue *model
|
|||
err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestLabel, &api.PullRequestPayload{
|
||||
Action: api.HookIssueLabelCleared,
|
||||
Index: issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
})
|
||||
|
|
@ -145,7 +145,7 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *mo
|
|||
issue.PullRequest.Issue = issue
|
||||
apiPullRequest := &api.PullRequestPayload{
|
||||
Index: issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
}
|
||||
|
|
@ -197,7 +197,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model
|
|||
From: oldTitle,
|
||||
},
|
||||
},
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
})
|
||||
|
|
@ -232,7 +232,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *mode
|
|||
// Merge pull request calls issue.changeStatus so we need to handle separately.
|
||||
apiPullRequest := &api.PullRequestPayload{
|
||||
Index: issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
}
|
||||
|
|
@ -301,7 +301,7 @@ func (m *webhookNotifier) NotifyNewPullRequest(pull *models.PullRequest, mention
|
|||
if err := webhook_services.PrepareWebhooks(pull.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
|
||||
Action: api.HookIssueOpened,
|
||||
Index: pull.Issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(pull),
|
||||
PullRequest: convert.ToAPIPullRequest(pull, nil),
|
||||
Repository: convert.ToRepo(pull.Issue.Repo, mode),
|
||||
Sender: convert.ToUser(pull.Issue.Poster, nil),
|
||||
}); err != nil {
|
||||
|
|
@ -322,7 +322,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *models.User, issue *mod
|
|||
From: oldContent,
|
||||
},
|
||||
},
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
})
|
||||
|
|
@ -500,7 +500,7 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *models.User, issue *mode
|
|||
err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestLabel, &api.PullRequestPayload{
|
||||
Action: api.HookIssueLabelUpdated,
|
||||
Index: issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, models.AccessModeNone),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
})
|
||||
|
|
@ -542,7 +542,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m
|
|||
err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestMilestone, &api.PullRequestPayload{
|
||||
Action: hookAction,
|
||||
Index: issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
})
|
||||
|
|
@ -609,7 +609,7 @@ func (*webhookNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mod
|
|||
// Merge pull request calls issue.changeStatus so we need to handle separately.
|
||||
apiPullRequest := &api.PullRequestPayload{
|
||||
Index: pr.Issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(pr),
|
||||
PullRequest: convert.ToAPIPullRequest(pr, nil),
|
||||
Repository: convert.ToRepo(pr.Issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
Action: api.HookIssueClosed,
|
||||
|
|
@ -642,7 +642,7 @@ func (m *webhookNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User,
|
|||
From: oldBranch,
|
||||
},
|
||||
},
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
|
||||
PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
|
||||
Repository: convert.ToRepo(issue.Repo, mode),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
})
|
||||
|
|
@ -681,7 +681,7 @@ func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review
|
|||
if err := webhook_services.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{
|
||||
Action: api.HookIssueReviewed,
|
||||
Index: review.Issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(pr),
|
||||
PullRequest: convert.ToAPIPullRequest(pr, nil),
|
||||
Repository: convert.ToRepo(review.Issue.Repo, mode),
|
||||
Sender: convert.ToUser(review.Reviewer, nil),
|
||||
Review: &api.ReviewPayload{
|
||||
|
|
@ -736,7 +736,7 @@ func (m *webhookNotifier) NotifyPullRequestSynchronized(doer *models.User, pr *m
|
|||
if err := webhook_services.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequestSync, &api.PullRequestPayload{
|
||||
Action: api.HookIssueSynchronized,
|
||||
Index: pr.Issue.Index,
|
||||
PullRequest: convert.ToAPIPullRequest(pr),
|
||||
PullRequest: convert.ToAPIPullRequest(pr, nil),
|
||||
Repository: convert.ToRepo(pr.Issue.Repo, models.AccessModeNone),
|
||||
Sender: convert.ToUser(doer, nil),
|
||||
}); err != nil {
|
||||
|
|
|
|||
|
|
@ -115,7 +115,7 @@ func ListPullRequests(ctx *context.APIContext) {
|
|||
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
|
||||
return
|
||||
}
|
||||
apiPrs[i] = convert.ToAPIPullRequest(prs[i])
|
||||
apiPrs[i] = convert.ToAPIPullRequest(prs[i], ctx.User)
|
||||
}
|
||||
|
||||
ctx.SetLinkHeader(int(maxResults), listOptions.PageSize)
|
||||
|
|
@ -171,7 +171,7 @@ func GetPullRequest(ctx *context.APIContext) {
|
|||
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
|
||||
return
|
||||
}
|
||||
ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(pr))
|
||||
ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(pr, ctx.User))
|
||||
}
|
||||
|
||||
// DownloadPullDiffOrPatch render a pull's raw diff or patch
|
||||
|
|
@ -417,7 +417,7 @@ func CreatePullRequest(ctx *context.APIContext) {
|
|||
}
|
||||
|
||||
log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID)
|
||||
ctx.JSON(http.StatusCreated, convert.ToAPIPullRequest(pr))
|
||||
ctx.JSON(http.StatusCreated, convert.ToAPIPullRequest(pr, ctx.User))
|
||||
}
|
||||
|
||||
// EditPullRequest does what it says
|
||||
|
|
@ -624,7 +624,7 @@ func EditPullRequest(ctx *context.APIContext) {
|
|||
}
|
||||
|
||||
// TODO this should be 200, not 201
|
||||
ctx.JSON(http.StatusCreated, convert.ToAPIPullRequest(pr))
|
||||
ctx.JSON(http.StatusCreated, convert.ToAPIPullRequest(pr, ctx.User))
|
||||
}
|
||||
|
||||
// IsPullRequestMerged checks if a PR exists given an index
|
||||
|
|
|
|||
Loading…
Reference in a new issue