You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "janhoy (via GitHub)" <gi...@apache.org> on 2023/02/15 09:14:59 UTC

[GitHub] [solr] janhoy opened a new pull request, #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

janhoy opened a new pull request, #1359:
URL: https://github.com/apache/solr/pull/1359

   https://issues.apache.org/jira/browse/SOLR-16658
   
   It's a trappy design to allow "null" keys in maps like this. And to define a "null" role in a permission in Admin UI security editor you need to actively de-select all roles, which took me a while to figure. Not trying to fix this here, but at least trying to resolve correct set of permissions for a given user, taking into account that permissions may have `*` or `null` roles.
   
   I discovered this bug when working with a quite open-ended security.json for JWT, where some endpoints don't require authentication and all others do require auth, but we don't care what role the user has, e.g.
   
   ```json
   "permissions":[
     {
       "name": "info",
       "role": null,
       "collection": null,
       "path": "/admin/info/system"
     },
     {
       "name": "health",
       "role": null
     },
     {
       "name": "all",
       "role": "*"
     }
   ]
   ```


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] ozlerhakan commented on pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "ozlerhakan (via GitHub)" <gi...@apache.org>.
ozlerhakan commented on PR #1359:
URL: https://github.com/apache/solr/pull/1359#issuecomment-1494008705

   Hi @janhoy and @epugh , 
   
   After we have upgraded our solr cloud to solr 9.2 cloud, which runs on k8s using the solr operator, this changes brings an exception as we follow this security.json mentioned at https://apache.github.io/solr-operator/docs/solr-cloud/solr-cloud-crd.html#authorization. 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy merged pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy merged PR #1359:
URL: https://github.com/apache/solr/pull/1359


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a diff in pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1359:
URL: https://github.com/apache/solr/pull/1359#discussion_r1154744612


##########
solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java:
##########
@@ -339,7 +341,10 @@ public SimpleOrderedMap<Object> getSecurityInfo(SolrQueryRequest req) {
         RuleBasedAuthorizationPluginBase rbap = (RuleBasedAuthorizationPluginBase) auth;
         Set<String> roles = rbap.getUserRoles(req.getUserPrincipal());
         info.add("roles", roles);
-        info.add("permissions", rbap.getPermissionNamesForRoles(roles));
+        info.add(
+            "permissions",
+            rbap.getPermissionNamesForRoles(
+                Stream.concat(roles.stream(), Stream.of("*", null)).collect(Collectors.toSet())));

Review Comment:
   https://issues.apache.org/jira/browse/SOLR-16729



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a diff in pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1359:
URL: https://github.com/apache/solr/pull/1359#discussion_r1106858436


##########
solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java:
##########
@@ -339,7 +341,10 @@ public SimpleOrderedMap<Object> getSecurityInfo(SolrQueryRequest req) {
         RuleBasedAuthorizationPluginBase rbap = (RuleBasedAuthorizationPluginBase) auth;
         Set<String> roles = rbap.getUserRoles(req.getUserPrincipal());
         info.add("roles", roles);
-        info.add("permissions", rbap.getPermissionNamesForRoles(roles));
+        info.add(
+            "permissions",
+            rbap.getPermissionNamesForRoles(
+                Stream.concat(roles.stream(), Stream.of("*", null)).collect(Collectors.toSet())));

Review Comment:
   I considered letting `getPermissionNamesForRoles` always consider `*` and `null` roles, since this is the only place it is used, but the name of the method suggests that you want permissions for exactly those roles, so I expanded on the caller side instead...



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] tflobbe commented on a diff in pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "tflobbe (via GitHub)" <gi...@apache.org>.
tflobbe commented on code in PR #1359:
URL: https://github.com/apache/solr/pull/1359#discussion_r1153902800


##########
solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java:
##########
@@ -339,7 +341,10 @@ public SimpleOrderedMap<Object> getSecurityInfo(SolrQueryRequest req) {
         RuleBasedAuthorizationPluginBase rbap = (RuleBasedAuthorizationPluginBase) auth;
         Set<String> roles = rbap.getUserRoles(req.getUserPrincipal());
         info.add("roles", roles);
-        info.add("permissions", rbap.getPermissionNamesForRoles(roles));
+        info.add(
+            "permissions",
+            rbap.getPermissionNamesForRoles(
+                Stream.concat(roles.stream(), Stream.of("*", null)).collect(Collectors.toSet())));

Review Comment:
   I'm hitting a NPE with Solr 9.2 in this line. I believe the issue is that `roles` can actually be null, if the user has no roles defined. Unfortunately, this is the case for the `$` user, that we use in `PKIAuthenticationPlugin`, this this causes the `/solr/admin/info/system` API not to work when using `nodes` (which the UI uses to populate the Nodes tab). 



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a diff in pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1359:
URL: https://github.com/apache/solr/pull/1359#discussion_r1106858436


##########
solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java:
##########
@@ -339,7 +341,10 @@ public SimpleOrderedMap<Object> getSecurityInfo(SolrQueryRequest req) {
         RuleBasedAuthorizationPluginBase rbap = (RuleBasedAuthorizationPluginBase) auth;
         Set<String> roles = rbap.getUserRoles(req.getUserPrincipal());
         info.add("roles", roles);
-        info.add("permissions", rbap.getPermissionNamesForRoles(roles));
+        info.add(
+            "permissions",
+            rbap.getPermissionNamesForRoles(
+                Stream.concat(roles.stream(), Stream.of("*", null)).collect(Collectors.toSet())));

Review Comment:
   I considered letting `getPermissionNamesForRoles` always consider `*` and `null` roles, since this is the only place it is used, but the name of the method suggests that you want permissions for exactly those roles, so I left it generic and instead expanded on the JavaDoc for the method and appended the two special roles on the caller side...



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] tflobbe commented on a diff in pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "tflobbe (via GitHub)" <gi...@apache.org>.
tflobbe commented on code in PR #1359:
URL: https://github.com/apache/solr/pull/1359#discussion_r1154757112


##########
solr/core/src/java/org/apache/solr/handler/admin/SystemInfoHandler.java:
##########
@@ -339,7 +341,10 @@ public SimpleOrderedMap<Object> getSecurityInfo(SolrQueryRequest req) {
         RuleBasedAuthorizationPluginBase rbap = (RuleBasedAuthorizationPluginBase) auth;
         Set<String> roles = rbap.getUserRoles(req.getUserPrincipal());
         info.add("roles", roles);
-        info.add("permissions", rbap.getPermissionNamesForRoles(roles));
+        info.add(
+            "permissions",
+            rbap.getPermissionNamesForRoles(
+                Stream.concat(roles.stream(), Stream.of("*", null)).collect(Collectors.toSet())));

Review Comment:
   Ugh, sorry, I created SOLR-16730



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on pull request #1359: SOLR-16658 List of permissions returned to Admin UI is not complete

Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1359:
URL: https://github.com/apache/solr/pull/1359#issuecomment-1494356950

   See https://issues.apache.org/jira/browse/SOLR-16730


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org