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:10 UTC

[lucene-solr] 02/02: SOLR-13355: Obey 'ALL' for handlers with other predefined perms

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]" +
-    "]}";
-
 }