Set Access REST endpoint: Reject invalid label ranges
It should not be possible to add permissions with a range where min is
greater than max, where min is present but max is missing or where max
is present but min is missing.
Only validate the access sections that are being added, but not the ones
which are being removed (so that invalid ranges can still be removed).
Release-Notes: Set Access REST endpoint rejects invalid label ranges
Signed-off-by: Edwin Kempin <ekempin@google.com>
Bug: Issue Google b/260961794
Change-Id: Ib375546980861b5f8f89ce53c7316ce7e974b50f
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccess.java b/java/com/google/gerrit/server/restapi/project/SetAccess.java
index 23d60fe..6957275 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccess.java
@@ -19,6 +19,9 @@
import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.InvalidNameException;
+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.ProjectAccessInfo;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -39,6 +42,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.List;
+import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
@@ -80,6 +84,8 @@
throws Exception {
MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();
+ validateInput(input);
+
ProjectConfig config;
List<AccessSection> removals =
@@ -137,4 +143,65 @@
return Response.ok(getAccess.apply(rsrc.getNameKey()));
}
+
+ private static void validateInput(ProjectAccessInput input) throws BadRequestException {
+ if (input.add != null) {
+ for (Map.Entry<String, AccessSectionInfo> accessSectionEntry : input.add.entrySet()) {
+ validateAccessSection(accessSectionEntry.getKey(), accessSectionEntry.getValue());
+ }
+ }
+ }
+
+ private static void validateAccessSection(String ref, AccessSectionInfo accessSectionInfo)
+ throws BadRequestException {
+ if (accessSectionInfo != null) {
+ for (Map.Entry<String, PermissionInfo> permissionEntry :
+ accessSectionInfo.permissions.entrySet()) {
+ validatePermission(ref, permissionEntry.getKey(), permissionEntry.getValue());
+ }
+ }
+ }
+
+ private static void validatePermission(
+ String ref, String permission, PermissionInfo permissionInfo) throws BadRequestException {
+ if (permissionInfo != null) {
+ for (Map.Entry<String, PermissionRuleInfo> permissionRuleEntry :
+ permissionInfo.rules.entrySet()) {
+ validatePermissionRule(
+ ref, permission, permissionRuleEntry.getKey(), permissionRuleEntry.getValue());
+ }
+ }
+ }
+
+ private static void validatePermissionRule(
+ String ref, String permission, String groupId, PermissionRuleInfo permissionRuleInfo)
+ throws BadRequestException {
+ if (permissionRuleInfo != null) {
+ if (permissionRuleInfo.min != null || permissionRuleInfo.max != null) {
+ if (permissionRuleInfo.min == null) {
+ throw new BadRequestException(
+ String.format(
+ "Invalid range for permission rule that assigns %s to group %s on ref %s:"
+ + " ..%d (min is required if max is set)",
+ permission, groupId, ref, permissionRuleInfo.max));
+ }
+
+ if (permissionRuleInfo.max == null) {
+ throw new BadRequestException(
+ String.format(
+ "Invalid range for permission rule that assigns %s to group %s on ref %s:"
+ + " %d.. (max is required if min is set)",
+ permission, groupId, ref, permissionRuleInfo.min));
+ }
+
+ if (permissionRuleInfo.min > permissionRuleInfo.max) {
+ throw new BadRequestException(
+ String.format(
+ "Invalid range for permission rule that assigns %s to group %s on ref %s:"
+ + " %d..%d (min must be <= max)",
+ permission, groupId, ref, permissionRuleInfo.min, permissionRuleInfo.max));
+ }
+ }
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
index 7c33ec2..311d4be 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
@@ -325,6 +325,79 @@
}
@Test
+ public void addAccessSectionWithInvalidLabelRange_minGreaterThanMax() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo permissionRuleInfo =
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), permissionRuleInfo);
+ permissionRuleInfo.min = 1;
+ permissionRuleInfo.max = -1;
+ accessSectionInfo.permissions.put("label-Code-Review", permissionInfo);
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Invalid range for permission rule that assigns label-Code-Review to group %s"
+ + " on ref refs/heads/*: 1..-1 (min must be <= max)",
+ SystemGroupBackend.REGISTERED_USERS.get()));
+ }
+
+ @Test
+ public void addAccessSectionWithInvalidLabelRange_minSetMaxMissing() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo permissionRuleInfo =
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), permissionRuleInfo);
+ permissionRuleInfo.min = -1;
+ accessSectionInfo.permissions.put("label-Code-Review", permissionInfo);
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Invalid range for permission rule that assigns label-Code-Review to group %s"
+ + " on ref refs/heads/*: -1.. (max is required if min is set)",
+ SystemGroupBackend.REGISTERED_USERS.get()));
+ }
+
+ @Test
+ public void addAccessSectionWithInvalidLabelRange_maxSetMinMissing() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ PermissionInfo permissionInfo = newPermissionInfo();
+ PermissionRuleInfo permissionRuleInfo =
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
+ permissionInfo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), permissionRuleInfo);
+ permissionRuleInfo.max = 1;
+ accessSectionInfo.permissions.put("label-Code-Review", permissionInfo);
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ BadRequestException ex =
+ assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
+ assertThat(ex)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Invalid range for permission rule that assigns label-Code-Review to group %s"
+ + " on ref refs/heads/*: ..1 (min is required if max is set)",
+ SystemGroupBackend.REGISTERED_USERS.get()));
+ }
+
+ @Test
public void createAccessChangeNop() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
assertThrows(BadRequestException.class, () -> pApi().accessChange(accessInput));
