Sign protected branches (#8993)

* Move SignMerge to PullRequest

* Add approved signing mode

* As per @guillep2k comment

custom/conf/app.ini.sample

@@ -110,6 +110,7 @@ WIKI = never
 ; Determines when to sign on merges
 ; - basesigned: require that the parent of commit on the base repo is signed.
 ; - commitssigned: require that all the commits in the head branch are signed.
+; - approved: only sign when merging an approved pr to a protected branch
 MERGES = pubkey, twofa, basesigned, commitssigned
 
 [cors]

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -96,7 +96,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
 - `CRUD_ACTIONS`: **pubkey, twofa, parentsigned**: \[never, pubkey, twofa, parentsigned, always\]: Sign CRUD actions.
   - Options as above, with the addition of:
   - `parentsigned`: Only sign if the parent commit is signed.
-- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, basesigned, commitssigned, always\]: Sign merges.
+- `MERGES`: **pubkey, twofa, basesigned, commitssigned**: \[never, pubkey, twofa, approved, basesigned, commitssigned, always\]: Sign merges.
+  - `approved`: Only sign approved merges to a protected branch.
   - `basesigned`: Only sign if the parent commit in the base repo is signed.
   - `headsigned`: Only sign if the head commit in the head branch is signed.
   - `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed.

docs/content/doc/advanced/signing.en-us.md

@@ -136,6 +136,7 @@ The possible options are:
 * `basesigned`: Only sign if the parent commit in the base repo is signed.
 * `headsigned`: Only sign if the head commit in the head branch is signed.
 * `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed.
+* `approved`: Only sign approved merges to a protected branch.
 * `always`: Always sign
 
 Options other than `never` and `always` can be combined as a comma

models/pull_sign.go

@@ -0,0 +1,121 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+import (
+	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// SignMerge determines if we should sign a PR merge commit to the base repository
+func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) {
+	if err := pr.GetBaseRepo(); err != nil {
+		log.Error("Unable to get Base Repo for pull request")
+		return false, ""
+	}
+	repo := pr.BaseRepo
+
+	signingKey := signingKey(repo.RepoPath())
+	if signingKey == "" {
+		return false, ""
+	}
+	rules := signingModeFromStrings(setting.Repository.Signing.Merges)
+
+	var gitRepo *git.Repository
+	var err error
+
+	for _, rule := range rules {
+		switch rule {
+		case never:
+			return false, ""
+		case always:
+			break
+		case pubkey:
+			keys, err := ListGPGKeys(u.ID)
+			if err != nil || len(keys) == 0 {
+				return false, ""
+			}
+		case twofa:
+			twofa, err := GetTwoFactorByUID(u.ID)
+			if err != nil || twofa == nil {
+				return false, ""
+			}
+		case approved:
+			protectedBranch, err := GetProtectedBranchBy(repo.ID, pr.BaseBranch)
+			if err != nil || protectedBranch == nil {
+				return false, ""
+			}
+			if protectedBranch.GetGrantedApprovalsCount(pr) < 1 {
+				return false, ""
+			}
+		case baseSigned:
+			if gitRepo == nil {
+				gitRepo, err = git.OpenRepository(tmpBasePath)
+				if err != nil {
+					return false, ""
+				}
+				defer gitRepo.Close()
+			}
+			commit, err := gitRepo.GetCommit(baseCommit)
+			if err != nil {
+				return false, ""
+			}
+			verification := ParseCommitWithSignature(commit)
+			if !verification.Verified {
+				return false, ""
+			}
+		case headSigned:
+			if gitRepo == nil {
+				gitRepo, err = git.OpenRepository(tmpBasePath)
+				if err != nil {
+					return false, ""
+				}
+				defer gitRepo.Close()
+			}
+			commit, err := gitRepo.GetCommit(headCommit)
+			if err != nil {
+				return false, ""
+			}
+			verification := ParseCommitWithSignature(commit)
+			if !verification.Verified {
+				return false, ""
+			}
+		case commitsSigned:
+			if gitRepo == nil {
+				gitRepo, err = git.OpenRepository(tmpBasePath)
+				if err != nil {
+					return false, ""
+				}
+				defer gitRepo.Close()
+			}
+			commit, err := gitRepo.GetCommit(headCommit)
+			if err != nil {
+				return false, ""
+			}
+			verification := ParseCommitWithSignature(commit)
+			if !verification.Verified {
+				return false, ""
+			}
+			// need to work out merge-base
+			mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit)
+			if err != nil {
+				return false, ""
+			}
+			commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit)
+			if err != nil {
+				return false, ""
+			}
+			for e := commitList.Front(); e != nil; e = e.Next() {
+				commit = e.Value.(*git.Commit)
+				verification := ParseCommitWithSignature(commit)
+				if !verification.Verified {
+					return false, ""
+				}
+			}
+		}
+	}
+	return true, signingKey
+}

