Revised committer verification to require email address

Change-Id: I5298c93e03099813f5713a4effd87913429aa3dc

diff --git a/releases.moxie b/releases.moxie
index dd06c8234e073224e9ac13b75f3f5666f470a61c..d834f230742e7401aa3de78bca86c58c65878229 100644 (file)
--- a/releases.moxie
+++ b/releases.moxie
@@ -35,6 +35,7 @@ r20: {
        - Removed docs indicator on the repositories page
        - Removed the repository setting to enable Markdown document enumeration, this is now automatic and expanded
        - Retrieve LDAP groups with dereferencing aliases (pr-122)
+       - Revised committer verification to require a matching displayname or account name AND the email address
     additions:
        - Added an optional MirrorExecutor which will periodically fetch ref updates from source repositories for mirrors (issue-5).  Repositories must be manually cloned using native git and "--mirror".
        - Added branch graph image servlet based on EGit's branch graph renderer (issue-194)

diff --git a/src/main/java/com/gitblit/git/GitblitReceivePack.java b/src/main/java/com/gitblit/git/GitblitReceivePack.java
index 95d17fa5a626ab1ce3d00bf2780f8a26a65956ba..ebea2658242281fcff743cb37d54ddf8eed9c1d6 100644 (file)
--- a/src/main/java/com/gitblit/git/GitblitReceivePack.java
+++ b/src/main/java/com/gitblit/git/GitblitReceivePack.java
@@ -167,8 +167,11 @@ public class GitblitReceivePack extends ReceivePack implements PreReceiveHook, P
                if (repository.accessRestriction.atLeast(AccessRestrictionType.PUSH) && repository.verifyCommitter) {\r
                        // enforce committer verification\r
                        if (StringUtils.isEmpty(user.emailAddress)) {\r
-                               // emit warning if user does not have an email address\r
-                               LOGGER.warn(MessageFormat.format("Consider setting an email address for {0} ({1}) to improve committer verification.", user.getDisplayName(), user.username));\r
+                               // reject the push because the pushing account does not have an email address\r
+                               for (ReceiveCommand cmd : commands) {\r
+                                       sendRejection(cmd, "Sorry, the account \"{0}\" does not have an email address set for committer verification!", user.username);\r
+                               }\r
+                               return;\r
                        }\r
 \r
                        // Optionally enforce that the committer of first parent chain\r
@@ -201,16 +204,9 @@ public class GitblitReceivePack extends ReceivePack implements PreReceiveHook, P
 \r
                                                PersonIdent committer = commit.getCommitterIdent();\r
                                                if (!user.is(committer.getName(), committer.getEmailAddress())) {\r
-                                                       String reason;\r
-                                                       if (StringUtils.isEmpty(user.emailAddress)) {\r
-                                                               // account does not have an email address\r
-                                                               reason = MessageFormat.format("{0} by {1} <{2}> was not committed by {3} ({4})",\r
-                                                                               commit.getId().name(), committer.getName(), StringUtils.isEmpty(committer.getEmailAddress()) ? "?":committer.getEmailAddress(), user.getDisplayName(), user.username);\r
-                                                       } else {\r
-                                                               // account has an email address\r
-                                                               reason = MessageFormat.format("{0} by {1} <{2}> was not committed by {3} ({4}) <{5}>",\r
-                                                                               commit.getId().name(), committer.getName(), StringUtils.isEmpty(committer.getEmailAddress()) ? "?":committer.getEmailAddress(), user.getDisplayName(), user.username, user.emailAddress);\r
-                                                       }\r
+                                                       // verification failed\r
+                                                       String reason = MessageFormat.format("{0} by {1} <{2}> was not committed by {3} ({4}) <{5}>",\r
+                                                                       commit.getId().name(), committer.getName(), StringUtils.isEmpty(committer.getEmailAddress()) ? "?":committer.getEmailAddress(), user.getDisplayName(), user.username, user.emailAddress);\r
                                                        LOGGER.warn(reason);\r
                                                        cmd.setResult(Result.REJECTED_OTHER_REASON, reason);\r
                                                        allRejected &= true;\r

diff --git a/src/main/java/com/gitblit/models/UserModel.java b/src/main/java/com/gitblit/models/UserModel.java
index 446db3ab2d2cc0a3436a9202a1e13ecf1471c42b..0b59927fee851c867290c48a0172d2302404afa1 100644 (file)
--- a/src/main/java/com/gitblit/models/UserModel.java
+++ b/src/main/java/com/gitblit/models/UserModel.java
@@ -648,22 +648,19 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
         * @return true, if the name and email address match this account\r
         */\r
        public boolean is(String name, String email) {\r
-               // at a minimum a usename or display name must be supplied\r
-               if (StringUtils.isEmpty(name)) {\r
+               // at a minimum a username or display name AND email address must be supplied\r
+               if (StringUtils.isEmpty(name) || StringUtils.isEmpty(email)) {\r
                        return false;\r
                }\r
                boolean nameVerified = name.equalsIgnoreCase(username) || name.equalsIgnoreCase(getDisplayName());\r
                boolean emailVerified = false;\r
                if (StringUtils.isEmpty(emailAddress)) {\r
                        // user account has not specified an email address\r
-                       // rely on username/displayname verification\r
-                       emailVerified = true;\r
+                       // fail\r
+                       emailVerified = false;\r
                } else {\r
                        // user account has specified an email address\r
-                       // require email address verification\r
-                       if (!StringUtils.isEmpty(email)) {\r
-                               emailVerified = email.equalsIgnoreCase(emailAddress);\r
-                       }\r
+                       emailVerified = email.equalsIgnoreCase(emailAddress);\r
                }\r
                return nameVerified && emailVerified;\r
        }\r

diff --git a/src/site/administration.mkd b/src/site/administration.mkd
index dad4f414366f1308400143d1d7d0070fb5edd783..1a5aca1a91e9f9da19e9a657adcb355ec8770b5e 100644 (file)
--- a/src/site/administration.mkd
+++ b/src/site/administration.mkd
@@ -94,7 +94,7 @@ You may optionally enable committer verification which requires that each commit
 \r
 **How is this enforced?**\r
 \r
-Bob must set his *user.name* and *user.email* values for the repository to match his Gitblit user account **BEFORE** committing to his repository.\r
+Bob must properly set his *user.name* and *user.email* values for the repository to match his Gitblit user account **BEFORE** committing to his repository.\r
 \r
 ```\r
 [user "bob"]\r
@@ -109,7 +109,7 @@ or
     git config user.name bob\r
     git config user.email bob@somewhere.com    \r
 \r
-If the Gitblit account does not specify an email address, then the committer email address is ignored.  However, if the account does specify an address it must match the committer's email address.  Display name or username can be used as the committer name.\r
+The committer email address is required to be identical.  Display name or username can be used as the committer name.\r
 \r
 All checks are case-insensitive.\r
 \r

diff --git a/src/test/java/com/gitblit/tests/GitBlitTest.java b/src/test/java/com/gitblit/tests/GitBlitTest.java
index c01862feb9e4a51241eab9cd244a49c77c0ef0bb..1f81b8756d99b9e2ada9cd57aa5f7564a84789f0 100644 (file)
--- a/src/test/java/com/gitblit/tests/GitBlitTest.java
+++ b/src/test/java/com/gitblit/tests/GitBlitTest.java
@@ -70,13 +70,13 @@ public class GitBlitTest extends GitblitUnitTest {
                UserModel user = new UserModel("james");\r
                user.displayName = "James Moger";\r
 \r
-               assertTrue(user.is("James", null));\r
-               assertTrue(user.is("James", ""));\r
-               assertTrue(user.is("JaMeS", "anything"));\r
+               assertFalse(user.is("James", null));\r
+               assertFalse(user.is("James", ""));\r
+               assertFalse(user.is("JaMeS", "anything"));\r
 \r
-               assertTrue(user.is("james moger", null));\r
-               assertTrue(user.is("james moger", ""));\r
-               assertTrue(user.is("james moger", "anything"));\r
+               assertFalse(user.is("james moger", null));\r
+               assertFalse(user.is("james moger", ""));\r
+               assertFalse(user.is("james moger", "anything"));\r
 \r
                assertFalse(user.is("joe", null));\r
                assertFalse(user.is("joe", ""));\r

diff --git a/src/test/java/com/gitblit/tests/GitServletTest.java b/src/test/java/com/gitblit/tests/GitServletTest.java
index bc392885437a348bbd5c3a30851e4a28ec605533..ad61a6769fddaf03744624b813aaca4430e7f77c 100644 (file)
--- a/src/test/java/com/gitblit/tests/GitServletTest.java
+++ b/src/test/java/com/gitblit/tests/GitServletTest.java
@@ -380,27 +380,15 @@ public class GitServletTest extends GitblitUnitTest {
        public void testCommitterVerification() throws Exception {\r
                UserModel user = getUser();\r
 \r
-               // account only uses account name to verify\r
-               testCommitterVerification(user, user.username, null, true);\r
-               // committer email address is ignored because account does not specify email\r
-               testCommitterVerification(user, user.username, "something", true);\r
-               // completely different committer\r
                testCommitterVerification(user, "joe", null, false);\r
+               testCommitterVerification(user, "joe", user.emailAddress, false);\r
+               testCommitterVerification(user, user.username, null, false);\r
+               testCommitterVerification(user, user.username, user.emailAddress, true);\r
 \r
-               // test display name verification\r
                user.displayName = "James Moger";\r
-               testCommitterVerification(user, user.displayName, null, true);\r
-               testCommitterVerification(user, user.displayName, "something", true);\r
-               testCommitterVerification(user, "joe", null, false);\r
-\r
-               // test email address verification\r
-               user.emailAddress = "something";\r
                testCommitterVerification(user, user.displayName, null, false);\r
-               testCommitterVerification(user, user.displayName, "somethingelse", false);\r
+               testCommitterVerification(user, user.displayName, "something", false);\r
                testCommitterVerification(user, user.displayName, user.emailAddress, true);\r
-\r
-               // use same email address but with different committer\r
-               testCommitterVerification(user, "joe", "somethingelse", false);\r
        }\r
 \r
        private void testCommitterVerification(UserModel user, String displayName, String emailAddress, boolean expectedSuccess) throws Exception {\r