opendev/gerrit
Code Issues Proposed changes

UiActions: defer GlobalPermission checks on views

Browse Source 
Instead of running the global permission check immediately, always get
the view's UiAction.Description and chain the global permissions
checks into visible's BooleanCondition tree.  This allows global
permissions and plugin-specific permissions to evaluate through
bulkEvaluateTest.

Change-Id: I80d7f08d56526838d5a0990e91ba586ca0d7c9d3
This commit is contained in:
Shawn Pearce
2017-08-11 23:12:16 -07:00
parent 2c76a76449
commit a42f8b5e75
1 changed files with 19 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java
View File

@@ -14,12 +14,15 @@
package com.google.gerrit.server.extensions.webui; package com.google.gerrit.server.extensions.webui;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toList;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.collect.Streams; import com.google.common.collect.Streams;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission; import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestCollection;
import com.google.gerrit.extensions.restapi.RestResource; import com.google.gerrit.extensions.restapi.RestResource;
@@ -35,6 +38,7 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
@@ -110,22 +114,27 @@ public class UiActions {
return null; return null;
} }
UiAction.Description dsc = ((UiAction<R>) view).getDescription(resource);
if (dsc == null) {
return null;
}
Set<GlobalOrPluginPermission> globalRequired;
try { try {
Set<GlobalOrPluginPermission> need = globalRequired = GlobalPermission.fromAnnotation(e.getPluginName(), view.getClass());
GlobalPermission.fromAnnotation(e.getPluginName(), view.getClass());
if (!need.isEmpty() && permissionBackend.user(userProvider).test(need).isEmpty()) {
// A permission is required, but test returned no candidates.
return null;
}
} catch (PermissionBackendException err) { } catch (PermissionBackendException err) {
log.error( log.error(
String.format("exception testing view %s.%s", e.getPluginName(), e.getExportName()), err); String.format("exception testing view %s.%s", e.getPluginName(), e.getExportName()), err);
return null; return null;
} }
if (!globalRequired.isEmpty()) {
UiAction.Description dsc = ((UiAction<R>) view).getDescription(resource); PermissionBackend.WithUser withUser = permissionBackend.user(userProvider);
if (dsc == null) { Iterator<GlobalOrPluginPermission> i = globalRequired.iterator();
return null; BooleanCondition p = withUser.testCond(i.next());
while (i.hasNext()) {
p = or(p, withUser.testCond(i.next()));
}
dsc.setVisible(and(p, dsc.getVisibleCondition()));
} }
String name = e.getExportName().substring(d + 1); String name = e.getExportName().substring(d + 1);