janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418962495



##########
File path: 
solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java
##########
@@ -43,47 +45,61 @@
 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.CommandOperation.captureErrors;
 import static org.apache.solr.common.util.Utils.getObjectByPath;
 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]," +
-      "    tim: [dev,admin]," +
-      "    joe: [user]," +
-      "    noble:[dev,user]" +
-      "  }," +
-      "  permissions : [" +
-      "    {name:'schema-edit'," +
-      "     role:admin}," +
-      "    {name:'collection-admin-read'," +
-      "    role:null}," +
-      "    {name:collection-admin-edit ," +
-      "    role:admin}," +
-      "    {name:mycoll_update," +
-      "      collection:mycoll," +
-      "      path:'/update/*'," +
-      "      role:[dev,admin]" +
-      "    }," +
-      "{name:read , role:dev }," +
-      "{name:freeforall, path:'/foo', role:'*'}]}";
 
+/**
+ * Base class for testing RBAC. This will test the {@link 
RuleBasedAuthorizationPlugin} implementation
+ * but also serves as a base class for testing other sub classes
+ */
+@SuppressWarnings("unchecked")
+public class BaseTestRuleBasedAuthorizationPlugin extends SolrTestCaseJ4 {

Review comment:
       This test baseclass is not refactored 100%, as it tests the json 
user-role mapping while the test subclass overrides stuff to tests the external 
roles...

##########
File path: solr/solr-ref-guide/src/securing-solr.adoc
##########
@@ -53,6 +53,7 @@ Authorization makes sure that only users with the necessary 
roles/permissions ca
 
 // tag::list-of-authorization-plugins[]
 * 
<<rule-based-authorization-plugin.adoc#rule-based-authorization-plugin,Rule-Based
 Authorization Plugin>>
+* 
<<rule-based-authorization-plugin.adoc#rule-based-authorization-plugin,External 
Role Rule-Based Authorization Plugin>>

Review comment:
       Did not find a better sub header to link to

##########
File path: 
solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java
##########
@@ -440,13 +431,29 @@ public void 
testAllPermissionDeniesActionsWhenUserIsNotCorrectRole() {
         "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'}" +
-            "]}"));
+        , FORBIDDEN);
+  }
+
+  void addPermission(String permissionName, String role, String path, 
Map<String, Object> params) {

Review comment:
       The test class is based on the old test, but I refactored it by adding 
the `addPermission`, `removePermission`, `clearUserRoles`, `setUserRole` 
methods for use in the test




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to