Only check at least one email gpg key (#2266)

* Only require one email (possibly not yet validated)

* Update message error and check validation of commit

* Add integrations tests

* Complete integration for import

* Add pre-check/optimization

* Add some test (not finished)

* Finish

* Fix fixtures

* Fix typo

* Don't guess key ID

4 files changed, 49 insertions, 21 deletions

diff --git a/models/error.go b/models/error.go
index 74551ad8f7..2adb4b45e1 100644
--- a/models/error.go
+++ b/models/error.go
@@ -4,9 +4,7 @@
 
 package models
 
-import (
-	"fmt"
-)
+import "fmt"
 
 // ErrNameReserved represents a "reserved name" error.
 type ErrNameReserved struct {
@@ -260,19 +258,19 @@ func (err ErrKeyNameAlreadyUsed) Error() string {
 	return fmt.Sprintf("public key already exists [owner_id: %d, name: %s]", err.OwnerID, err.Name)
 }
 
-// ErrGPGEmailNotFound represents a "ErrGPGEmailNotFound" kind of error.
-type ErrGPGEmailNotFound struct {
-	Email string
+// ErrGPGNoEmailFound represents a "ErrGPGNoEmailFound" kind of error.
+type ErrGPGNoEmailFound struct {
+	FailedEmails []string
 }
 
-// IsErrGPGEmailNotFound checks if an error is a ErrGPGEmailNotFound.
-func IsErrGPGEmailNotFound(err error) bool {
-	_, ok := err.(ErrGPGEmailNotFound)
+// IsErrGPGNoEmailFound checks if an error is a ErrGPGNoEmailFound.
+func IsErrGPGNoEmailFound(err error) bool {
+	_, ok := err.(ErrGPGNoEmailFound)
 	return ok
 }
 
-func (err ErrGPGEmailNotFound) Error() string {
-	return fmt.Sprintf("failed to found email or is not confirmed : %s", err.Email)
+func (err ErrGPGNoEmailFound) Error() string {
+	return fmt.Sprintf("none of the emails attached to the GPG key could be found: %v", err.FailedEmails)
 }
 
 // ErrGPGKeyParsing represents a "ErrGPGKeyParsing" kind of error.
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index b10b85cf24..c6b39d3f4c 100644
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -176,3 +176,15 @@
   lower_name: repo15
   name: repo15
   is_bare: true
+
+-
+  id: 16
+  owner_id: 2
+  lower_name: repo16
+  name: repo16
+  is_private: false
+  num_issues: 0
+  num_closed_issues: 0
+  num_pulls: 0
+  num_closed_pulls: 0
+  num_watches: 0
diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index b3e7864a6d..abd72b1168 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -26,7 +26,7 @@
   is_admin: false
   avatar: avatar2
   avatar_email: user2@example.com
-  num_repos: 3
+  num_repos: 4
   num_stars: 2
   num_followers: 2
   num_following: 1
diff --git a/models/gpg_key.go b/models/gpg_key.go
index 8d10511002..70dd4fbda0 100644
--- a/models/gpg_key.go
+++ b/models/gpg_key.go
@@ -208,21 +208,27 @@ func parseGPGKey(ownerID int64, e *openpgp.Entity) (*GPGKey, error) {
 	if err != nil {
 		return nil, err
 	}
-	emails := make([]*EmailAddress, len(e.Identities))
-	n := 0
+
+	emails := make([]*EmailAddress, 0, len(e.Identities))
 	for _, ident := range e.Identities {
 		email := strings.ToLower(strings.TrimSpace(ident.UserId.Email))
 		for _, e := range userEmails {
-			if e.Email == email && e.IsActivated {
-				emails[n] = e
+			if e.Email == email {
+				emails = append(emails, e)
 				break
 			}
 		}
-		if emails[n] == nil {
-			return nil, ErrGPGEmailNotFound{ident.UserId.Email}
+	}
+
+	//In the case no email as been found
+	if len(emails) == 0 {
+		failedEmails := make([]string, 0, len(e.Identities))
+		for _, ident := range e.Identities {
+			failedEmails = append(failedEmails, ident.UserId.Email)
 		}
-		n++
+		return nil, ErrGPGNoEmailFound{failedEmails}
 	}
+
 	content, err := base64EncPubKey(pubkey)
 	if err != nil {
 		return nil, err
@@ -380,8 +386,8 @@ func ParseCommitWithSignature(c *git.Commit) *CommitVerification {
 		}
 
 		//Find Committer account
-		committer, err := GetUserByEmail(c.Committer.Email)
-		if err != nil { //Skipping not user for commiter
+		committer, err := GetUserByEmail(c.Committer.Email) //This find the user by primary email or activated email so commit will not be valid if email is not
+		if err != nil {                                     //Skipping not user for commiter
 			log.Error(3, "NoCommitterAccount: %v", err)
 			return &CommitVerification{
 				Verified: false,
@@ -399,6 +405,18 @@ func ParseCommitWithSignature(c *git.Commit) *CommitVerification {
 		}
 
 		for _, k := range keys {
+			//Pre-check (& optimization) that emails attached to key can be attached to the commiter email and can validate
+			canValidate := false
+			for _, e := range k.Emails {
+				if e.IsActivated && e.Email == c.Committer.Email {
+					canValidate = true
+					break
+				}
+			}
+			if !canValidate {
+				continue //Skip this key
+			}
+
 			//Generating hash of commit
 			hash, err := populateHash(sig.Hash, []byte(c.Signature.Payload))
 			if err != nil { //Skipping ailed to generate hash