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>

diff --git a/modules/convert/pull.go b/modules/convert/pull.go
index 6c5d15c82ec443c54758ba36ecb5325c07fdeecc..ab17c13421357c70ef898d76b7a795f6ec189358 100644 (file)
--- a/modules/convert/pull.go
+++ b/modules/convert/pull.go
@@ -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 {

diff --git a/modules/convert/pull_test.go b/modules/convert/pull_test.go
index 655fc763297ca51610f13473485aec353865917d..a2b1e12a3718738aa7ba45b205eb1814976dddaa 100644 (file)
--- a/modules/convert/pull_test.go
+++ b/modules/convert/pull_test.go
@@ -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)

diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go
index acdb91efe373e3a19cdbe710a4ee05bea09a8334..c767acf134bb260a951ba12d3ff19b0039754e3d 100644 (file)
--- a/modules/notification/webhook/webhook.go
+++ b/modules/notification/webhook/webhook.go
@@ -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 {

diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 6718224d7dff1d4cd005df313625f6c027a9a255..d8e0d8099ab0dd46778bda26416c22b7fa7eb082 100644 (file)
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -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