opendev/gerrit
Code Issues Proposed changes

Merge changes I08379862,Ieff95d1b,I5f9a084f,If01d2b0f

Browse Source 
* changes:
  SetAccess: Reject label permissions with invalid label names
  SetAccess: Reject invalid permission names
  SetAccess: Test adding a plugin project permission
  SetAccess: Test adding a plugin global capability
This commit is contained in:
David Pursehouse
2020-03-17 00:12:09 +00:00
committed by Gerrit Code Review
parent 72fd0e8cfa bd36fd822b
commit a4224ac50f
3 changed files with 145 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
java/com/google/gerrit/acceptance/ExtensionRegistry.java
View File

@@ -15,7 +15,9 @@
package com.google.gerrit.acceptance;
import com.google.gerrit.extensions.api.changes.ActionVisitor;
import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.config.DownloadScheme;
import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
import com.google.gerrit.extensions.events.AccountIndexedListener;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.events.CommentAddedListener;
@@ -47,6 +49,8 @@ import java.util.ArrayList;
import java.util.List;
public class ExtensionRegistry {
public static final String PLUGIN_NAME = "myPlugin";
private final DynamicSet<AccountIndexedListener> accountIndexedListeners;
private final DynamicSet<ChangeIndexedListener> changeIndexedListeners;
private final DynamicSet<GroupIndexedListener> groupIndexedListeners;
@@ -71,6 +75,8 @@ public class ExtensionRegistry {
accountActivationValidationListeners;
private final DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
private final DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners;
private final DynamicMap<CapabilityDefinition> capabilityDefinitions;
private final DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions;
@Inject
ExtensionRegistry(
@@ -96,7 +102,9 @@ public class ExtensionRegistry {
DynamicSet<GroupBackend> groupBackends,
DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners,
DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners,
DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners) {
DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners,
DynamicMap<CapabilityDefinition> capabilityDefinitions,
DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -120,6 +128,8 @@ public class ExtensionRegistry {
this.accountActivationValidationListeners = accountActivationValidationListeners;
this.onSubmitValidationListeners = onSubmitValidationListeners;
this.workInProgressStateChangedListeners = workInProgressStateChangedListeners;
this.capabilityDefinitions = capabilityDefinitions;
this.pluginProjectPermissionDefinitions = pluginProjectPermissionDefinitions;
}
public Registration newRegistration() {
@@ -227,6 +237,15 @@ public class ExtensionRegistry {
return add(workInProgressStateChangedListeners, workInProgressStateChangedListener);
}
public Registration add(CapabilityDefinition capabilityDefinition, String exportName) {
return add(capabilityDefinitions, capabilityDefinition, exportName);
}
public Registration add(
PluginProjectPermissionDefinition pluginProjectPermissionDefinition, String exportName) {
return add(pluginProjectPermissionDefinitions, pluginProjectPermissionDefinition, exportName);
}
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
@@ -240,7 +259,7 @@ public class ExtensionRegistry {
private <T> Registration add(DynamicMap<T> dynamicMap, T extension, String exportName) {
RegistrationHandle registrationHandle =
((PrivateInternals_DynamicMapImpl<T>) dynamicMap)
.put("myPlugin", exportName, Providers.of(extension));
.put(PLUGIN_NAME, exportName, Providers.of(extension));
registrationHandles.add(registrationHandle);
return this;
}

25
java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
View File

@@ -19,6 +19,7 @@ import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.entities.Project;
@@ -150,6 +151,13 @@ public class SetAccessUtil {
throw new BadRequestException("invalid section name");
}
RefPattern.validate(name);
// Check all permissions for soundness
for (Permission p : section.getPermissions()) {
if (!isPermission(p.getName())) {
throw new BadRequestException("Unknown permission: " + p.getName());
}
}
} else {
// Check all permissions for soundness
for (Permission p : section.getPermissions()) {
@@ -239,6 +247,23 @@ public class SetAccessUtil {
}
}
private boolean isPermission(String name) {
if (Permission.isPermission(name)) {
if (Permission.isLabel(name) || Permission.isLabelAs(name)) {
String labelName = Permission.extractLabel(name);
try {
LabelType.checkName(labelName);
} catch (IllegalArgumentException e) {
return false;
}
}
return true;
}
Set<String> pluginPermissions =
pluginPermissionsUtil.collectPluginProjectPermissions().keySet();
return pluginPermissions.contains(name);
}
private boolean isCapability(String name) {
if (GlobalCapability.isGlobalCapability(name)) {
return true;

99
javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
View File

@@ -44,6 +44,8 @@ import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -142,6 +144,68 @@ public class AccessIT extends AbstractDaemonTest {
newProjectName.get(), RefNames.REFS_CONFIG, null, initialHead, initialHead, updatedHead);
}
@Test
public void addAccessSectionForPluginPermission() throws Exception {
try (Registration registration =
extensionRegistry
.newRegistration()
.add(
new PluginProjectPermissionDefinition() {
@Override
public String getDescription() {
return "A Plugin Project Permission";
}
},
"fooPermission")) {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = newAccessSectionInfo();
PermissionInfo foo = newPermissionInfo();
PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
foo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
accessSectionInfo.permissions.put(
"plugin-" + ExtensionRegistry.PLUGIN_NAME + "-fooPermission", foo);
accessInput.add.put(REFS_HEADS, accessSectionInfo);
ProjectAccessInfo updatedAccessSectionInfo = pApi().access(accessInput);
assertThat(updatedAccessSectionInfo.local).isEqualTo(accessInput.add);
assertThat(pApi().access().local).isEqualTo(accessInput.add);
}
}
@Test
public void addAccessSectionWithInvalidPermission() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
PermissionInfo push = newPermissionInfo();
PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
push.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
accessSectionInfo.permissions.put("Invalid Permission", push);
accessInput.add.put(REFS_HEADS, accessSectionInfo);
BadRequestException ex =
assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
assertThat(ex).hasMessageThat().isEqualTo("Unknown permission: Invalid Permission");
}
@Test
public void addAccessSectionWithInvalidLabelPermission() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
PermissionInfo push = newPermissionInfo();
PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
push.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
accessSectionInfo.permissions.put("label-Invalid Permission", push);
accessInput.add.put(REFS_HEADS, accessSectionInfo);
BadRequestException ex =
assertThrows(BadRequestException.class, () -> pApi().access(accessInput));
assertThat(ex).hasMessageThat().isEqualTo("Unknown permission: label-Invalid Permission");
}
@Test
public void createAccessChangeNop() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();
@@ -455,6 +519,41 @@ public class AccessIT extends AbstractDaemonTest {
.containsAtLeastElementsIn(accessSectionInfo.permissions.keySet());
}
@Test
public void addPluginGlobalCapability() throws Exception {
try (Registration registration =
extensionRegistry
.newRegistration()
.add(
new CapabilityDefinition() {
@Override
public String getDescription() {
return "A Plugin Global Capability";
}
},
"fooCapability")) {
ProjectAccessInput accessInput = newProjectAccessInput();
AccessSectionInfo accessSectionInfo = newAccessSectionInfo();
PermissionInfo foo = newPermissionInfo();
PermissionRuleInfo pri = new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false);
foo.rules.put(SystemGroupBackend.REGISTERED_USERS.get(), pri);
accessSectionInfo.permissions.put(ExtensionRegistry.PLUGIN_NAME + "-fooCapability", foo);
accessInput.add.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo);
ProjectAccessInfo updatedAccessSectionInfo =
gApi.projects().name(allProjects.get()).access(accessInput);
assertThat(
updatedAccessSectionInfo
.local
.get(AccessSection.GLOBAL_CAPABILITIES)
.permissions
.keySet())
.containsAtLeastElementsIn(accessSectionInfo.permissions.keySet());
}
}
@Test
public void addPermissionAsGlobalCapability() throws Exception {
ProjectAccessInput accessInput = newProjectAccessInput();