Fix bug on branch API (#10767) (#10775)

* Fix bug on branch API (#10767)

* Fix branch api canPush and canMerge

diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go
index 037a42deec6a374abff9ecd075d820156e6e5139..2d64e9360aac8e51d1419b2de60009d35a5bd796 100644 (file)
--- a/integrations/api_branch_test.go
+++ b/integrations/api_branch_test.go
@@ -28,6 +28,8 @@ func testAPIGetBranch(t *testing.T, branchName string, exists bool) {
        var branch api.Branch
        DecodeJSON(t, resp, &branch)
        assert.EqualValues(t, branchName, branch.Name)
+       assert.True(t, branch.UserCanPush)
+       assert.True(t, branch.UserCanMerge)
 }
 
 func TestAPIGetBranch(t *testing.T) {

diff --git a/models/branches.go b/models/branches.go
index 385817e4f982bfb9a45d6edc340c3e71f398ef12..715b9d3a385a5bd2c94e95672c88e996121ba3ae 100644 (file)
--- a/models/branches.go
+++ b/models/branches.go
@@ -113,6 +113,28 @@ func (protectBranch *ProtectedBranch) CanUserMerge(userID int64) bool {
        return in
 }
 
+// IsUserMergeWhitelisted checks if some user is whitelisted to merge to this branch
+func (protectBranch *ProtectedBranch) IsUserMergeWhitelisted(userID int64) bool {
+       if !protectBranch.EnableMergeWhitelist {
+               return true
+       }
+
+       if base.Int64sContains(protectBranch.MergeWhitelistUserIDs, userID) {
+               return true
+       }
+
+       if len(protectBranch.MergeWhitelistTeamIDs) == 0 {
+               return false
+       }
+
+       in, err := IsUserInTeams(userID, protectBranch.MergeWhitelistTeamIDs)
+       if err != nil {
+               log.Error("IsUserInTeams: %v", err)
+               return false
+       }
+       return in
+}
+
 // IsUserOfficialReviewer check if user is official reviewer for the branch (counts towards required approvals)
 func (protectBranch *ProtectedBranch) IsUserOfficialReviewer(user *User) (bool, error) {
        return protectBranch.isUserOfficialReviewer(x, user)

diff --git a/modules/convert/convert.go b/modules/convert/convert.go
index 4479653174ca7c3654e8ffbdb519cb11f9731be2..b8c7dc5627864b633943045816603e6927393a95 100644 (file)
--- a/modules/convert/convert.go
+++ b/modules/convert/convert.go
@@ -30,8 +30,17 @@ func ToEmail(email *models.EmailAddress) *api.Email {
 }
 
 // ToBranch convert a git.Commit and git.Branch to an api.Branch
-func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User) *api.Branch {
+func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User) (*api.Branch, error) {
        if bp == nil {
+               var hasPerm bool
+               var err error
+               if user != nil {
+                       hasPerm, err = models.HasAccessUnit(user, repo, models.UnitTypeCode, models.AccessModeWrite)
+                       if err != nil {
+                               return nil, err
+                       }
+               }
+
                return &api.Branch{
                        Name:                b.Name,
                        Commit:              ToCommit(repo, c),
@@ -39,20 +48,25 @@ func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.
                        RequiredApprovals:   0,
                        EnableStatusCheck:   false,
                        StatusCheckContexts: []string{},
-                       UserCanPush:         true,
-                       UserCanMerge:        true,
-               }
+                       UserCanPush:         hasPerm,
+                       UserCanMerge:        hasPerm,
+               }, nil
        }
-       return &api.Branch{
+
+       branch := &api.Branch{
                Name:                b.Name,
                Commit:              ToCommit(repo, c),
                Protected:           true,
                RequiredApprovals:   bp.RequiredApprovals,
                EnableStatusCheck:   bp.EnableStatusCheck,
                StatusCheckContexts: bp.StatusCheckContexts,
-               UserCanPush:         bp.CanUserPush(user.ID),
-               UserCanMerge:        bp.CanUserMerge(user.ID),
        }
+
+       if user != nil {
+               branch.UserCanPush = bp.CanUserPush(user.ID)
+               branch.UserCanMerge = bp.IsUserMergeWhitelisted(user.ID)
+       }
+       return branch, nil
 }
 
 // ToTag convert a git.Tag to an api.Tag

diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index afc4763b522c49b36493d915e0dab2a6b52dba37..55dac7d2182a7a37536bda7c2a14f2ba38390b2f 100644 (file)
--- a/routers/api/v1/repo/branch.go
+++ b/routers/api/v1/repo/branch.go
@@ -70,7 +70,13 @@ func GetBranch(ctx *context.APIContext) {
                return
        }
 
-       ctx.JSON(http.StatusOK, convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User))
+       br, err := convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User)
+       if err != nil {
+               ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err)
+               return
+       }
+
+       ctx.JSON(http.StatusOK, br)
 }
 
 // ListBranches list all the branches of a repository
@@ -113,7 +119,14 @@ func ListBranches(ctx *context.APIContext) {
                        ctx.Error(http.StatusInternalServerError, "GetBranchProtection", err)
                        return
                }
-               apiBranches[i] = convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User)
+
+               br, err := convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User)
+               if err != nil {
+                       ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err)
+                       return
+               }
+
+               apiBranches[i] = br
        }
 
        ctx.JSON(http.StatusOK, &apiBranches)