Merge changes If3ddb515,I6e1604b4

* changes:
  ProjectsRestApiBindingsIT: Fix javadoc typo
  Fix: Require contributor agreement for config changes

java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java

diff --git a/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java b/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
index a34b7d8..62c88276 100644
--- a/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
+++ b/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
@@ -42,6 +42,7 @@
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.project.ContributorAgreementsChecker;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectConfig;
 import com.google.gerrit.server.update.BatchUpdate;
@@ -75,6 +76,7 @@
 
   private final PermissionBackend permissionBackend;
   private final ChangeJson.Factory jsonFactory;
+  private final ContributorAgreementsChecker contributorAgreements;
 
   @Inject
   RepoMetaDataUpdater(
@@ -86,7 +88,8 @@
       Sequences seq,
       BatchUpdate.Factory updateFactory,
       PermissionBackend permissionBackend,
-      ChangeJson.Factory jsonFactory) {
+      ChangeJson.Factory jsonFactory,
+      ContributorAgreementsChecker contributorAgreements) {
     this.metaDataUpdateFactory = metaDataUpdateFactory;
     this.user = user;
     this.projectConfigFactory = projectConfigFactory;
@@ -96,6 +99,7 @@
     this.updateFactory = updateFactory;
     this.permissionBackend = permissionBackend;
     this.jsonFactory = jsonFactory;
+    this.contributorAgreements = contributorAgreements;
   }
 
   /**
@@ -125,6 +129,8 @@
   public ConfigChangeCreator configChangeCreator(
       Project.NameKey projectName, @Nullable String message, String defaultMessage)
       throws PermissionBackendException, AuthException, IOException, ConfigInvalidException {
+    contributorAgreements.check(projectName, user.get());
+
     message = validateMessage(message, defaultMessage);
     PermissionBackend.ForProject forProject =
         permissionBackend.user(user.get()).project(projectName);

javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java

diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 2803603..e7b548d 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -23,8 +23,10 @@
 import static java.util.Comparator.comparing;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.UseClockStep;
 import com.google.gerrit.acceptance.config.GerritConfig;
 import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
@@ -37,6 +39,11 @@
 import com.google.gerrit.entities.InternalGroup;
 import com.google.gerrit.entities.Permission;
 import com.google.gerrit.entities.PermissionRule;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.access.AccessSectionInfo;
+import com.google.gerrit.extensions.api.access.PermissionInfo;
+import com.google.gerrit.extensions.api.access.PermissionRuleInfo;
+import com.google.gerrit.extensions.api.access.ProjectAccessInput;
 import com.google.gerrit.extensions.api.changes.CherryPickInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
@@ -52,7 +59,10 @@
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.testing.ConfigSuite;
+import com.google.gson.Strictness;
+import com.google.gson.stream.JsonReader;
 import com.google.inject.Inject;
 import java.util.List;
 import org.eclipse.jgit.lib.Config;
@@ -362,6 +372,69 @@
   }
 
   @Test
+  public void createAccessChangeRespectsCLA() throws Exception {
+    assume().that(isContributorAgreementsEnabled()).isTrue();
+
+    projectOperations
+        .project(project)
+        .forUpdate()
+        .add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
+        .add(allow(Permission.PUSH).ref("refs/for/" + RefNames.REFS_CONFIG).group(REGISTERED_USERS))
+        .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(REGISTERED_USERS))
+        .add(allow(Permission.SUBMIT).ref(RefNames.REFS_CONFIG).group(REGISTERED_USERS))
+        .update();
+
+    // Create an access change succeeds when agreement is not required
+    setUseContributorAgreements(InheritableBoolean.FALSE);
+    RestResponse resp = createPermissionsChange("agreement not required");
+    resp.assertCreated();
+    ChangeInfo got;
+    try (JsonReader jsonReader = new JsonReader(resp.getReader())) {
+      jsonReader.setStrictness(Strictness.LENIENT);
+      got = newGson().fromJson(jsonReader, ChangeInfo.class);
+      assertThat(got.subject).isEqualTo("agreement not required");
+    }
+
+    // Create an access change is not allowed when CLA is required but not signed
+    setUseContributorAgreements(InheritableBoolean.TRUE);
+    resp = createPermissionsChange("agreement required - fails");
+    resp.assertForbidden();
+    assertThat(resp.hasContent()).isTrue();
+    assertThat(resp.getEntityContent()).startsWith("No Contributor Agreement on file ");
+
+    // Sign the agreement
+    gApi.accounts().self().signAgreement(caAutoVerify.getName());
+
+    // Explicitly reset the user to force a new request context
+    requestScopeOperations.setApiUser(user.id());
+
+    // Create an access change succeeds after signing the agreement
+    resp = createPermissionsChange("agreement required - passes");
+    resp.assertCreated();
+    try (JsonReader jsonReader = new JsonReader(resp.getReader())) {
+      jsonReader.setStrictness(Strictness.LENIENT);
+      got = newGson().fromJson(jsonReader, ChangeInfo.class);
+      assertThat(got.subject).isEqualTo("agreement required - passes");
+    }
+  }
+
+  private RestResponse createPermissionsChange(String message) throws Exception {
+    ProjectAccessInput in = new ProjectAccessInput();
+    in.message = message;
+
+    PermissionInfo p = new PermissionInfo(null, null);
+    p.rules =
+        ImmutableMap.of(
+            SystemGroupBackend.REGISTERED_USERS.get(),
+            new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false));
+    AccessSectionInfo a = new AccessSectionInfo();
+    a.permissions = ImmutableMap.of("read", p);
+    in.add = ImmutableMap.of("refs/heads/*", a);
+
+    return userRestSession.put("/projects/" + project.get() + "/access:review", in);
+  }
+
+  @Test
   public void createExcludedProjectChangeIgnoresCLA() throws Exception {
     // Contributor agreements configured with excludeProjects = ExcludedProject
     // in AbstractDaemonTest.configureContributorAgreement(...)

javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java

diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
index a4cf221..6d72a64 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
@@ -46,7 +46,7 @@
 /**
  * Tests for checking the bindings of the projects REST API.
  *
- * <p>These tests only verify that the project REST endpoints are correctly bound, they do no test
+ * <p>These tests only verify that the project REST endpoints are correctly bound, they do not test
  * the functionality of the project REST endpoints.
  */
 public class ProjectsRestApiBindingsIT extends AbstractDaemonTest {