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