Merge "Add config to disable private changes"

Documentation/config-gerrit.txt

@@ -1252,6 +1252,12 @@ Zero or negative values allow robot comments of unlimited size.
 +
 The default limit is 1024kB.
 
+[[change.disablePrivateChanges]]change.disablePrivateChanges::
++
+If set to true, users are not allowed to create private changes.
++
+The default is false.
+
 [[changeCleanup]]
 === Section changeCleanup
 

Documentation/rest-api-config.txt

@@ -1398,6 +1398,8 @@ interface should poll for updates to the currently open change].
 |`submit_whole_topic` ||
 link:config-gerrit.html#change.submitWholeTopic[A configuration if
 the whole topic is submitted].
+|`disable_private_changes` |not set if `false`|
+Returns true if private changes are disabled.
 |=============================
 
 [[check-account-external-ids-input]]

java/com/google/gerrit/extensions/common/ChangeConfigInfo.java

@@ -18,6 +18,7 @@ public class ChangeConfigInfo {
   public Boolean allowBlame;
   public Boolean showAssigneeInChangesTable;
   public Boolean allowDrafts;
+  public Boolean disablePrivateChanges;
   public int largeChange;
   public String replyLabel;
   public String replyTooltip;

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -376,6 +376,7 @@ class ReceiveCommits {
   private MagicBranchInput magicBranch;
   private boolean newChangeForAllNotInTarget;
   private String setFullNameTo;
+  private boolean setChangeAsPrivate;
 
   // Handles for outputting back over the wire to the end user.
   private Task newProgress;
@@ -1521,6 +1522,18 @@ class ReceiveCommits {
       return;
     }
 
+    boolean privateByDefault =
+        projectCache.get(project.getNameKey()).is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
+    setChangeAsPrivate =
+        magicBranch.draft
+            || magicBranch.isPrivate
+            || (privateByDefault && !magicBranch.removePrivate);
+
+    if (receiveConfig.disablePrivateChanges && setChangeAsPrivate) {
+      reject(cmd, "private changes are disabled");
+      return;
+    }
+
     if (magicBranch.workInProgress && magicBranch.ready) {
       reject(cmd, "the options 'wip' and 'ready' are mutually exclusive");
       return;
@@ -2133,18 +2146,13 @@ class ReceiveCommits {
     }
 
     private void setChangeId(int id) {
-      boolean privateByDefault =
-          projectCache.get(project.getNameKey()).is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
 
       changeId = new Change.Id(id);
       ins =
           changeInserterFactory
               .create(changeId, commit, refName)
               .setTopic(magicBranch.topic)
-              .setPrivate(
+              .setPrivate(setChangeAsPrivate)
-                  magicBranch.draft
-                      || magicBranch.isPrivate
-                      || (privateByDefault && !magicBranch.removePrivate))
               .setWorkInProgress(magicBranch.workInProgress)
               // Changes already validated in validateNewCommits.
               .setValidate(false);

java/com/google/gerrit/server/git/receive/ReceiveConfig.java

@@ -28,6 +28,7 @@ class ReceiveConfig {
   final boolean checkMagicRefs;
   final boolean checkReferencedObjectsAreReachable;
   final int maxBatchCommits;
+  final boolean disablePrivateChanges;
   private final int systemMaxBatchChanges;
   private final AccountLimits.Factory limitsFactory;
 
@@ -38,6 +39,7 @@ class ReceiveConfig {
         config.getBoolean("receive", null, "checkReferencedObjectsAreReachable", true);
     maxBatchCommits = config.getInt("receive", null, "maxBatchCommits", 10000);
     systemMaxBatchChanges = config.getInt("receive", "maxBatchChanges", 0);
+    disablePrivateChanges = config.getBoolean("change", null, "disablePrivateChanges", false);
     this.limitsFactory = limitsFactory;
   }
 

java/com/google/gerrit/server/restapi/change/CreateChange.java

@@ -28,6 +28,7 @@ import com.google.gerrit.extensions.common.ChangeInput;
 import com.google.gerrit.extensions.common.MergeInput;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -114,6 +115,7 @@ public class CreateChange
   private final SubmitType submitType;
   private final NotifyUtil notifyUtil;
   private final ContributorAgreementsChecker contributorAgreements;
+  private final boolean disablePrivateChanges;
 
   @Inject
   CreateChange(
@@ -152,6 +154,7 @@ public class CreateChange
     this.changeFinder = changeFinder;
     this.psUtil = psUtil;
     this.submitType = config.getEnum("project", null, "submitType", SubmitType.MERGE_IF_NECESSARY);
+    this.disablePrivateChanges = config.getBoolean("change", null, "disablePrivateChanges", false);
     this.mergeUtilFactory = mergeUtilFactory;
     this.notifyUtil = notifyUtil;
     this.contributorAgreements = contributorAgreements;
@@ -181,6 +184,13 @@ public class CreateChange
     }
 
     ProjectResource rsrc = projectsCollection.parse(input.project);
+    boolean privateByDefault = rsrc.getProjectState().is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
+    boolean isPrivate = input.isPrivate == null ? privateByDefault : input.isPrivate;
+
+    if (isPrivate && disablePrivateChanges) {
+      throw new MethodNotAllowedException("private changes are disabled");
+    }
+
     contributorAgreements.check(rsrc.getNameKey(), rsrc.getUser());
 
     Project.NameKey project = rsrc.getNameKey();
@@ -257,7 +267,6 @@ public class CreateChange
         c = newCommit(oi, rw, author, mergeTip, commitMessage);
       }
 
-      boolean privateByDefault = rsrc.getProjectState().is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
       Change.Id changeId = new Change.Id(seq.nextChangeId());
       ChangeInserter ins = changeInserterFactory.create(changeId, c, refName);
       ins.setMessage(String.format("Uploaded patch set %s.", ins.getPatchSetId().get()));
@@ -266,7 +275,7 @@ public class CreateChange
         topic = Strings.emptyToNull(topic.trim());
       }
       ins.setTopic(topic);
-      ins.setPrivate(input.isPrivate == null ? privateByDefault : input.isPrivate);
+      ins.setPrivate(isPrivate);
       ins.setWorkInProgress(input.workInProgress != null && input.workInProgress);
       ins.setGroups(groups);
       ins.setNotify(input.notify);

java/com/google/gerrit/server/restapi/change/PostPrivate.java

@@ -20,6 +20,7 @@ import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.webui.UiAction;
@@ -27,6 +28,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.update.BatchUpdate;
@@ -36,6 +38,7 @@ import com.google.gerrit.server.update.UpdateException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import org.eclipse.jgit.lib.Config;
 
 @Singleton
 public class PostPrivate
@@ -45,6 +48,7 @@ public class PostPrivate
   private final Provider<ReviewDb> dbProvider;
   private final PermissionBackend permissionBackend;
   private final SetPrivateOp.Factory setPrivateOpFactory;
+  private final boolean disablePrivateChanges;
 
   @Inject
   PostPrivate(
@@ -52,18 +56,24 @@ public class PostPrivate
       RetryHelper retryHelper,
       ChangeMessagesUtil cmUtil,
       PermissionBackend permissionBackend,
-      SetPrivateOp.Factory setPrivateOpFactory) {
+      SetPrivateOp.Factory setPrivateOpFactory,
+      @GerritServerConfig Config config) {
     super(retryHelper);
     this.dbProvider = dbProvider;
     this.cmUtil = cmUtil;
     this.permissionBackend = permissionBackend;
     this.setPrivateOpFactory = setPrivateOpFactory;
+    this.disablePrivateChanges = config.getBoolean("change", null, "disablePrivateChanges", false);
   }
 
   @Override
   public Response<String> applyImpl(
       BatchUpdate.Factory updateFactory, ChangeResource rsrc, SetPrivateOp.Input input)
       throws RestApiException, UpdateException {
+    if (disablePrivateChanges) {
+      throw new MethodNotAllowedException("private changes are disabled");
+    }
+
     if (!canSetPrivate(rsrc).value()) {
       throw new AuthException("not allowed to mark private");
     }
@@ -88,7 +98,7 @@ public class PostPrivate
     return new UiAction.Description()
         .setLabel("Mark private")
         .setTitle("Mark change as private")
-        .setVisible(and(!change.isPrivate(), canSetPrivate(rsrc)));
+        .setVisible(and(!disablePrivateChanges && !change.isPrivate(), canSetPrivate(rsrc)));
   }
 
   private BooleanCondition canSetPrivate(ChangeResource rsrc) {

java/com/google/gerrit/server/restapi/config/GetServerInfo.java

@@ -246,6 +246,8 @@ public class GetServerInfo implements RestReadView<ConfigResource> {
     info.updateDelay =
         (int) ConfigUtil.getTimeUnit(cfg, "change", null, "updateDelay", 300, TimeUnit.SECONDS);
     info.submitWholeTopic = MergeSuperSet.wholeTopicEnabled(cfg);
+    info.disablePrivateChanges =
+        toBoolean(config.getBoolean("change", null, "disablePrivateChanges", false));
     return info;
   }
 

javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java

@@ -0,0 +1,118 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Test;
+
+public class DisablePrivateChangesIT extends AbstractDaemonTest {
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void createPrivateChangeWithDisablePrivateChangesTrue() throws Exception {
+    ChangeInput input = new ChangeInput(project.get(), "master", "empty change");
+    input.isPrivate = true;
+    exception.expect(MethodNotAllowedException.class);
+    exception.expectMessage("private changes are disabled");
+    gApi.changes().create(input);
+  }
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void createNonPrivateChangeWithDisablePrivateChangesTrue() throws Exception {
+    ChangeInput input = new ChangeInput(project.get(), "master", "empty change");
+    assertThat(gApi.changes().create(input).get().isPrivate).isNull();
+  }
+
+  @Test
+  public void createPrivateChangeWithDisablePrivateChangesFalse() throws Exception {
+    ChangeInput input = new ChangeInput(project.get(), "master", "empty change");
+    input.isPrivate = true;
+    assertThat(gApi.changes().create(input).get().isPrivate).isEqualTo(true);
+  }
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void pushPrivatesWithDisablePrivateChangesTrue() throws Exception {
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%private");
+    result.assertErrorStatus();
+  }
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void pushDraftsWithDisablePrivateChangesTrue() throws Exception {
+    RevCommit initialHead = getRemoteHead();
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%draft");
+    result.assertErrorStatus();
+
+    testRepo.reset(initialHead);
+    result = pushFactory.create(db, admin.getIdent(), testRepo).to("refs/drafts/master");
+    result.assertErrorStatus();
+  }
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void pushWithDisablePrivateChangesTrue() throws Exception {
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master");
+    result.assertOkStatus();
+    assertThat(result.getChange().change().isPrivate()).isFalse();
+  }
+
+  @Test
+  public void pushPrivatesWithDisablePrivateChangesFalse() throws Exception {
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%private");
+    assertThat(result.getChange().change().isPrivate()).isEqualTo(true);
+  }
+
+  @Test
+  public void pushDraftsWithDisablePrivateChangesFalse() throws Exception {
+    RevCommit initialHead = getRemoteHead();
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%draft");
+    assertThat(result.getChange().change().isPrivate()).isEqualTo(true);
+
+    testRepo.reset(initialHead);
+    result = pushFactory.create(db, admin.getIdent(), testRepo).to("refs/drafts/master");
+    assertThat(result.getChange().change().isPrivate()).isEqualTo(true);
+  }
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void setPrivateWithDisablePrivateChangesTrue() throws Exception {
+    PushOneCommit.Result result = createChange();
+
+    exception.expect(MethodNotAllowedException.class);
+    exception.expectMessage("private changes are disabled");
+    gApi.changes().id(result.getChangeId()).setPrivate(true, "set private");
+  }
+
+  @Test
+  public void setPrivateWithDisablePrivateChangesFalse() throws Exception {
+    PushOneCommit.Result result = createChange();
+    gApi.changes().id(result.getChangeId()).setPrivate(true, "set private");
+    assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
+  }
+}

javatests/com/google/gerrit/acceptance/rest/change/PrivateByDefaultIT.java

@@ -17,14 +17,17 @@ package com.google.gerrit.acceptance.rest.change;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.extensions.api.projects.ConfigInput;
 import com.google.gerrit.extensions.client.InheritableBoolean;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.reviewdb.client.Project;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -69,6 +72,17 @@ public class PrivateByDefaultIT extends AbstractDaemonTest {
     assertThat(info.isPrivate).isTrue();
   }
 
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void createChangeWithPrivateByDefaultAndDisablePrivateChangesTrue() throws Exception {
+    setPrivateByDefault(project2, InheritableBoolean.TRUE);
+
+    ChangeInput input = new ChangeInput(project2.get(), "master", "empty change");
+    exception.expect(MethodNotAllowedException.class);
+    exception.expectMessage("private changes are disabled");
+    gApi.changes().create(input);
+  }
+
   @Test
   public void pushWithPrivateByDefaultEnabled() throws Exception {
     setPrivateByDefault(project2, InheritableBoolean.TRUE);
@@ -97,6 +111,33 @@ public class PrivateByDefaultIT extends AbstractDaemonTest {
     assertThat(createChange(project2).getChange().change().isPrivate()).isEqualTo(true);
   }
 
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void pushPrivatesWithPrivateByDefaultAndDisablePrivateChangesTrue() throws Exception {
+    setPrivateByDefault(project2, InheritableBoolean.TRUE);
+
+    TestRepository<InMemoryRepository> testRepo = cloneProject(project2);
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%private");
+    result.assertErrorStatus();
+  }
+
+  @Test
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
+  public void pushDraftsWithPrivateByDefaultAndDisablePrivateChangesTrue() throws Exception {
+    setPrivateByDefault(project2, InheritableBoolean.TRUE);
+
+    RevCommit initialHead = getRemoteHead();
+    TestRepository<InMemoryRepository> testRepo = cloneProject(project2);
+    PushOneCommit.Result result =
+        pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%draft");
+    result.assertErrorStatus();
+
+    testRepo.reset(initialHead);
+    result = pushFactory.create(db, admin.getIdent(), testRepo).to("refs/drafts/master");
+    result.assertErrorStatus();
+  }
+
   private void setPrivateByDefault(Project.NameKey proj, InheritableBoolean value)
       throws Exception {
     ConfigInput input = new ConfigInput();

javatests/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java

@@ -61,6 +61,7 @@ public class ServerInfoIT extends AbstractDaemonTest {
   @GerritConfig(name = "change.replyTooltip", value = "Publish votes and draft comments")
   @GerritConfig(name = "change.replyLabel", value = "Vote")
   @GerritConfig(name = "change.updateDelay", value = "50s")
+  @GerritConfig(name = "change.disablePrivateChanges", value = "true")
 
   // download
   @GerritConfig(
@@ -105,6 +106,7 @@ public class ServerInfoIT extends AbstractDaemonTest {
     assertThat(i.change.replyTooltip).startsWith("Publish votes and draft comments");
     assertThat(i.change.replyLabel).isEqualTo("Vote\u2026");
     assertThat(i.change.updateDelay).isEqualTo(50);
+    assertThat(i.change.disablePrivateChanges).isTrue();
 
     // download
     assertThat(i.download.archives).containsExactly("tar", "tbz2", "tgz", "txz");
@@ -178,6 +180,7 @@ public class ServerInfoIT extends AbstractDaemonTest {
     assertThat(i.change.replyTooltip).startsWith("Reply and score");
     assertThat(i.change.replyLabel).isEqualTo("Reply\u2026");
     assertThat(i.change.updateDelay).isEqualTo(300);
+    assertThat(i.change.disablePrivateChanges).isNull();
 
     // download
     assertThat(i.download.archives).containsExactly("tar", "tbz2", "tgz", "txz");