models/repo_sign.go

@@ -24,6 +24,7 @@ const (
 	baseSigned    signingMode = "basesigned"
 	headSigned    signingMode = "headsigned"
 	commitsSigned signingMode = "commitssigned"
+	approved      signingMode = "approved"
 )
 
 func signingModeFromStrings(modeStrings []string) []signingMode {
@@ -45,6 +46,8 @@ func signingModeFromStrings(modeStrings []string) []signingMode {
 			fallthrough
 		case headSigned:
 			fallthrough
+		case approved:
+			fallthrough
 		case commitsSigned:
 			returnable = append(returnable, signMode)
 		}
@@ -211,98 +214,3 @@ func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string
 	}
 	return true, signingKey
 }
-
-// SignMerge determines if we should sign a merge commit to this repository
-func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) {
-	rules := signingModeFromStrings(setting.Repository.Signing.Merges)
-	signingKey := signingKey(repo.RepoPath())
-	if signingKey == "" {
-		return false, ""
-	}
-	var gitRepo *git.Repository
-	var err error
-
-	for _, rule := range rules {
-		switch rule {
-		case never:
-			return false, ""
-		case always:
-			break
-		case pubkey:
-			keys, err := ListGPGKeys(u.ID)
-			if err != nil || len(keys) == 0 {
-				return false, ""
-			}
-		case twofa:
-			twofa, err := GetTwoFactorByUID(u.ID)
-			if err != nil || twofa == nil {
-				return false, ""
-			}
-		case baseSigned:
-			if gitRepo == nil {
-				gitRepo, err = git.OpenRepository(tmpBasePath)
-				if err != nil {
-					return false, ""
-				}
-				defer gitRepo.Close()
-			}
-			commit, err := gitRepo.GetCommit(baseCommit)
-			if err != nil {
-				return false, ""
-			}
-			verification := ParseCommitWithSignature(commit)
-			if !verification.Verified {
-				return false, ""
-			}
-		case headSigned:
-			if gitRepo == nil {
-				gitRepo, err = git.OpenRepository(tmpBasePath)
-				if err != nil {
-					return false, ""
-				}
-				defer gitRepo.Close()
-			}
-			commit, err := gitRepo.GetCommit(headCommit)
-			if err != nil {
-				return false, ""
-			}
-			verification := ParseCommitWithSignature(commit)
-			if !verification.Verified {
-				return false, ""
-			}
-		case commitsSigned:
-			if gitRepo == nil {
-				gitRepo, err = git.OpenRepository(tmpBasePath)
-				if err != nil {
-					return false, ""
-				}
-				defer gitRepo.Close()
-			}
-			commit, err := gitRepo.GetCommit(headCommit)
-			if err != nil {
-				return false, ""
-			}
-			verification := ParseCommitWithSignature(commit)
-			if !verification.Verified {
-				return false, ""
-			}
-			// need to work out merge-base
-			mergeBaseCommit, _, err := gitRepo.GetMergeBase("", baseCommit, headCommit)
-			if err != nil {
-				return false, ""
-			}
-			commitList, err := commit.CommitsBeforeUntil(mergeBaseCommit)
-			if err != nil {
-				return false, ""
-			}
-			for e := commitList.Front(); e != nil; e = e.Next() {
-				commit = e.Value.(*git.Commit)
-				verification := ParseCommitWithSignature(commit)
-				if !verification.Verified {
-					return false, ""
-				}
-			}
-		}
-	}
-	return true, signingKey
-}

services/pull/merge.go

@@ -162,7 +162,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor
 	// Determine if we should sign
 	signArg := ""
 	if version.Compare(binVersion, "1.7.9", ">=") {
-		sign, keyID := pr.BaseRepo.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
+		sign, keyID := pr.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
 		if sign {
 			signArg = "-S" + keyID
 		} else if version.Compare(binVersion, "2.0.0", ">=") {