Description
Currently the RuleBasedAuthorizationPlugin relies on explicitly mapping users to roles. However, when users are authenticated by an external Identity service (e.g. JWT as implemented in SOLR-12121), that external service keeps track of the user's roles, and will pass that as a "claim" in the token (JWT).
In order for Solr to be able to Authorise requests based on those roles, the Authorization plugin should be able to accept (verified) roles from the request instead of explicit mapping.
Suggested approach is to create a new interface VerifiedUserRoles and a PrincipalWithUserRoles which implements the interface. The Authorization plugin can then pull the roles from request. By piggy-backing on the Principal, we have a seamless way to transfer extra external information, and there is also a natural relationship:
User Authentication -> Role validation -> Creating a Principal
I plan to add the interface, the custom Principal class and restructure RuleBasedAuthorizationPlugin in an abstract base class and two implementations: RuleBasedAuthorizationPlugin (as today) and a new ExternalRoleRuleBasedAuthorizationPlugin.
Attachments
Issue Links
- Is contained by
-
SOLR-12121 JWT Authentication plugin
- Resolved
- relates to
-
SOLR-12121 JWT Authentication plugin
- Resolved
- links to
Activity
-
- Time Spent:
- 10m
-
GitHub user janhoy opened a pull request:
https://github.com/apache/lucene-solr/pull/341
SOLR-12131: ExternalRoleRuleBasedAuthorizationPlugin
...which gets user's roles from request
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cominvent/lucene-solr solr12131-jwt-autz
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/341.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #341
----
----
-
- Time Spent:
- 10m
-
janhoy commented on pull request #341:
SOLR-12131: ExternalRoleRuleBasedAuthorizationPlugin
URL: https://github.com/apache/lucene-solr/pull/341
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on issue #341:
SOLR-12131: ExternalRoleRuleBasedAuthorizationPlugin
URL: https://github.com/apache/lucene-solr/pull/341#issuecomment-455226804
Closing, will be implemented inSOLR-12121
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on issue #341:
SOLR-12131: ExternalRoleRuleBasedAuthorizationPlugin
URL: https://github.com/apache/lucene-solr/pull/341#issuecomment-455227661
Sorry, reopening as only part of this will be part ofSOLR-12121, the new Authz plugin will still be committed in this issue
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on pull request #341:
SOLR-12131: ExternalRoleRuleBasedAuthorizationPlugin
URL: https://github.com/apache/lucene-solr/pull/341
...which gets user's roles from request
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
-
- Time Spent:
- 10m
-
madrob commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418632788
##########
File path: solr/core/src/java/org/apache/solr/security/PrincipalWithUserRoles.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.Serializable;
+import java.security.Principal;
+import java.util.Set;
+
+import org.apache.http.util.Args;
+
+/**
+ * Type of Principal object that can contain also a list of roles the user has.
+ * One use case can be to keep track of user-role mappings in an Identity Server
+ * external to Solr and pass the information to Solr in a signed JWT token or in
+ * another secure manner. The role information can then be used to authorize
+ * requests without the need to maintain or lookup what roles each user belongs to.
+ */
+public class PrincipalWithUserRoles implements Principal, VerifiedUserRoles, Serializable {
Review comment:
Could this class be `final`? Also, why Serializable?
##########
File path: solr/solr-ref-guide/src/rule-based-authorization-plugin.adoc
##########
@@ -64,6 +66,34 @@ There are several things defined in this example:
<5> The 'admin' role has been defined, and it has permission to edit security settings.
<6> The 'solr' user has been defined to the 'admin' role.
+=== Example for ExternalRoleRuleBasedAuthorizationPlugin
+This example `security.json` shows how an imagined `JWTAuthPlugin`, which pulls user and user roles from JWT claims, can work with the `ExternalRoleRuleBasedAuthorizationPlugin` plugin:
Review comment:
s/imagined//, this exists, right?
##########
File path: solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPluginBase.java
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+
+import org.apache.solr.common.SpecProvider;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.Utils;
+import org.apache.solr.common.util.ValidatingJsonMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Collections.unmodifiableMap;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+import static org.apache.solr.handler.admin.SecurityConfHandler.getListValue;
+
+/**
+ * Base class for rule based authorization plugins
+ */
+public abstract class RuleBasedAuthorizationPluginBase implements AuthorizationPlugin, ConfigEditablePlugin, SpecProvider {
Review comment:
Is this largely a copy of the old RuleBasedAuthorizationPlugin? If you can let me know which parts were changed, it will be easier to review.
##########
File path: solr/core/src/java/org/apache/solr/security/PrincipalWithUserRoles.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.Serializable;
+import java.security.Principal;
+import java.util.Set;
+
+import org.apache.http.util.Args;
+
+/**
+ * Type of Principal object that can contain also a list of roles the user has.
+ * One use case can be to keep track of user-role mappings in an Identity Server
+ * external to Solr and pass the information to Solr in a signed JWT token or in
+ * another secure manner. The role information can then be used to authorize
+ * requests without the need to maintain or lookup what roles each user belongs to.
+ */
+public class PrincipalWithUserRoles implements Principal, VerifiedUserRoles, Serializable {
+ private static final long serialVersionUID = 4144666467522831388L;
+ private final String username;
+
+ private final Set<String> roles;
+
+ /**
+ * User principal with user name as well as one or more roles that he/she belong to
+ * @param username string with user name for user
+ * @param roles a set of roles that we know this user belongs to, or empty list for no roles
+ */
+ public PrincipalWithUserRoles(final String username, Set<String> roles) {
+ super();
+ Args.notNull(username, "User name");
Review comment:
nit: prefer `Objects.requireNonNull`
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418721727
##########
File path: solr/core/src/java/org/apache/solr/security/PrincipalWithUserRoles.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.Serializable;
+import java.security.Principal;
+import java.util.Set;
+
+import org.apache.http.util.Args;
+
+/**
+ * Type of Principal object that can contain also a list of roles the user has.
+ * One use case can be to keep track of user-role mappings in an Identity Server
+ * external to Solr and pass the information to Solr in a signed JWT token or in
+ * another secure manner. The role information can then be used to authorize
+ * requests without the need to maintain or lookup what roles each user belongs to.
+ */
+public class PrincipalWithUserRoles implements Principal, VerifiedUserRoles, Serializable {
Review comment:
This class is in test scope, so no big deal. I removed Serializable though.
Real world Principal classes would implement VerifiedUserRoles directly instead of having to use or subclass this one
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418722327
##########
File path: solr/core/src/java/org/apache/solr/security/PrincipalWithUserRoles.java
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.Serializable;
+import java.security.Principal;
+import java.util.Set;
+
+import org.apache.http.util.Args;
+
+/**
+ * Type of Principal object that can contain also a list of roles the user has.
+ * One use case can be to keep track of user-role mappings in an Identity Server
+ * external to Solr and pass the information to Solr in a signed JWT token or in
+ * another secure manner. The role information can then be used to authorize
+ * requests without the need to maintain or lookup what roles each user belongs to.
+ */
+public class PrincipalWithUserRoles implements Principal, VerifiedUserRoles, Serializable {
+ private static final long serialVersionUID = 4144666467522831388L;
+ private final String username;
+
+ private final Set<String> roles;
+
+ /**
+ * User principal with user name as well as one or more roles that he/she belong to
+ * @param username string with user name for user
+ * @param roles a set of roles that we know this user belongs to, or empty list for no roles
+ */
+ public PrincipalWithUserRoles(final String username, Set<String> roles) {
+ super();
+ Args.notNull(username, "User name");
Review comment:
Fixed
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418725066
##########
File path: solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPluginBase.java
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+
+import org.apache.solr.common.SpecProvider;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.Utils;
+import org.apache.solr.common.util.ValidatingJsonMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Collections.unmodifiableMap;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+import static org.apache.solr.handler.admin.SecurityConfHandler.getListValue;
+
+/**
+ * Base class for rule based authorization plugins
+ */
+public abstract class RuleBasedAuthorizationPluginBase implements AuthorizationPlugin, ConfigEditablePlugin, SpecProvider {
Review comment:
I brought it up to date now. The base class is a copy of the old RBAC class, minus the user-roles map. Instead it delegates the role fecthing to the sub classes.
The new RuleBasedAuthorizationPlugin.java is now very slim, mainly dealing with parsing roles from security.json.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
madrob commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418758313
##########
File path: solr/core/src/test/org/apache/solr/security/PrincipalWithUserRoles.java
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.security.Principal;
+import java.util.Objects;
+import java.util.Set;
+
+import org.apache.http.util.Args;
Review comment:
nit: I think precommit will fail on this unused import.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#issuecomment-622592651
> Wait, what happened to the ref guide changes?
Hmm, will try to get them back
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418774258
##########
File path: solr/solr-ref-guide/src/rule-based-authorization-plugin.adoc
##########
@@ -64,6 +66,34 @@ There are several things defined in this example:
<5> The 'admin' role has been defined, and it has permission to edit security settings.
<6> The 'solr' user has been defined to the 'admin' role.
+=== Example for ExternalRoleRuleBasedAuthorizationPlugin
+This example `security.json` shows how an imagined `JWTAuthPlugin`, which pulls user and user roles from JWT claims, can work with the `ExternalRoleRuleBasedAuthorizationPlugin` plugin:
Review comment:
I brought back some of the refguide changes from the original patch. Please review again.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#issuecomment-622957422
I believe it is up to date with master now.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r418962126
##########
File path: solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPluginBase.java
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+
+import org.apache.solr.common.SpecProvider;
+import org.apache.solr.common.util.CommandOperation;
+import org.apache.solr.common.util.Utils;
+import org.apache.solr.common.util.ValidatingJsonMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Collections.unmodifiableMap;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+import static org.apache.solr.handler.admin.SecurityConfHandler.getListValue;
+
+/**
+ * Base class for rule based authorization plugins
+ */
+public abstract class RuleBasedAuthorizationPluginBase implements AuthorizationPlugin, ConfigEditablePlugin, SpecProvider {
Review comment:
We could of course have kept one RBAC class, made the user-group mapping optional and always checked for roles on Principal, but I like the subclass approach better.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r419163646
##########
File path: solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
##########
@@ -16,329 +16,45 @@
*/
package org.apache.solr.security;
-import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.security.Principal;
-import java.util.ArrayList;
import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.function.Function;
-import org.apache.solr.common.SpecProvider;
-import org.apache.solr.common.util.Utils;
-import org.apache.solr.common.util.ValidatingJsonMap;
-import org.apache.solr.common.util.CommandOperation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static java.util.Arrays.asList;
-import static java.util.Collections.unmodifiableMap;
-import static java.util.function.Function.identity;
-import static java.util.stream.Collectors.toMap;
-import static org.apache.solr.handler.admin.SecurityConfHandler.getListValue;
import static org.apache.solr.handler.admin.SecurityConfHandler.getMapValue;
-
-public class RuleBasedAuthorizationPlugin implements AuthorizationPlugin, ConfigEditablePlugin, SpecProvider {
+/**
+ * Original implementation of Rule Based Authz plugin which configures user/role
+ * mapping in the security.json configuration
+ */
+public class RuleBasedAuthorizationPlugin extends RuleBasedAuthorizationPluginBase {
Review comment:
Should this plugin have a new name, e.g. `ExplicitRolesRuleBasedAuthorizationPlugin`? So far I just kept the name to stay 100% back compat.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
madrob commented on a change in pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#discussion_r419480359
##########
File path: solr/core/src/java/org/apache/solr/security/RuleBasedAuthorizationPlugin.java
##########
@@ -16,329 +16,45 @@
*/
package org.apache.solr.security;
-import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.security.Principal;
-import java.util.ArrayList;
import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.function.Function;
-import org.apache.solr.common.SpecProvider;
-import org.apache.solr.common.util.Utils;
-import org.apache.solr.common.util.ValidatingJsonMap;
-import org.apache.solr.common.util.CommandOperation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static java.util.Arrays.asList;
-import static java.util.Collections.unmodifiableMap;
-import static java.util.function.Function.identity;
-import static java.util.stream.Collectors.toMap;
-import static org.apache.solr.handler.admin.SecurityConfHandler.getListValue;
import static org.apache.solr.handler.admin.SecurityConfHandler.getMapValue;
-
-public class RuleBasedAuthorizationPlugin implements AuthorizationPlugin, ConfigEditablePlugin, SpecProvider {
+/**
+ * Original implementation of Rule Based Authz plugin which configures user/role
+ * mapping in the security.json configuration
+ */
+public class RuleBasedAuthorizationPlugin extends RuleBasedAuthorizationPluginBase {
Review comment:
Yea, planning for back compat is fine, I think.
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
madrob commented on pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#issuecomment-623502714
LGTM
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
madrob commented on pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341#issuecomment-628226952
@janhoy Do you want to push this? I'm starting to work onSOLR-10814and would like to be able to build on top of the great work you've already done here!
----------------------------------------------------------------
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:
users@infra.apache.org
-
- Time Spent:
- 10m
-
janhoy merged pull request #341:
URL: https://github.com/apache/lucene-solr/pull/341
----------------------------------------------------------------
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:
users@infra.apache.org