Add repository-specific non-ff disallowed option (#1579)

Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>
2025-12-21 15:59:48 +01:00 · 2021-03-11 10:13:26 +01:00
parent 831877564d
commit 0d3339b0cb
18 changed files with 142 additions and 64 deletions
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitRepositoryConfigDto.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitRepositoryConfigDto.java
@@ -40,6 +40,8 @@ public class GitRepositoryConfigDto extends HalRepresentation implements UpdateG

  private String defaultBranch;

+  private boolean nonFastForwardDisallowed;
+
  @Override
  @SuppressWarnings("squid:S1185") // We want to have this method available in this package
  protected HalRepresentation add(Links links) {
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitRepositoryConfigResource.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitRepositoryConfigResource.java
@@ -119,7 +119,7 @@ public class GitRepositoryConfigResource {
        schema = @Schema(implementation = UpdateGitRepositoryConfigDto.class),
        examples = @ExampleObject(
          name = "Overwrites current configuration with this one.",
-          value = "{\n  \"defaultBranch\":\"main\"\n}",
+          value = "{\n  \"defaultBranch\":\"main\"\n  \"nonFastForwardDisallowed\":false,\n}",
          summary = "Simple update configuration"
        )
      )
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitRepositoryConfigStoreProvider.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/GitRepositoryConfigStoreProvider.java
@@ -21,7 +21,7 @@
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 * SOFTWARE.
 */
-    
+
 package sonia.scm.api.v2.resources;

 import sonia.scm.event.ScmEventBus;
@@ -42,7 +42,22 @@ public class GitRepositoryConfigStoreProvider {
  }

  public ConfigurationStore<GitRepositoryConfig> get(Repository repository) {
-    return new StoreWrapper(configurationStoreFactory.withType(GitRepositoryConfig.class).withName("gitConfig").forRepository(repository).build(), repository);
+    return new StoreWrapper(createStore(repository.getId()), repository);
+  }
+
+  public GitRepositoryConfig getGitRepositoryConfig(String repositoryId) {
+    return getFronStore(createStore(repositoryId));
+  }
+
+  private static GitRepositoryConfig getFronStore(ConfigurationStore<GitRepositoryConfig> store) {
+    return store.getOptional().orElse(new GitRepositoryConfig());
+  }
+
+  private ConfigurationStore<GitRepositoryConfig> createStore(String id) {
+    return configurationStoreFactory
+      .withType(GitRepositoryConfig.class)
+      .withName("gitConfig")
+      .forRepository(id).build();
  }

  private static class StoreWrapper implements ConfigurationStore<GitRepositoryConfig> {
@@ -57,11 +72,7 @@ public class GitRepositoryConfigStoreProvider {

    @Override
    public GitRepositoryConfig get() {
-      GitRepositoryConfig config = delegate.get();
-      if (config == null) {
-        return new GitRepositoryConfig();
-      }
-      return config;
+      return getFronStore(delegate);
    }

    @Override
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/UpdateGitRepositoryConfigDto.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/api/v2/resources/UpdateGitRepositoryConfigDto.java
@@ -26,4 +26,5 @@ package sonia.scm.api.v2.resources;

 interface UpdateGitRepositoryConfigDto {
  String getDefaultBranch();
+  boolean isNonFastForwardDisallowed();
 }
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java
@@ -29,7 +29,9 @@ 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.api.v2.resources.GitRepositoryConfigStoreProvider;
 import sonia.scm.repository.GitChangesetConverterFactory;
+import sonia.scm.repository.GitRepositoryConfig;
 import sonia.scm.repository.GitRepositoryHandler;
 import sonia.scm.web.CollectingPackParserListener;
 import sonia.scm.web.GitHookEventFacade;
@@ -39,16 +41,21 @@ public abstract class BaseReceivePackFactory<T> implements ReceivePackFactory<T>

  private final GitRepositoryHandler handler;
  private final GitReceiveHook hook;
+  private final GitRepositoryConfigStoreProvider storeProvider;

-  protected BaseReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, GitHookEventFacade hookEventFacade) {
+  protected BaseReceivePackFactory(GitChangesetConverterFactory converterFactory,
+                                   GitRepositoryHandler handler,
+                                   GitHookEventFacade hookEventFacade,
+                                   GitRepositoryConfigStoreProvider storeProvider) {
    this.handler = handler;
+    this.storeProvider = storeProvider;
    this.hook = new GitReceiveHook(converterFactory, hookEventFacade, handler);
  }

  @Override
  public final ReceivePack create(T connection, Repository repository) throws ServiceNotAuthorizedException, ServiceNotEnabledException {
    ReceivePack receivePack = createBasicReceivePack(connection, repository);
-    receivePack.setAllowNonFastForwards(isNonFastForwardAllowed());
+    receivePack.setAllowNonFastForwards(isNonFastForwardAllowed(repository));

    receivePack.setPreReceiveHook(hook);
    receivePack.setPostReceiveHook(hook);
@@ -61,7 +68,9 @@ public abstract class BaseReceivePackFactory<T> implements ReceivePackFactory<T>
  protected abstract ReceivePack createBasicReceivePack(T request, Repository repository)
    throws ServiceNotEnabledException, ServiceNotAuthorizedException;

-  private boolean isNonFastForwardAllowed() {
-    return ! handler.getConfig().isNonFastForwardDisallowed();
+  private boolean isNonFastForwardAllowed(Repository repository) {
+    String repositoryId = handler.getRepositoryId(repository.getConfig());
+    GitRepositoryConfig gitRepositoryConfig = storeProvider.getGitRepositoryConfig(repositoryId);
+    return !(handler.getConfig().isNonFastForwardDisallowed() || gitRepositoryConfig.isNonFastForwardDisallowed());
  }
 }
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java
@@ -27,6 +27,7 @@ package sonia.scm.protocolcommand.git;
 import com.google.inject.Inject;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.ReceivePack;
+import sonia.scm.api.v2.resources.GitRepositoryConfigStoreProvider;
 import sonia.scm.protocolcommand.RepositoryContext;
 import sonia.scm.repository.GitChangesetConverterFactory;
 import sonia.scm.repository.GitRepositoryHandler;
@@ -35,8 +36,11 @@ import sonia.scm.web.GitHookEventFacade;
 public class ScmReceivePackFactory extends BaseReceivePackFactory<RepositoryContext> {

  @Inject
-  public ScmReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, GitHookEventFacade hookEventFacade) {
-    super(converterFactory, handler, hookEventFacade);
+  public ScmReceivePackFactory(GitChangesetConverterFactory converterFactory,
+                               GitRepositoryHandler handler,
+                               GitHookEventFacade hookEventFacade,
+                               GitRepositoryConfigStoreProvider gitRepositoryConfigStoreProvider) {
+    super(converterFactory, handler, hookEventFacade, gitRepositoryConfigStoreProvider);
  }

  @Override
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryConfig.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryConfig.java
@@ -40,6 +40,7 @@ public class GitRepositoryConfig {
  }

  private String defaultBranch;
+  private boolean nonFastForwardDisallowed;

  public String getDefaultBranch() {
    return defaultBranch;
@@ -48,4 +49,10 @@ public class GitRepositoryConfig {
  public void setDefaultBranch(String defaultBranch) {
    this.defaultBranch = defaultBranch;
  }
+
+  public boolean isNonFastForwardDisallowed() { return nonFastForwardDisallowed; }
+
+  public void setNonFastForwardDisallowed(boolean nonFastForwardDisallowed) {
+    this.nonFastForwardDisallowed = nonFastForwardDisallowed;
+  }
 }
--- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java
+++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java
@@ -33,6 +33,7 @@ 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.api.v2.resources.GitRepositoryConfigStoreProvider;
 import sonia.scm.protocolcommand.git.BaseReceivePackFactory;
 import sonia.scm.repository.GitChangesetConverterFactory;
 import sonia.scm.repository.GitRepositoryHandler;
@@ -53,8 +54,11 @@ public class GitReceivePackFactory extends BaseReceivePackFactory<HttpServletReq
  private ReceivePackFactory<HttpServletRequest> wrapped;

  @Inject
-  public GitReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, GitHookEventFacade hookEventFacade) {
-    super(converterFactory, handler, hookEventFacade);
+  public GitReceivePackFactory(GitChangesetConverterFactory converterFactory,
+                               GitRepositoryHandler handler,
+                               GitHookEventFacade hookEventFacade,
+                               GitRepositoryConfigStoreProvider gitRepositoryConfigStoreProvider) {
+    super(converterFactory, handler, hookEventFacade, gitRepositoryConfigStoreProvider);
    this.wrapped = new DefaultReceivePackFactory();
  }

--- a/scm-plugins/scm-git-plugin/src/main/js/RepositoryConfig.tsx
+++ b/scm-plugins/scm-git-plugin/src/main/js/RepositoryConfig.tsx
@@ -27,6 +27,7 @@ import { Branch, Repository, Link } from "@scm-manager/ui-types";
 import {
  apiClient,
  BranchSelector,
+  Checkbox,
  ErrorPage,
  Loading,
  Subtitle,
@@ -45,8 +46,10 @@ type State = {
  error?: Error;
  branches: Branch[];
  selectedBranchName: string;
-  defaultBranchChanged: boolean;
+  nonFastForwardDisallowed: boolean;
+  changesSubmitted: boolean;
  disabled: boolean;
+  changed: boolean;
 };

 const GIT_CONFIG_CONTENT_TYPE = "application/vnd.scmm-gitConfig+json";
@@ -61,15 +64,16 @@ class RepositoryConfig extends React.Component<Props, State> {
      submitPending: false,
      branches: [],
      selectedBranchName: "",
-      defaultBranchChanged: false,
-      disabled: true
+      nonFastForwardDisallowed: false,
+      changesSubmitted: false,
+      disabled: true,
+      changed: false
    };
  }

  componentDidMount() {
    const { repository } = this.props;
    this.setState({
-      ...this.state,
      loadingBranches: true
    });
    const branchesLink = repository._links.branches as Link;
@@ -79,21 +83,18 @@ class RepositoryConfig extends React.Component<Props, State> {
      .then(payload => payload._embedded.branches)
      .then(branches =>
        this.setState({
-          ...this.state,
          branches,
          loadingBranches: false
        })
      )
      .catch(error =>
        this.setState({
-          ...this.state,
          error
        })
      );

    const configurationLink = repository._links.configuration as Link;
    this.setState({
-      ...this.state,
      loadingDefaultBranch: true
    });
    apiClient
@@ -101,15 +102,15 @@ class RepositoryConfig extends React.Component<Props, State> {
      .then(response => response.json())
      .then(payload =>
        this.setState({
-          ...this.state,
          selectedBranchName: payload.defaultBranch,
+          nonFastForwardDisallowed: payload.nonFastForwardDisallowed,
          disabled: !payload._links.update,
-          loadingDefaultBranch: false
+          loadingDefaultBranch: false,
+          changed: false
        })
      )
      .catch(error =>
        this.setState({
-          ...this.state,
          error
        })
      );
@@ -118,28 +119,36 @@ class RepositoryConfig extends React.Component<Props, State> {
  branchSelected = (branch?: Branch) => {
    if (!branch) {
      this.setState({
-        ...this.state,
        selectedBranchName: "",
-        defaultBranchChanged: false
+        changesSubmitted: false,
+        changed: true
      });
    } else {
      this.setState({
-        ...this.state,
        selectedBranchName: branch.name,
-        defaultBranchChanged: false
+        changesSubmitted: false,
+        changed: true
      });
    }
  };

+  onNonFastForwardDisallowed = (value: boolean) => {
+    this.setState({
+      nonFastForwardDisallowed: value,
+      changed: true
+    });
+  };
+
  submit = (event: FormEvent) => {
    event.preventDefault();

    const { repository } = this.props;
+    const { selectedBranchName, nonFastForwardDisallowed } = this.state;
    const newConfig = {
-      defaultBranch: this.state.selectedBranchName
+      defaultBranch: selectedBranchName,
+      nonFastForwardDisallowed
    };
    this.setState({
-      ...this.state,
      submitPending: true
    });
    const configurationLink = repository._links.configuration as Link;
@@ -147,14 +156,13 @@ class RepositoryConfig extends React.Component<Props, State> {
      .put(configurationLink.href, newConfig, GIT_CONFIG_CONTENT_TYPE)
      .then(() =>
        this.setState({
-          ...this.state,
          submitPending: false,
-          defaultBranchChanged: true
+          changesSubmitted: true,
+          changed: false
        })
      )
      .catch(error =>
        this.setState({
-          ...this.state,
          error
        })
      );
@@ -162,13 +170,13 @@ class RepositoryConfig extends React.Component<Props, State> {

  render() {
    const { t } = this.props;
-    const { loadingBranches, loadingDefaultBranch, submitPending, error, disabled } = this.state;
+    const { loadingBranches, loadingDefaultBranch, submitPending, error, disabled, changed } = this.state;

    if (error) {
      return (
        <ErrorPage
-          title={t("scm-git-plugin.repo-config.error.title")}
-          subtitle={t("scm-git-plugin.repo-config.error.subtitle")}
+          title={t("scm-git-plugin.repoConfig.error.title")}
+          subtitle={t("scm-git-plugin.repoConfig.error.subtitle")}
          error={error}
        />
      );
@@ -177,27 +185,32 @@ class RepositoryConfig extends React.Component<Props, State> {
    const submitButton = disabled ? null : (
      <Level
        right={
-          <SubmitButton
-            label={t("scm-git-plugin.repo-config.submit")}
-            loading={submitPending}
-            disabled={!this.state.selectedBranchName}
-          />
+          <SubmitButton label={t("scm-git-plugin.repoConfig.submit")} loading={submitPending} disabled={!changed} />
        }
      />
    );

    if (!(loadingBranches || loadingDefaultBranch)) {
+      const { branches, selectedBranchName, nonFastForwardDisallowed } = this.state;
      return (
        <>
          <hr />
-          <Subtitle subtitle={t("scm-git-plugin.repo-config.title")} />
+          <Subtitle subtitle={t("scm-git-plugin.repoConfig.title")} />
          {this.renderBranchChangedNotification()}
          <form onSubmit={this.submit}>
            <BranchSelector
-              label={t("scm-git-plugin.repo-config.default-branch")}
-              branches={this.state.branches}
+              label={t("scm-git-plugin.repoConfig.defaultBranch")}
+              branches={branches}
              onSelectBranch={this.branchSelected}
-              selectedBranch={this.state.selectedBranchName}
+              selectedBranch={selectedBranchName}
+              disabled={disabled}
+            />
+            <Checkbox
+              name="nonFastForwardDisallowed"
+              label={t("scm-git-plugin.repoConfig.nonFastForwardDisallowed")}
+              helpText={t("scm-git-plugin.repoConfig.nonFastForwardDisallowedHelpText")}
+              checked={nonFastForwardDisallowed}
+              onChange={this.onNonFastForwardDisallowed}
              disabled={disabled}
            />
            {submitButton}
@@ -210,19 +223,19 @@ class RepositoryConfig extends React.Component<Props, State> {
  }

  renderBranchChangedNotification = () => {
-    if (this.state.defaultBranchChanged) {
+    if (this.state.changesSubmitted) {
      return (
        <div className="notification is-primary">
          <button
            className="delete"
            onClick={() =>
              this.setState({
-                ...this.state,
-                defaultBranchChanged: false
+                changesSubmitted: false,
+                changed: false
              })
            }
          />
-          {this.props.t("scm-git-plugin.repo-config.success")}
+          {this.props.t("scm-git-plugin.repoConfig.success")}
        </div>
      );
    }
--- a/scm-plugins/scm-git-plugin/src/main/resources/locales/de/plugins.json
+++ b/scm-plugins/scm-git-plugin/src/main/resources/locales/de/plugins.json
@@ -31,16 +31,17 @@
      "disabledHelpText": "Aktiviere oder deaktiviere das Git Plugin",
      "submit": "Speichern"
    },
-    "repo-config": {
-      "link": "Konfiguration",
+    "repoConfig": {
      "title": "Git Einstellungen",
-      "default-branch": "Standard Branch",
+      "defaultBranch": "Default Branch",
+      "nonFastForwardDisallowed": "Deaktiviere \"Non Fast-Forward\"",
+      "nonFastForwardDisallowedHelpText": "Git Pushes ablehnen, die nicht \"fast-forward\" sind, wie \"--force\".",
      "submit": "Speichern",
      "error": {
        "title": "Fehler",
        "subtitle": "Ein Fehler ist aufgetreten."
      },
-      "success": "Der standard Branch wurde geändert!"
+      "success": "Einstellungen wurden erfolgreich geändert!"
    }
  },
  "permissions": {
--- a/scm-plugins/scm-git-plugin/src/main/resources/locales/en/plugins.json
+++ b/scm-plugins/scm-git-plugin/src/main/resources/locales/en/plugins.json
@@ -31,16 +31,17 @@
      "disabledHelpText": "Enable or disable the Git plugin",
      "submit": "Submit"
    },
-    "repo-config": {
-      "link": "Configuration",
+    "repoConfig": {
      "title": "Git Settings",
-      "default-branch": "Default Branch",
+      "defaultBranch": "Default Branch",
+      "nonFastForwardDisallowed": "Disallow Non Fast-Forward",
+      "nonFastForwardDisallowedHelpText": "Reject git pushes which are non fast-forward such as --force.",
      "submit": "Submit",
      "error": {
        "title": "Error",
        "subtitle": "Something went wrong"
      },
-      "success": "Default branch changed!"
+      "success": "Configuration changed successfully!"
    }
  },
  "permissions" : {
--- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/BaseReceivePackFactoryTest.java
+++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/BaseReceivePackFactoryTest.java
@@ -38,7 +38,9 @@ import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
+import sonia.scm.api.v2.resources.GitRepositoryConfigStoreProvider;
 import sonia.scm.repository.GitConfig;
+import sonia.scm.repository.GitRepositoryConfig;
 import sonia.scm.repository.GitRepositoryHandler;
 import sonia.scm.repository.GitTestHelper;
 import sonia.scm.web.CollectingPackParserListener;
@@ -59,17 +61,22 @@ public class BaseReceivePackFactoryTest {
  @Mock
  private GitRepositoryHandler handler;

-  private GitConfig config;
+  private GitConfig gitConfig;

  @Mock
  private ReceivePackFactory<Object> wrappedReceivePackFactory;

+  @Mock
+  private GitRepositoryConfigStoreProvider gitRepositoryConfigStoreProvider;
+
  private BaseReceivePackFactory<Object> factory;

  private Object request = new Object();

  private Repository repository;

+  private GitRepositoryConfig gitRepositoryConfig = new GitRepositoryConfig();
+
  @Rule
  public TemporaryFolder temporaryFolder = new TemporaryFolder();

@@ -77,13 +84,16 @@ public class BaseReceivePackFactoryTest {
  public void setUpObjectUnderTest() throws Exception {
    this.repository = createRepositoryForTesting();

-    config = new GitConfig();
-    when(handler.getConfig()).thenReturn(config);
+    gitConfig = new GitConfig();
+    when(handler.getConfig()).thenReturn(gitConfig);
+    when(handler.getRepositoryId(repository.getConfig())).thenReturn("heart-of-gold");

    ReceivePack receivePack = new ReceivePack(repository);
    when(wrappedReceivePackFactory.create(request, repository)).thenReturn(receivePack);

-    factory = new BaseReceivePackFactory<Object>(GitTestHelper.createConverterFactory(), handler, null) {
+    when(gitRepositoryConfigStoreProvider.getGitRepositoryConfig("heart-of-gold")).thenReturn(gitRepositoryConfig);
+
+    factory = new BaseReceivePackFactory<Object>(GitTestHelper.createConverterFactory(), handler, null, gitRepositoryConfigStoreProvider) {
      @Override
      protected ReceivePack createBasicReceivePack(Object request, Repository repository) throws ServiceNotEnabledException, ServiceNotAuthorizedException {
        return wrappedReceivePackFactory.create(request, repository);
@@ -108,7 +118,14 @@ public class BaseReceivePackFactoryTest {

  @Test
  public void testCreateWithDisabledNonFastForward() throws Exception {
-    config.setNonFastForwardDisallowed(true);
+    gitConfig.setNonFastForwardDisallowed(true);
+    ReceivePack receivePack = factory.create(request, repository);
+    assertFalse(receivePack.isAllowNonFastForwards());
+  }
+
+  @Test
+  public void testCreateWithLocalDisabledNonFastForward() throws Exception {
+    gitRepositoryConfig.setNonFastForwardDisallowed(true);
    ReceivePack receivePack = factory.create(request, repository);
    assertFalse(receivePack.isAllowNonFastForwards());
  }