lavender-legacy/gitea
7
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

API pull's head/base have correct permission(#17214) (#17245)

Browse source 
* for all pull requests API return permissions of caller
* for all webhook return empty permissions

Signed-off-by: Danila Kryukov <pricly_yellow@dismail.de>

* Fix incorrect error handler

Co-authored-by: delvh <dev.lh@web.de>

* Fix wrong assumption in tests

* Change paramenter name to doer to indicate source

Co-authored-by: 6543 <6543@obermui.de>

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: 6543 <6543@obermui.de>
This commit is contained in:
pricly-yellow 2021-10-07 16:39:23 +07:00 committed by GitHub
parent 6995be66e7
commit 7b1153e943
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
modules/convert/pull.go
View file

@ -17,7 +17,7 @@ import (
// ToAPIPullRequest assumes following fields have been assigned with valid values: // ToAPIPullRequest assumes following fields have been assigned with valid values:
// Required - Issue // Required - Issue
// Optional - Merger // Optional - Merger
func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest { func ToAPIPullRequest(pr *models.PullRequest, doer *models.User) *api.PullRequest {
var ( var (
baseBranch *git.Branch baseBranch *git.Branch
headBranch *git.Branch headBranch *git.Branch
@ -41,6 +41,12 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
return nil 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{ apiPullRequest := &api.PullRequest{
ID: pr.ID, ID: pr.ID,
URL: pr.Issue.HTMLURL(), URL: pr.Issue.HTMLURL(),
@ -68,7 +74,7 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
Name: pr.BaseBranch, Name: pr.BaseBranch,
Ref: pr.BaseBranch, Ref: pr.BaseBranch,
RepoID: pr.BaseRepoID, RepoID: pr.BaseRepoID,
Repository: ToRepo(pr.BaseRepo, models.AccessModeNone), Repository: ToRepo(pr.BaseRepo, perm.AccessMode),
}, },
Head: &api.PRBranchInfo{ Head: &api.PRBranchInfo{
Name: pr.HeadBranch, Name: pr.HeadBranch,
@ -96,8 +102,14 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
} }
if pr.HeadRepo != nil { if pr.HeadRepo != nil {
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.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()) headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil { if err != nil {

6
modules/convert/pull_test.go
View file

@ -20,14 +20,14 @@ func TestPullRequest_APIFormat(t *testing.T) {
pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest) pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest)
assert.NoError(t, pr.LoadAttributes()) assert.NoError(t, pr.LoadAttributes())
assert.NoError(t, pr.LoadIssue()) assert.NoError(t, pr.LoadIssue())
apiPullRequest := ToAPIPullRequest(pr) apiPullRequest := ToAPIPullRequest(pr, nil)
assert.NotNil(t, apiPullRequest) assert.NotNil(t, apiPullRequest)
assert.EqualValues(t, &structs.PRBranchInfo{ assert.EqualValues(t, &structs.PRBranchInfo{
Name: "branch1", Name: "branch1",
Ref: "refs/pull/2/head", Ref: "refs/pull/2/head",
Sha: "4a357436d925b5c974181ff12a994538ddc5a269", Sha: "4a357436d925b5c974181ff12a994538ddc5a269",
RepoID: 1, RepoID: 1,
Repository: ToRepo(headRepo, models.AccessModeNone), Repository: ToRepo(headRepo, models.AccessModeRead),
}, apiPullRequest.Head) }, apiPullRequest.Head)
//withOut HeadRepo //withOut HeadRepo
@ -37,7 +37,7 @@ func TestPullRequest_APIFormat(t *testing.T) {
// simulate fork deletion // simulate fork deletion
pr.HeadRepo = nil pr.HeadRepo = nil
pr.HeadRepoID = 100000 pr.HeadRepoID = 100000
apiPullRequest = ToAPIPullRequest(pr) apiPullRequest = ToAPIPullRequest(pr, nil)
assert.NotNil(t, apiPullRequest) assert.NotNil(t, apiPullRequest)
assert.Nil(t, apiPullRequest.Head.Repository) assert.Nil(t, apiPullRequest.Head.Repository)
assert.EqualValues(t, -1, apiPullRequest.Head.RepoID) assert.EqualValues(t, -1, apiPullRequest.Head.RepoID)

24
modules/notification/webhook/webhook.go
View file

@ -51,7 +51,7 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *models.User, issue *model
err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestLabel, &api.PullRequestPayload{ err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestLabel, &api.PullRequestPayload{
Action: api.HookIssueLabelCleared, Action: api.HookIssueLabelCleared,
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), Sender: convert.ToUser(doer, nil),
}) })
@ -145,7 +145,7 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *mo
issue.PullRequest.Issue = issue issue.PullRequest.Issue = issue
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), Sender: convert.ToUser(doer, nil),
} }
@ -197,7 +197,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model
From: oldTitle, From: oldTitle,
}, },
}, },
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), 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. // Merge pull request calls issue.changeStatus so we need to handle separately.
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), 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{ if err := webhook_services.PrepareWebhooks(pull.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
Action: api.HookIssueOpened, Action: api.HookIssueOpened,
Index: pull.Issue.Index, Index: pull.Issue.Index,
PullRequest: convert.ToAPIPullRequest(pull), PullRequest: convert.ToAPIPullRequest(pull, nil),
Repository: convert.ToRepo(pull.Issue.Repo, mode), Repository: convert.ToRepo(pull.Issue.Repo, mode),
Sender: convert.ToUser(pull.Issue.Poster, nil), Sender: convert.ToUser(pull.Issue.Poster, nil),
}); err != nil { }); err != nil {
@ -322,7 +322,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *models.User, issue *mod
From: oldContent, From: oldContent,
}, },
}, },
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), 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{ err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestLabel, &api.PullRequestPayload{
Action: api.HookIssueLabelUpdated, Action: api.HookIssueLabelUpdated,
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, models.AccessModeNone), Repository: convert.ToRepo(issue.Repo, models.AccessModeNone),
Sender: convert.ToUser(doer, nil), 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{ err = webhook_services.PrepareWebhooks(issue.Repo, models.HookEventPullRequestMilestone, &api.PullRequestPayload{
Action: hookAction, Action: hookAction,
Index: issue.Index, Index: issue.Index,
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), 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. // Merge pull request calls issue.changeStatus so we need to handle separately.
apiPullRequest := &api.PullRequestPayload{ apiPullRequest := &api.PullRequestPayload{
Index: pr.Issue.Index, Index: pr.Issue.Index,
PullRequest: convert.ToAPIPullRequest(pr), PullRequest: convert.ToAPIPullRequest(pr, nil),
Repository: convert.ToRepo(pr.Issue.Repo, mode), Repository: convert.ToRepo(pr.Issue.Repo, mode),
Sender: convert.ToUser(doer, nil), Sender: convert.ToUser(doer, nil),
Action: api.HookIssueClosed, Action: api.HookIssueClosed,
@ -642,7 +642,7 @@ func (m *webhookNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User,
From: oldBranch, From: oldBranch,
}, },
}, },
PullRequest: convert.ToAPIPullRequest(issue.PullRequest), PullRequest: convert.ToAPIPullRequest(issue.PullRequest, nil),
Repository: convert.ToRepo(issue.Repo, mode), Repository: convert.ToRepo(issue.Repo, mode),
Sender: convert.ToUser(doer, nil), 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{ if err := webhook_services.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{
Action: api.HookIssueReviewed, Action: api.HookIssueReviewed,
Index: review.Issue.Index, Index: review.Issue.Index,
PullRequest: convert.ToAPIPullRequest(pr), PullRequest: convert.ToAPIPullRequest(pr, nil),
Repository: convert.ToRepo(review.Issue.Repo, mode), Repository: convert.ToRepo(review.Issue.Repo, mode),
Sender: convert.ToUser(review.Reviewer, nil), Sender: convert.ToUser(review.Reviewer, nil),
Review: &api.ReviewPayload{ 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{ if err := webhook_services.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequestSync, &api.PullRequestPayload{
Action: api.HookIssueSynchronized, Action: api.HookIssueSynchronized,
Index: pr.Issue.Index, Index: pr.Issue.Index,
PullRequest: convert.ToAPIPullRequest(pr), PullRequest: convert.ToAPIPullRequest(pr, nil),
Repository: convert.ToRepo(pr.Issue.Repo, models.AccessModeNone), Repository: convert.ToRepo(pr.Issue.Repo, models.AccessModeNone),
Sender: convert.ToUser(doer, nil), Sender: convert.ToUser(doer, nil),
}); err != nil { }); err != nil {

8
routers/api/v1/repo/pull.go
View file

@ -115,7 +115,7 @@ func ListPullRequests(ctx *context.APIContext) {
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err) ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
return return
} }
apiPrs[i] = convert.ToAPIPullRequest(prs[i]) apiPrs[i] = convert.ToAPIPullRequest(prs[i], ctx.User)
} }
ctx.SetLinkHeader(int(maxResults), listOptions.PageSize) ctx.SetLinkHeader(int(maxResults), listOptions.PageSize)
@ -172,7 +172,7 @@ func GetPullRequest(ctx *context.APIContext) {
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err) ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
return return
} }
ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(pr)) ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(pr, ctx.User))
} }
// DownloadPullDiff render a pull's raw diff // DownloadPullDiff render a pull's raw diff
@ -437,7 +437,7 @@ func CreatePullRequest(ctx *context.APIContext) {
} }
log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID) 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 // EditPullRequest does what it says
@ -640,7 +640,7 @@ func EditPullRequest(ctx *context.APIContext) {
} }
// TODO this should be 200, not 201 // 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 // IsPullRequestMerged checks if a PR exists given an index