From 15640f86032169ad9bfef17c387b94f30a61582f Mon Sep 17 00:00:00 2001 From: James Moger Date: Thu, 11 Oct 2012 18:11:50 -0400 Subject: Experimental committer verification --- src/com/gitblit/GitBlit.java | 2 + src/com/gitblit/GitServlet.java | 44 +++++++++++++++++++++- src/com/gitblit/models/RepositoryModel.java | 1 + src/com/gitblit/models/UserModel.java | 28 ++++++++++++++ src/com/gitblit/wicket/GitBlitWebApp.properties | 4 +- .../gitblit/wicket/pages/EditRepositoryPage.html | 3 +- .../gitblit/wicket/pages/EditRepositoryPage.java | 4 +- 7 files changed, 82 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/com/gitblit/GitBlit.java b/src/com/gitblit/GitBlit.java index bf9b1ed1..36f5f35c 100644 --- a/src/com/gitblit/GitBlit.java +++ b/src/com/gitblit/GitBlit.java @@ -1257,6 +1257,7 @@ public class GitBlit implements ServletContextListener { "accessRestriction", settings.getString(Keys.git.defaultAccessRestriction, null))); model.authorizationControl = AuthorizationControl.fromName(getConfig(config, "authorizationControl", settings.getString(Keys.git.defaultAuthorizationControl, null))); + model.verifyCommitter = getConfig(config, "verifyCommitter", false); model.showRemoteBranches = getConfig(config, "showRemoteBranches", hasOrigin); model.isFrozen = getConfig(config, "isFrozen", false); model.showReadme = getConfig(config, "showReadme", false); @@ -1698,6 +1699,7 @@ public class GitBlit implements ServletContextListener { config.setBoolean(Constants.CONFIG_GITBLIT, null, "allowForks", repository.allowForks); config.setString(Constants.CONFIG_GITBLIT, null, "accessRestriction", repository.accessRestriction.name()); config.setString(Constants.CONFIG_GITBLIT, null, "authorizationControl", repository.authorizationControl.name()); + config.setBoolean(Constants.CONFIG_GITBLIT, null, "verifyCommitter", repository.verifyCommitter); config.setBoolean(Constants.CONFIG_GITBLIT, null, "showRemoteBranches", repository.showRemoteBranches); config.setBoolean(Constants.CONFIG_GITBLIT, null, "isFrozen", repository.isFrozen); config.setBoolean(Constants.CONFIG_GITBLIT, null, "showReadme", repository.showReadme); diff --git a/src/com/gitblit/GitServlet.java b/src/com/gitblit/GitServlet.java index 8e2326d4..42d88c91 100644 --- a/src/com/gitblit/GitServlet.java +++ b/src/com/gitblit/GitServlet.java @@ -28,6 +28,7 @@ import java.text.MessageFormat; import java.util.Collection; import java.util.Enumeration; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; import javax.servlet.ServletConfig; @@ -38,6 +39,7 @@ import javax.servlet.http.HttpServletRequest; import org.eclipse.jgit.http.server.resolver.DefaultReceivePackFactory; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.PostReceiveHook; import org.eclipse.jgit.transport.PreReceiveHook; import org.eclipse.jgit.transport.ReceiveCommand; @@ -48,10 +50,12 @@ import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.gitblit.Constants.AccessRestrictionType; import com.gitblit.models.RepositoryModel; import com.gitblit.models.UserModel; import com.gitblit.utils.ClientLogger; import com.gitblit.utils.HttpUtils; +import com.gitblit.utils.JGitUtils; import com.gitblit.utils.StringUtils; /** @@ -191,10 +195,48 @@ public class GitServlet extends org.eclipse.jgit.http.server.GitServlet { @Override public void onPreReceive(ReceivePack rp, Collection commands) { RepositoryModel repository = GitBlit.self().getRepositoryModel(repositoryName); + UserModel user = getUserModel(rp); + + if (repository.accessRestriction.atLeast(AccessRestrictionType.PUSH) && repository.verifyCommitter) { + if (StringUtils.isEmpty(user.emailAddress)) { + // emit warning if user does not have an email address + logger.warn(MessageFormat.format("Consider setting an email address for {0} ({1}) to improve committer verification.", user.getDisplayName(), user.username)); + } + + // Optionally enforce that the committer of the left parent chain + // match the account being used to push the commits. + // + // This requires all merge commits are executed with the "--no-ff" + // option to force a merge commit even if fast-forward is possible. + // This ensures that the chain of left parents has the commit + // identity of the merging user. + for (ReceiveCommand cmd : commands) { + try { + List commits = JGitUtils.getRevLog(rp.getRepository(), cmd.getOldId().name(), cmd.getNewId().name()); + for (RevCommit commit : commits) { + PersonIdent committer = commit.getCommitterIdent(); + if (!user.is(committer.getName(), committer.getEmailAddress())) { + String reason; + if (StringUtils.isEmpty(user.emailAddress)) { + // account does not have en email address + reason = MessageFormat.format("{0} by {1} <{2}> was not committed by {3} ({4})", commit.getId().name(), committer.getName(), StringUtils.isEmpty(committer.getEmailAddress()) ? "?":committer.getEmailAddress(), user.getDisplayName(), user.username); + } else { + // account has an email address + reason = MessageFormat.format("{0} by {1} <{2}> was not committed by {3} ({4}) <{5}>", commit.getId().name(), committer.getName(), StringUtils.isEmpty(committer.getEmailAddress()) ? "?":committer.getEmailAddress(), user.getDisplayName(), user.username, user.emailAddress); + } + cmd.setResult(Result.REJECTED_OTHER_REASON, reason); + break; + } + } + } catch (Exception e) { + logger.error("Failed to verify commits were made by pushing user", e); + } + } + } + Set scripts = new LinkedHashSet(); scripts.addAll(GitBlit.self().getPreReceiveScriptsInherited(repository)); scripts.addAll(repository.preReceiveScripts); - UserModel user = getUserModel(rp); runGroovy(repository, user, commands, rp, scripts); for (ReceiveCommand cmd : commands) { if (!Result.NOT_ATTEMPTED.equals(cmd.getResult())) { diff --git a/src/com/gitblit/models/RepositoryModel.java b/src/com/gitblit/models/RepositoryModel.java index 914523df..502f886e 100644 --- a/src/com/gitblit/models/RepositoryModel.java +++ b/src/com/gitblit/models/RepositoryModel.java @@ -75,6 +75,7 @@ public class RepositoryModel implements Serializable, Comparable forks; public String originRepository; + public boolean verifyCommitter; public RepositoryModel() { this("", "", "", new Date(0)); diff --git a/src/com/gitblit/models/UserModel.java b/src/com/gitblit/models/UserModel.java index 6fe8df2b..8b3fe826 100644 --- a/src/com/gitblit/models/UserModel.java +++ b/src/com/gitblit/models/UserModel.java @@ -405,4 +405,32 @@ public class UserModel implements Principal, Serializable, Comparable public int compareTo(UserModel o) { return username.compareTo(o.username); } + + /** + * Returns true if the name/email pair match this user account. + * + * @param name + * @param email + * @return true, if the name and email address match this account + */ + public boolean is(String name, String email) { + // at a minimum a usename or display name must be supplied + if (StringUtils.isEmpty(name)) { + return false; + } + boolean nameVerified = name.equalsIgnoreCase(username) || name.equalsIgnoreCase(getDisplayName()); + boolean emailVerified = false; + if (StringUtils.isEmpty(emailAddress)) { + // user account has not specified an email address + // rely on username/displayname verification + emailVerified = true; + } else { + // user account has specified an email address + // require email address verification + if (!StringUtils.isEmpty(email)) { + emailVerified = email.equalsIgnoreCase(emailAddress); + } + } + return nameVerified && emailVerified; + } } diff --git a/src/com/gitblit/wicket/GitBlitWebApp.properties b/src/com/gitblit/wicket/GitBlitWebApp.properties index d18dcf11..e786d311 100644 --- a/src/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/com/gitblit/wicket/GitBlitWebApp.properties @@ -341,4 +341,6 @@ gb.preparingFork = preparing your fork... gb.isFork = is fork gb.canCreate = can create gb.canCreateDescription = can create personal repositories -gb.illegalPersonalRepositoryLocation = your personal repository must be located at \"{0}\" \ No newline at end of file +gb.illegalPersonalRepositoryLocation = your personal repository must be located at \"{0}\" +gb.verifyCommitter = verify committer +gb.verifyCommitterDescription = require committer identity to match pushing Gitblt user account (all merges require "--no-ff" to enforce committer identity) \ No newline at end of file diff --git a/src/com/gitblit/wicket/pages/EditRepositoryPage.html b/src/com/gitblit/wicket/pages/EditRepositoryPage.html index 5869917f..20b77e52 100644 --- a/src/com/gitblit/wicket/pages/EditRepositoryPage.html +++ b/src/com/gitblit/wicket/pages/EditRepositoryPage.html @@ -35,11 +35,12 @@
+

 

-

 

diff --git a/src/com/gitblit/wicket/pages/EditRepositoryPage.java b/src/com/gitblit/wicket/pages/EditRepositoryPage.java index 8176c28b..a2dad104 100644 --- a/src/com/gitblit/wicket/pages/EditRepositoryPage.java +++ b/src/com/gitblit/wicket/pages/EditRepositoryPage.java @@ -406,7 +406,7 @@ public class EditRepositoryPage extends RootSubPage { form.add(new DropDownChoice("federationStrategy", federationStrategies, new FederationTypeRenderer())); form.add(new CheckBox("useTickets")); - form.add(new CheckBox("useDocs")); + form.add(new CheckBox("useDocs")); form.add(new CheckBox("showRemoteBranches")); form.add(new CheckBox("showReadme")); form.add(new CheckBox("skipSizeCalculation")); @@ -423,6 +423,8 @@ public class EditRepositoryPage extends RootSubPage { group.add(allowNamed); form.add(group); + form.add(new CheckBox("verifyCommitter")); + form.add(usersPalette); form.add(teamsPalette); form.add(federationSetsPalette); -- cgit v1.2.3