added option to disallow non fast-forward git pushes

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitConfig.java

@@ -51,9 +51,18 @@ public class GitConfig extends SimpleRepositoryConfig {
   @XmlElement(name = "gc-expression")
   private String gcExpression;
 
-  public String getGcExpression()
-  {
+  @XmlElement(name = "disallow-non-fast-forward")
+  private boolean nonFastForwardDisallowed;
+
+  public String getGcExpression() {
     return gcExpression;
   }
 
+  public boolean isNonFastForwardDisallowed() {
+    return nonFastForwardDisallowed;
+  }
+
+  public void setNonFastForwardDisallowed(boolean nonFastForwardDisallowed) {
+    this.nonFastForwardDisallowed = nonFastForwardDisallowed;
+  }
 }

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java

@@ -35,79 +35,63 @@ package sonia.scm.web;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.inject.Inject;
-
 import org.eclipse.jgit.http.server.resolver.DefaultReceivePackFactory;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.ReceivePack;
 import org.eclipse.jgit.transport.resolver.ReceivePackFactory;
 import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
-
 import sonia.scm.repository.GitRepositoryHandler;
 import sonia.scm.repository.spi.HookEventFacade;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import javax.servlet.http.HttpServletRequest;
 
+//~--- JDK imports ------------------------------------------------------------
+
 /**
+ * GitReceivePackFactory creates {@link ReceivePack} objects and assigns the required
+ * Hook components.
  *
  * @author Sebastian Sdorra
  */
-public class GitReceivePackFactory
-  implements ReceivePackFactory<HttpServletRequest>
+public class GitReceivePackFactory implements ReceivePackFactory<HttpServletRequest>
 {
 
-  /**
-   * Constructs ...
-   *
-   *
-   *
-   * @param hookEventFacade
-   * @param handler
-   */
+  private final GitRepositoryHandler handler;
+
+  private ReceivePackFactory wrapped;
+
+  private final GitReceiveHook hook;
+
   @Inject
-  public GitReceivePackFactory(HookEventFacade hookEventFacade,
-    GitRepositoryHandler handler)
-  {
-    hook = new GitReceiveHook(hookEventFacade, handler);
+  public GitReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) {
+    this.handler = handler;
+    this.hook = new GitReceiveHook(hookEventFacade, handler);
+    this.wrapped =  new DefaultReceivePackFactory();
   }
 
-  //~--- methods --------------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param request
-   * @param repository
-   *
-   * @return
-   *
-   * @throws ServiceNotAuthorizedException
-   * @throws ServiceNotEnabledException
-   */
   @Override
   public ReceivePack create(HttpServletRequest request, Repository repository)
-    throws ServiceNotEnabledException, ServiceNotAuthorizedException
-  {
-    ReceivePack rpack = defaultFactory.create(request, repository);
+    throws ServiceNotEnabledException, ServiceNotAuthorizedException {
+    ReceivePack receivePack = wrapped.create(request, repository);
+    receivePack.setAllowNonFastForwards(isNonFastForwardAllowed());
 
-    rpack.setPreReceiveHook(hook);
-    rpack.setPostReceiveHook(hook);
+    receivePack.setPreReceiveHook(hook);
+    receivePack.setPostReceiveHook(hook);
     // apply collecting listener, to be able to check which commits are new
-    CollectingPackParserListener.set(rpack);
+    CollectingPackParserListener.set(receivePack);
     
-    return rpack;
+    return receivePack;
   }
 
-  //~--- fields ---------------------------------------------------------------
+  private boolean isNonFastForwardAllowed() {
+    return ! handler.getConfig().isNonFastForwardDisallowed();
+  }
 
-  /** Field description */
-  private DefaultReceivePackFactory defaultFactory =
-    new DefaultReceivePackFactory();
-
-  /** Field description */
-  private GitReceiveHook hook;
+  @VisibleForTesting
+  void setWrapped(ReceivePackFactory wrapped) {
+    this.wrapped = wrapped;
+  }
 }

scm-plugins/scm-git-plugin/src/main/resources/sonia/scm/git.config.js

@@ -37,6 +37,7 @@ Sonia.git.ConfigPanel = Ext.extend(Sonia.config.SimpleConfigForm, {
   titleText: 'Git Settings',
   repositoryDirectoryText: 'Repository directory',
   gcExpressionText: 'Git GC Cron Expression',
+  disallowNonFastForwardText: 'Disallow Non Fast-Forward',
   disabledText: 'Disabled',
 
   // helpTexts
@@ -53,6 +54,8 @@ Sonia.git.ConfigPanel = Ext.extend(Sonia.config.SimpleConfigForm, {
                          </table>\n\
                          <p>E.g.: To run the task on every sunday at two o\'clock in the morning: 0 0 2 ? * SUN</p>\n\
                          <p>For more informations please have a look at <a href="http://www.quartz-scheduler.org/documentation/quartz-2.2.x/tutorials/crontrigger.html">Quartz CronTrigger</a></p>',
+  // TODO i18n
+  disallowNonFastForwardHelpText: 'Reject git pushes which are non fast-forward such as --force.',
   disabledHelpText: 'Enable or disable the Git plugin.\n\
                     Note you have to reload the page, after changing this value.',
 
@@ -73,6 +76,12 @@ Sonia.git.ConfigPanel = Ext.extend(Sonia.config.SimpleConfigForm, {
         fieldLabel: this.gcExpressionText,
         helpText: this.gcExpressionHelpText,
         allowBlank : true        
+      },{
+        xtype: 'checkbox',
+        name: 'disallow-non-fast-forward',
+        fieldLabel: this.disallowNonFastForwardText,
+        inputValue: 'true',
+        helpText: this.disallowNonFastForwardHelpText
       },{
         xtype: 'checkbox',
         name: 'disabled',

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitReceivePackFactoryTest.java

@@ -0,0 +1,117 @@
+/**
+ * Copyright (c) 2010, Sebastian Sdorra
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ * 3. Neither the name of SCM-Manager; nor the names of its
+ *    contributors may be used to endorse or promote products derived from this
+ *    software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * http://bitbucket.org/sdorra/scm-manager
+ *
+ */
+
+package sonia.scm.web;
+
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ReceivePack;
+import org.eclipse.jgit.transport.resolver.ReceivePackFactory;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import sonia.scm.repository.GitConfig;
+import sonia.scm.repository.GitRepositoryHandler;
+
+import javax.servlet.http.HttpServletRequest;
+import java.io.File;
+import java.io.IOException;
+
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.when;
+
+
+/**
+ * Unit tests for {@link GitReceivePackFactory}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class GitReceivePackFactoryTest {
+
+  @Mock
+  private GitRepositoryHandler handler;
+
+  private GitConfig config;
+
+  @Mock
+  private ReceivePackFactory wrappedReceivePackFactory;
+
+  private GitReceivePackFactory factory;
+
+  @Mock
+  private HttpServletRequest request;
+
+  private Repository repository;
+
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpObjectUnderTest() throws Exception {
+    this.repository = createRepositoryForTesting();
+
+    config = new GitConfig();
+    when(handler.getConfig()).thenReturn(config);
+
+    ReceivePack receivePack = new ReceivePack(repository);
+    when(wrappedReceivePackFactory.create(request, repository)).thenReturn(receivePack);
+
+    factory = new GitReceivePackFactory(handler, null);
+    factory.setWrapped(wrappedReceivePackFactory);
+  }
+
+  private Repository createRepositoryForTesting() throws GitAPIException, IOException {
+    File directory = temporaryFolder.newFolder();
+    return Git.init().setDirectory(directory).call().getRepository();
+  }
+
+  @Test
+  public void testCreate() throws Exception {
+    ReceivePack receivePack = factory.create(request, repository);
+    assertThat(receivePack.getPackParserListener(), instanceOf(CollectingPackParserListener.class));
+    assertThat(receivePack.getPreReceiveHook(), instanceOf(GitReceiveHook.class));
+    assertThat(receivePack.getPostReceiveHook(), instanceOf(GitReceiveHook.class));
+    assertTrue(receivePack.isAllowNonFastForwards());
+  }
+
+  @Test
+  public void testCreateWithDisabledNonFastForward() throws Exception {
+    config.setNonFastForwardDisallowed(true);
+    ReceivePack receivePack = factory.create(request, repository);
+    assertFalse(receivePack.isAllowNonFastForwards());
+  }
+
+}

scm-webapp/src/test/java/sonia/scm/it/GitNonFastForwardITCase.java

@@ -0,0 +1,222 @@
+/**
+ * Copyright (c) 2010, Sebastian Sdorra
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ * 3. Neither the name of SCM-Manager; nor the names of its
+ *    contributors may be used to endorse or promote products derived from this
+ *    software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * http://bitbucket.org/sdorra/scm-manager
+ *
+ */
+
+
+package sonia.scm.it;
+
+import com.google.common.base.Charsets;
+import com.google.common.io.Files;
+import com.sun.jersey.api.client.Client;
+import com.sun.jersey.api.client.ClientResponse;
+import com.sun.jersey.api.client.WebResource;
+import org.eclipse.jgit.api.CommitCommand;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
+import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.junit.*;
+import org.junit.rules.TemporaryFolder;
+import sonia.scm.repository.GitConfig;
+import sonia.scm.repository.Repository;
+import sonia.scm.repository.RepositoryTestData;
+import sonia.scm.repository.client.api.RepositoryClientFactory;
+
+import java.io.File;
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static sonia.scm.it.IntegrationTestUtil.*;
+import static sonia.scm.it.RepositoryITUtil.createRepository;
+import static sonia.scm.it.RepositoryITUtil.deleteRepository;
+
+/**
+ * Integration Tests for Git with non fast-forward pushes.
+ */
+public class GitNonFastForwardITCase {
+
+  private static final RepositoryClientFactory REPOSITORY_CLIENT_FACTORY = new RepositoryClientFactory();
+
+  private Client apiClient;
+  private Repository repository;
+  private File workingCopy;
+  private Git git;
+
+  @Rule
+  public TemporaryFolder tempFolder = new TemporaryFolder();
+
+  @Before
+  public void createAndCloneTestRepository() throws IOException, GitAPIException {
+    apiClient = createAdminClient();
+    Repository testRepository = RepositoryTestData.createHeartOfGold("git");
+    this.repository = createRepository(apiClient, testRepository);
+    this.workingCopy = tempFolder.newFolder();
+
+    String url = repository.createUrl(BASE_URL);
+    this.git = clone(url);
+  }
+
+  @After
+  public void removeTestRepository() {
+    deleteRepository(apiClient, repository.getId());
+    apiClient.destroy();
+  }
+
+  /**
+   * Ensures that the normal behaviour (non fast-forward is allowed), is restored after the tests are executed.
+   */
+  @AfterClass
+  public static void allowNonFastForward() {
+    setNonFastForwardDisallowed(false);
+  }
+
+  @Test
+  public void testGitPushAmendWithoutForce() throws IOException, GitAPIException {
+    setNonFastForwardDisallowed(false);
+
+    addTestFileToWorkingCopyAndCommit("a");
+    pushAndAssert(false, Status.OK);
+
+    addTestFileToWorkingCopyAndCommitAmend("c");
+    pushAndAssert(false, Status.REJECTED_NONFASTFORWARD);
+  }
+
+  @Test
+  public void testGitPushAmendWithForce() throws IOException, GitAPIException {
+    setNonFastForwardDisallowed(false);
+
+    addTestFileToWorkingCopyAndCommit("a");
+    pushAndAssert(false, Status.OK);
+
+    addTestFileToWorkingCopyAndCommitAmend("c");
+    pushAndAssert(true, Status.OK);
+  }
+
+  @Test
+  public void testGitPushAmendForceWithDisallowNonFastForward() throws GitAPIException, IOException {
+    setNonFastForwardDisallowed(true);
+
+    addTestFileToWorkingCopyAndCommit("a");
+    pushAndAssert(false, Status.OK);
+
+    addTestFileToWorkingCopyAndCommitAmend("c");
+    pushAndAssert(true, Status.REJECTED_OTHER_REASON);
+
+    setNonFastForwardDisallowed(false);
+  }
+
+  private CredentialsProvider createCredentialProvider() {
+    return new UsernamePasswordCredentialsProvider(
+      IntegrationTestUtil.ADMIN_USERNAME, IntegrationTestUtil.ADMIN_PASSWORD
+    );
+  }
+
+  private Git clone(String url) throws GitAPIException {
+    return Git.cloneRepository()
+      .setDirectory(workingCopy)
+      .setURI(url)
+      .setCredentialsProvider(createCredentialProvider())
+      .call();
+  }
+
+  private void addTestFileToWorkingCopyAndCommit(String name) throws IOException, GitAPIException {
+    addTestFile(name);
+    prepareCommit()
+      .setMessage("added ".concat(name))
+      .call();
+  }
+
+  private void addTestFile(String name) throws IOException, GitAPIException {
+    String filename = name.concat(".txt");
+    Files.write(name, new File(workingCopy, filename), Charsets.UTF_8);
+    git.add().addFilepattern(filename).call();
+  }
+
+  private CommitCommand prepareCommit() {
+    return git.commit()
+      .setAuthor(IntegrationTestUtil.AUTHOR.getName(), IntegrationTestUtil.AUTHOR.getMail());
+  }
+
+  private void pushAndAssert(boolean force, Status expectedStatus) throws GitAPIException {
+    Iterable<PushResult> results = push(force);
+    assertStatus(results, expectedStatus);
+  }
+
+  private Iterable<PushResult> push(boolean force) throws GitAPIException {
+    return git.push()
+      .setRemote("origin")
+      .add("master")
+      .setForce(force)
+      .setCredentialsProvider(createCredentialProvider())
+      .call();
+  }
+
+  private void assertStatus(Iterable<PushResult> results, Status expectedStatus) {
+    for ( PushResult pushResult : results ) {
+      assertStatus(pushResult, expectedStatus);
+    }
+  }
+
+  private void assertStatus(PushResult pushResult, Status expectedStatus) {
+    for ( RemoteRefUpdate remoteRefUpdate : pushResult.getRemoteUpdates() ) {
+      assertEquals(expectedStatus, remoteRefUpdate.getStatus());
+    }
+  }
+
+  private void addTestFileToWorkingCopyAndCommitAmend(String name) throws IOException, GitAPIException {
+    addTestFile(name);
+    prepareCommit()
+      .setMessage("amend commit, because of missing ".concat(name))
+      .setAmend(true)
+      .call();
+  }
+
+  private static void setNonFastForwardDisallowed(boolean nonFastForwardDisallowed) {
+    Client adminClient = createAdminClient();
+    try {
+      WebResource resource = createResource(adminClient, "config/repositories/git");
+      GitConfig config = resource.get(GitConfig.class);
+
+      assertNotNull(config);
+      config.setNonFastForwardDisallowed(nonFastForwardDisallowed);
+
+      ClientResponse response = resource.post(ClientResponse.class, config);
+      assertNotNull(response);
+      assertEquals(201, response.getStatus());
+    } finally {
+      adminClient.destroy();
+    }
+  }
+
+}