From dcaa5643d70cd1be72e53e82a30d2bc6e5f8de92 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Sat, 21 Mar 2020 11:41:33 +0800
Subject: [PATCH] Fix branch api canPush and canMerge (#10776)

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
---
 integrations/api_branch_test.go |  2 ++
 modules/convert/convert.go      | 54 +++++++++++++++++++--------------
 routers/api/v1/repo/branch.go   | 14 +++++++--
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go
index 3fe7f23229d..b6452a6ab4e 100644
--- 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 testAPIGetBranchProtection(t *testing.T, branchName string, expectedHTTPStatus int) {
diff --git a/modules/convert/convert.go b/modules/convert/convert.go
index 240db77d2ce..d75a1305355 100644
--- a/modules/convert/convert.go
+++ b/modules/convert/convert.go
@@ -30,40 +30,48 @@ 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, isRepoAdmin bool) *api.Branch {
+func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models.ProtectedBranch, user *models.User, isRepoAdmin bool) (*api.Branch, error) {
 	if bp == nil {
-		return &api.Branch{
-			Name:                          b.Name,
-			Commit:                        ToCommit(repo, c),
-			Protected:                     false,
-			RequiredApprovals:             0,
-			EnableStatusCheck:             false,
-			StatusCheckContexts:           []string{},
-			UserCanPush:                   true,
-			UserCanMerge:                  true,
-			EffectiveBranchProtectionName: "",
+		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
+			}
 		}
-	}
-	branchProtectionName := ""
-	if isRepoAdmin {
-		branchProtectionName = bp.BranchName
+
+		return &api.Branch{
+			Name:                b.Name,
+			Commit:              ToCommit(repo, c),
+			Protected:           false,
+			RequiredApprovals:   0,
+			EnableStatusCheck:   false,
+			StatusCheckContexts: []string{},
+			UserCanPush:         hasPerm,
+			UserCanMerge:        hasPerm,
+		}, nil
 	}
 
 	branch := &api.Branch{
-		Name:                          b.Name,
-		Commit:                        ToCommit(repo, c),
-		Protected:                     true,
-		RequiredApprovals:             bp.RequiredApprovals,
-		EnableStatusCheck:             bp.EnableStatusCheck,
-		StatusCheckContexts:           bp.StatusCheckContexts,
-		EffectiveBranchProtectionName: branchProtectionName,
+		Name:                b.Name,
+		Commit:              ToCommit(repo, c),
+		Protected:           true,
+		RequiredApprovals:   bp.RequiredApprovals,
+		EnableStatusCheck:   bp.EnableStatusCheck,
+		StatusCheckContexts: bp.StatusCheckContexts,
+	}
+
+	if isRepoAdmin {
+		branch.EffectiveBranchProtectionName = bp.BranchName
 	}
 
 	if user != nil {
 		branch.UserCanPush = bp.CanUserPush(user.ID)
 		branch.UserCanMerge = bp.IsUserMergeWhitelisted(user.ID)
 	}
-	return branch
+
+	return branch, nil
 }
 
 // ToBranchProtection convert a ProtectedBranch to api.BranchProtection
diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go
index fccfc2bfe1a..0d4501cd793 100644
--- a/routers/api/v1/repo/branch.go
+++ b/routers/api/v1/repo/branch.go
@@ -72,7 +72,13 @@ func GetBranch(ctx *context.APIContext) {
 		return
 	}
 
-	ctx.JSON(http.StatusOK, convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User, ctx.Repo.IsAdmin()))
+	br, err := convert.ToBranch(ctx.Repo.Repository, branch, c, branchProtection, ctx.User, ctx.Repo.IsAdmin())
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err)
+		return
+	}
+
+	ctx.JSON(http.StatusOK, br)
 }
 
 // ListBranches list all the branches of a repository
@@ -115,7 +121,11 @@ 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, ctx.Repo.IsAdmin())
+		apiBranches[i], err = convert.ToBranch(ctx.Repo.Repository, branches[i], c, branchProtection, ctx.User, ctx.Repo.IsAdmin())
+		if err != nil {
+			ctx.Error(http.StatusInternalServerError, "convert.ToBranch", err)
+			return
+		}
 	}
 
 	ctx.JSON(http.StatusOK, &apiBranches)