From b799d545f37f7123aaa1ee1d0ff3b61f1f3cc8c2 Mon Sep 17 00:00:00 2001 From: David Ostrovsky <david@ostrovsky.org> Date: Thu, 10 Apr 2014 18:58:08 -0400 Subject: [PATCH] Add review SSH command --- src/main/java/com/gitblit/git/GitblitReceivePack.java | 76 ++++++++++++++++++++++++++++++-------- 1 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/gitblit/git/GitblitReceivePack.java b/src/main/java/com/gitblit/git/GitblitReceivePack.java index e5c0ea4..73da3d3 100644 --- a/src/main/java/com/gitblit/git/GitblitReceivePack.java +++ b/src/main/java/com/gitblit/git/GitblitReceivePack.java @@ -50,6 +50,7 @@ import com.gitblit.manager.IGitblit; import com.gitblit.models.RepositoryModel; import com.gitblit.models.UserModel; +import com.gitblit.tickets.BranchTicketService; import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.ClientLogger; import com.gitblit.utils.CommitCache; @@ -85,13 +86,11 @@ protected String gitblitUrl; - protected String repositoryUrl; - protected GroovyScriptEngine gse; - private final IStoredSettings settings; + protected final IStoredSettings settings; - private final IGitblit gitblit; + protected final IGitblit gitblit; public GitblitReceivePack( IGitblit gitblit, @@ -122,6 +121,32 @@ // setup pre and post receive hook setPreReceiveHook(this); setPostReceiveHook(this); + } + + /** + * Returns true if the user is permitted to apply the receive commands to + * the repository. + * + * @param commands + * @return true if the user may push these commands + */ + protected boolean canPush(Collection<ReceiveCommand> commands) { + // TODO Consider supporting branch permissions here (issue-36) + // Not sure if that should be Gerrit-style, refs/meta/config, or + // gitolite-style, permissions in users.conf + // + // How could commands be empty? + // + // Because a subclass, like PatchsetReceivePack, filters receive + // commands before this method is called. This makes it possible for + // this method to test an empty list. In this case, we assume that the + // subclass receive pack properly enforces push restrictions. for the + // ref. + // + // The empty test is not explicitly required, it's written here to + // clarify special-case behavior. + + return commands.isEmpty() ? true : user.canPush(repository); } /** @@ -156,7 +181,7 @@ return; } - if (!user.canPush(repository)) { + if (!canPush(commands)) { // user does not have push permissions for (ReceiveCommand cmd : commands) { sendRejection(cmd, "User \"{0}\" does not have push permissions for \"{1}\"!", user.username, repository.name); @@ -226,18 +251,28 @@ } } - // reset branch commit cache on REWIND and DELETE for (ReceiveCommand cmd : commands) { String ref = cmd.getRefName(); if (ref.startsWith(Constants.R_HEADS)) { switch (cmd.getType()) { case UPDATE_NONFASTFORWARD: case DELETE: + // reset branch commit cache on REWIND and DELETE CommitCache.instance().clear(repository.name, ref); break; default: break; } + } else if (ref.equals(BranchTicketService.BRANCH)) { + // ensure pushing user is an administrator OR an owner + // i.e. prevent ticket tampering + boolean permitted = user.canAdmin() || repository.isOwner(user.username); + if (!permitted) { + sendRejection(cmd, "{0} is not permitted to push to {1}", user.username, ref); + } + } else if (ref.startsWith(Constants.R_FOR)) { + // prevent accidental push to refs/for + sendRejection(cmd, "{0} is not configured to receive patchsets", repository.name); } } @@ -335,6 +370,15 @@ LOGGER.error(MessageFormat.format("Failed to update {0} pushlog", repository.name), e); } + // check for updates pushed to the BranchTicketService branch + // if the BranchTicketService is active it will reindex, as appropriate + for (ReceiveCommand cmd : commands) { + if (Result.OK.equals(cmd.getResult()) + && BranchTicketService.BRANCH.equals(cmd.getRefName())) { + rp.getRepository().fireEvent(new ReceiveCommandEvent(repository, cmd)); + } + } + // run Groovy hook scripts Set<String> scripts = new LinkedHashSet<String>(); scripts.addAll(gitblit.getPostReceiveScriptsInherited(repository)); @@ -390,10 +434,6 @@ this.gitblitUrl = url; } - protected void setRepositoryUrl(String url) { - this.repositoryUrl = url; - } - protected void sendRejection(final ReceiveCommand cmd, final String why, Object... objects) { String text; if (ArrayUtils.isEmpty(objects)) { @@ -406,14 +446,14 @@ } protected void sendHeader(String msg, Object... objects) { - sendMessage("--->", msg, objects); + sendInfo("--> ", msg, objects); } - protected void sendMessage(String msg, Object... objects) { - sendMessage(" ", msg, objects); + protected void sendInfo(String msg, Object... objects) { + sendInfo(" ", msg, objects); } - protected void sendMessage(String prefix, String msg, Object... objects) { + protected void sendInfo(String prefix, String msg, Object... objects) { String text; if (ArrayUtils.isEmpty(objects)) { text = msg; @@ -422,7 +462,9 @@ text = MessageFormat.format(msg, objects); super.sendMessage(prefix + text); } - LOGGER.info(text + " (" + user.username + ")"); + if (!StringUtils.isEmpty(msg)) { + LOGGER.info(text + " (" + user.username + ")"); + } } protected void sendError(String msg, Object... objects) { @@ -434,7 +476,9 @@ text = MessageFormat.format(msg, objects); super.sendError(text); } - LOGGER.error(text + " (" + user.username + ")"); + if (!StringUtils.isEmpty(msg)) { + LOGGER.error(text + " (" + user.username + ")"); + } } /** -- Gitblit v1.9.1