You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2019/04/03 11:35:08 UTC
[lucene-solr] branch branch_8x updated (b2e7ab6 -> d5b9fbe)
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a change to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.
from b2e7ab6 Adding 6.6.6 backcompat indexes
new 6891820 SOLR-13355: Small refactors to RuleBasedAuthorizationPlugin
new d5b9fbe SOLR-13355: Obey 'ALL' for handlers with other predefined perms
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../security/RuleBasedAuthorizationPlugin.java | 112 ++++++++++------
.../security/TestRuleBasedAuthorizationPlugin.java | 145 +++++++++++++++++++--
2 files changed, 200 insertions(+), 57 deletions(-)
[lucene-solr] 01/02: SOLR-13355: Small refactors to
RuleBasedAuthorizationPlugin
Posted by ge...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 68918206f56fc7a65ce9b84a9cf6a30edf8ce7c2
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Fri Mar 29 12:10:28 2019 -0400
SOLR-13355: Small refactors to RuleBasedAuthorizationPlugin
---
.../security/RuleBasedAuthorizationPlugin.java | 112 +++++++++++++--------
1 file changed, 70 insertions(+), 42 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java b/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
index 6bf2822..7f97c88 100644
--- a/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
@@ -112,55 +112,83 @@ public class RuleBasedAuthorizationPlugin implements AuthorizationPlugin, Config
private MatchStatus checkPathPerm(List<Permission> permissions, AuthorizationContext context) {
if (permissions == null || permissions.isEmpty()) return MatchStatus.NO_PERMISSIONS_FOUND;
Principal principal = context.getUserPrincipal();
- loopPermissions:
+
+ final Permission governingPermission = findFirstGoverningPermission(permissions, context);
+ if (governingPermission == null) {
+ log.debug("No permissions configured for the resource {} . So allowed to access", context.getResource());
+ return MatchStatus.NO_PERMISSIONS_FOUND;
+ }
+
+ return determineIfPermissionPermitsPrincipal(principal, governingPermission);
+ }
+
+ private Permission findFirstGoverningPermission(List<Permission> permissions, AuthorizationContext context) {
for (int i = 0; i < permissions.size(); i++) {
Permission permission = permissions.get(i);
- if (PermissionNameProvider.values.containsKey(permission.name)) {
- if (context.getHandler() instanceof PermissionNameProvider) {
- PermissionNameProvider handler = (PermissionNameProvider) context.getHandler();
- PermissionNameProvider.Name permissionName = handler.getPermissionName(context);
- if (permissionName == null || !permission.name.equals(permissionName.name)) {
- continue;
- }
- } else {
- //all is special. it can match any
- if(permission.wellknownName != PermissionNameProvider.Name.ALL) continue;
- }
- } else {
- if (permission.method != null && !permission.method.contains(context.getHttpMethod())) {
- //this permissions HTTP method does not match this rule. try other rules
- continue;
- }
- if (permission.params != null) {
- for (Map.Entry<String, Function<String[], Boolean>> e : permission.params.entrySet()) {
- String[] paramVal = context.getParams().getParams(e.getKey());
- if(!e.getValue().apply(paramVal)) continue loopPermissions;
- }
- }
- }
+ if (permissionAppliesToRequest(permission, context)) return permission;
+ }
- if (permission.role == null) {
- //no role is assigned permission.That means everybody is allowed to access
- return MatchStatus.PERMITTED;
- }
- if (principal == null) {
- log.info("request has come without principal. failed permission {} ",permission);
- //this resource needs a principal but the request has come without
- //any credential.
- return MatchStatus.USER_REQUIRED;
- } else if (permission.role.contains("*")) {
- return MatchStatus.PERMITTED;
+ return null;
+ }
+
+ private boolean permissionAppliesToRequest(Permission permission, AuthorizationContext context) {
+ if (PermissionNameProvider.values.containsKey(permission.name)) {
+ return predefinedPermissionAppliesToRequest(permission, context);
+ } else {
+ return customPermissionAppliesToRequest(permission, context);
+ }
+ }
+
+ private boolean predefinedPermissionAppliesToRequest(Permission predefinedPermission, AuthorizationContext context) {
+ if (context.getHandler() instanceof PermissionNameProvider) {
+ PermissionNameProvider handler = (PermissionNameProvider) context.getHandler();
+ PermissionNameProvider.Name permissionName = handler.getPermissionName(context);
+ if (permissionName == null || !predefinedPermission.name.equals(permissionName.name)) {
+ return false;
}
+ } else {
+ //all is special. it can match any
+ if(predefinedPermission.wellknownName != PermissionNameProvider.Name.ALL) return false;
+ }
- for (String role : permission.role) {
- Set<String> userRoles = usersVsRoles.get(principal.getName());
- if (userRoles != null && userRoles.contains(role)) return MatchStatus.PERMITTED;
+ return true;
+ }
+
+ private boolean customPermissionAppliesToRequest(Permission customPermission, AuthorizationContext context) {
+ if (customPermission.method != null && !customPermission.method.contains(context.getHttpMethod())) {
+ //this permissions HTTP method does not match this rule. try other rules
+ return false;
+ }
+ if (customPermission.params != null) {
+ for (Map.Entry<String, Function<String[], Boolean>> e : customPermission.params.entrySet()) {
+ String[] paramVal = context.getParams().getParams(e.getKey());
+ if(!e.getValue().apply(paramVal)) return false;
}
- log.info("This resource is configured to have a permission {}, The principal {} does not have the right role ", permission, principal);
- return MatchStatus.FORBIDDEN;
}
- log.debug("No permissions configured for the resource {} . So allowed to access", context.getResource());
- return MatchStatus.NO_PERMISSIONS_FOUND;
+
+ return true;
+ }
+
+ private MatchStatus determineIfPermissionPermitsPrincipal(Principal principal, Permission governingPermission) {
+ if (governingPermission.role == null) {
+ //no role is assigned permission.That means everybody is allowed to access
+ return MatchStatus.PERMITTED;
+ }
+ if (principal == null) {
+ log.info("request has come without principal. failed permission {} ", governingPermission);
+ //this resource needs a principal but the request has come without
+ //any credential.
+ return MatchStatus.USER_REQUIRED;
+ } else if (governingPermission.role.contains("*")) {
+ return MatchStatus.PERMITTED;
+ }
+
+ for (String role : governingPermission.role) {
+ Set<String> userRoles = usersVsRoles.get(principal.getName());
+ if (userRoles != null && userRoles.contains(role)) return MatchStatus.PERMITTED;
+ }
+ log.info("This resource is configured to have a permission {}, The principal {} does not have the right role ", governingPermission, principal);
+ return MatchStatus.FORBIDDEN;
}
@Override
[lucene-solr] 02/02: SOLR-13355: Obey 'ALL' for handlers with other
predefined perms
Posted by ge...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit d5b9fbee374e83045613fba5cb87bd550afcfa2b
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Fri Mar 29 12:36:30 2019 -0400
SOLR-13355: Obey 'ALL' for handlers with other predefined perms
Prior to this commit, RuleBasedAuthorizationPlugin would check for the
predefined 'ALL' permission only when the endpoint being hit wasn't
associated with another predefined-permission.
This resulted in some very unintuitive behavior. For example, the
permission {name:all, role:admin} would correctly prevent a
role:foo user from accessing /admin/info/properties, but would allow
write access to /admin/authorization because of the SECURITY_EDIT
predefined perm associated with that endpoint.
This commit fixes this bug so that the 'all' permission is always
consulted whether or not the endpoint is associated with other predefined
permissions.
---
.../security/RuleBasedAuthorizationPlugin.java | 16 +--
.../security/TestRuleBasedAuthorizationPlugin.java | 145 +++++++++++++++++++--
2 files changed, 138 insertions(+), 23 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java b/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
index 7f97c88..8b4a65c 100644
--- a/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
+++ b/solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
@@ -140,18 +140,16 @@ public class RuleBasedAuthorizationPlugin implements AuthorizationPlugin, Config
}
private boolean predefinedPermissionAppliesToRequest(Permission predefinedPermission, AuthorizationContext context) {
- if (context.getHandler() instanceof PermissionNameProvider) {
+ if (predefinedPermission.wellknownName == PermissionNameProvider.Name.ALL) {
+ return true; //'ALL' applies to everything!
+ } else if (! (context.getHandler() instanceof PermissionNameProvider)) {
+ return false; // We're not 'ALL', and the handler isn't associated with any other predefined permissions
+ } else {
PermissionNameProvider handler = (PermissionNameProvider) context.getHandler();
PermissionNameProvider.Name permissionName = handler.getPermissionName(context);
- if (permissionName == null || !predefinedPermission.name.equals(permissionName.name)) {
- return false;
- }
- } else {
- //all is special. it can match any
- if(predefinedPermission.wellknownName != PermissionNameProvider.Name.ALL) return false;
- }
- return true;
+ return permissionName != null && predefinedPermission.name.equals(permissionName.name);
+ }
}
private boolean customPermissionAppliesToRequest(Permission customPermission, AuthorizationContext context) {
diff --git a/solr/core/src/test/org/apache/solr/security/TestRuleBasedAuthorizationPlugin.java b/solr/core/src/test/org/apache/solr/security/TestRuleBasedAuthorizationPlugin.java
index bacfc10..4e1e9aa 100644
--- a/solr/core/src/test/org/apache/solr/security/TestRuleBasedAuthorizationPlugin.java
+++ b/solr/core/src/test/org/apache/solr/security/TestRuleBasedAuthorizationPlugin.java
@@ -38,11 +38,17 @@ import org.apache.solr.handler.SchemaHandler;
import org.apache.solr.handler.UpdateRequestHandler;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.handler.admin.CoreAdminHandler;
+import org.apache.solr.handler.admin.PropertiesRequestHandler;
import org.apache.solr.handler.component.SearchHandler;
+import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.security.AuthorizationContext.CollectionRequest;
import org.apache.solr.security.AuthorizationContext.RequestType;
import org.apache.solr.common.util.CommandOperation;
+import org.hamcrest.core.IsInstanceOf;
+import org.hamcrest.core.IsNot;
+import org.junit.Test;
+import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static org.apache.solr.common.util.Utils.getObjectByPath;
@@ -50,6 +56,10 @@ import static org.apache.solr.common.util.Utils.makeMap;
import static org.apache.solr.common.util.CommandOperation.captureErrors;
public class TestRuleBasedAuthorizationPlugin extends SolrTestCaseJ4 {
+ private static final int STATUS_OK = 200;
+ private static final int FORBIDDEN = 403;
+ private static final int PROMPT_FOR_CREDENTIALS = 401;
+
String permissions = "{" +
" user-role : {" +
" steve: [dev,user]," +
@@ -75,10 +85,6 @@ public class TestRuleBasedAuthorizationPlugin extends SolrTestCaseJ4 {
public void testBasicPermissions() {
- int STATUS_OK = 200;
- int FORBIDDEN = 403;
- int PROMPT_FOR_CREDENTIALS = 401;
-
checkRules(makeMap("resource", "/update/json/docs",
"httpMethod", "POST",
"userPrincipal", "unknownuser",
@@ -322,6 +328,127 @@ public class TestRuleBasedAuthorizationPlugin extends SolrTestCaseJ4 {
"]}"));
}
+ /*
+ * RuleBasedAuthorizationPlugin handles requests differently based on whether the underlying handler implements
+ * PermissionNameProvider or not. If this test fails because UpdateRequestHandler stops implementing
+ * PermissionNameProvider, or PropertiesRequestHandler starts to, then just change the handlers used here.
+ */
+ @Test
+ public void testAllPermissionAllowsActionsWhenUserHasCorrectRole() {
+ SolrRequestHandler handler = new UpdateRequestHandler();
+ assertThat(handler, new IsInstanceOf(PermissionNameProvider.class));
+ checkRules(makeMap("resource", "/update",
+ "userPrincipal", "dev",
+ "requestType", RequestType.UNKNOWN,
+ "collectionRequests", "go",
+ "handler", handler,
+ "params", new MapSolrParams(singletonMap("key", "VAL2")))
+ , STATUS_OK, (Map<String, Object>) Utils.fromJSONString( "{" +
+ " user-role:{" +
+ " dev:[dev_role]," +
+ " admin:[admin_role]}," +
+ " permissions:[" +
+ " {name:all, role:[dev_role, admin_role]}" +
+ "]}"));
+
+ handler = new PropertiesRequestHandler();
+ assertThat(handler, new IsNot<>(new IsInstanceOf(PermissionNameProvider.class)));
+ checkRules(makeMap("resource", "/admin/info/properties",
+ "userPrincipal", "dev",
+ "requestType", RequestType.UNKNOWN,
+ "collectionRequests", "go",
+ "handler", handler,
+ "params", new MapSolrParams(emptyMap()))
+ , STATUS_OK, (Map<String, Object>) Utils.fromJSONString( "{" +
+ " user-role:{" +
+ " dev:[dev_role]," +
+ " admin:[admin_role]}," +
+ " permissions:[" +
+ " {name:all, role:[dev_role, admin_role]}" +
+ "]}"));
+ }
+
+
+ /*
+ * RuleBasedAuthorizationPlugin handles requests differently based on whether the underlying handler implements
+ * PermissionNameProvider or not. If this test fails because UpdateRequestHandler stops implementing
+ * PermissionNameProvider, or PropertiesRequestHandler starts to, then just change the handlers used here.
+ */
+ @Test
+ public void testAllPermissionAllowsActionsWhenAssociatedRoleIsWildcard() {
+ SolrRequestHandler handler = new UpdateRequestHandler();
+ assertThat(handler, new IsInstanceOf(PermissionNameProvider.class));
+ checkRules(makeMap("resource", "/update",
+ "userPrincipal", "dev",
+ "requestType", RequestType.UNKNOWN,
+ "collectionRequests", "go",
+ "handler", new UpdateRequestHandler(),
+ "params", new MapSolrParams(singletonMap("key", "VAL2")))
+ , STATUS_OK, (Map<String, Object>) Utils.fromJSONString( "{" +
+ " user-role:{" +
+ " dev:[dev_role]," +
+ " admin:[admin_role]}," +
+ " permissions:[" +
+ " {name:all, role:'*'}" +
+ "]}"));
+
+ handler = new PropertiesRequestHandler();
+ assertThat(handler, new IsNot<>(new IsInstanceOf(PermissionNameProvider.class)));
+ checkRules(makeMap("resource", "/admin/info/properties",
+ "userPrincipal", "dev",
+ "requestType", RequestType.UNKNOWN,
+ "collectionRequests", "go",
+ "handler", handler,
+ "params", new MapSolrParams(emptyMap()))
+ , STATUS_OK, (Map<String, Object>) Utils.fromJSONString( "{" +
+ " user-role:{" +
+ " dev:[dev_role]," +
+ " admin:[admin_role]}," +
+ " permissions:[" +
+ " {name:all, role:'*'}" +
+ "]}"));
+ }
+
+ /*
+ * RuleBasedAuthorizationPlugin handles requests differently based on whether the underlying handler implements
+ * PermissionNameProvider or not. If this test fails because UpdateRequestHandler stops implementing
+ * PermissionNameProvider, or PropertiesRequestHandler starts to, then just change the handlers used here.
+ */
+ @Test
+ public void testAllPermissionDeniesActionsWhenUserIsNotCorrectRole() {
+ SolrRequestHandler handler = new UpdateRequestHandler();
+ assertThat(handler, new IsInstanceOf(PermissionNameProvider.class));
+ checkRules(makeMap("resource", "/update",
+ "userPrincipal", "dev",
+ "requestType", RequestType.UNKNOWN,
+ "collectionRequests", "go",
+ "handler", new UpdateRequestHandler(),
+ "params", new MapSolrParams(singletonMap("key", "VAL2")))
+ , FORBIDDEN, (Map<String, Object>) Utils.fromJSONString( "{" +
+ " user-role:{" +
+ " dev:[dev_role]," +
+ " admin:[admin_role]}," +
+ " permissions:[" +
+ " {name:all, role:'admin_role'}" +
+ "]}"));
+
+ handler = new PropertiesRequestHandler();
+ assertThat(handler, new IsNot<>(new IsInstanceOf(PermissionNameProvider.class)));
+ checkRules(makeMap("resource", "/admin/info/properties",
+ "userPrincipal", "dev",
+ "requestType", RequestType.UNKNOWN,
+ "collectionRequests", "go",
+ "handler", handler,
+ "params", new MapSolrParams(emptyMap()))
+ , FORBIDDEN, (Map<String, Object>) Utils.fromJSONString( "{" +
+ " user-role:{" +
+ " dev:[dev_role]," +
+ " admin:[admin_role]}," +
+ " permissions:[" +
+ " {name:all, role:'admin_role'}" +
+ "]}"));
+ }
+
public void testEditRules() throws IOException {
Perms perms = new Perms();
perms.runCmd("{set-permission : {name: config-edit, role: admin } }", true);
@@ -455,14 +582,4 @@ public class TestRuleBasedAuthorizationPlugin extends SolrTestCaseJ4 {
return (String) values.get("resource");
}
}
-
-static String testPerms = "{user-role:{" +
- " admin:[admin_role]," +
- " update:[update_role]," +
- " solr:[read_role]}," +
- " permissions:[" +
- " {name:update,role:[admin_role,update_role]}," +
- " {name:read,role:[admin_role,update_role,read_role]" +
- "]}";
-
}