Fix: Require contributor agreement for config changes
Apply the same contributor agreement check when creating a change that
updates project configuration.
For parity with the 'git push' path, this check should also happen when
updating project configs without creating a change, but that's more
likely to impact users, so it will be done in a follow-up change.
Bug: Issue 455720486
Release-Notes: Fixed project config update changes to require contributor agreement
Change-Id: I6e1604b4cce736a43d4aaf180aeac234cb2a9395
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);
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(...)
