adutra commented on code in PR #708:
URL: https://github.com/apache/polaris/pull/708#discussion_r1912463369


##########
dropwizard/service/src/main/java/org/apache/polaris/service/dropwizard/auth/PolarisPrincipalRoleSecurityContextProvider.java:
##########
@@ -53,6 +54,10 @@ public void filter(ContainerRequestContext requestContext) 
throws IOException {
   public SecurityContext createSecurityContext(
       SecurityContext ctx, AuthenticatedPolarisPrincipal principal) {
     Set<String> validRoleNames = activeRolesProvider.getActiveRoles(principal);
+    if (validRoleNames.isEmpty()) {

Review Comment:
   This is making the tests fail, and I think we do have a problem.  
`DefaultActiveRolesProvider` expects the principal to have the role 
`PRINCIPAL_ROLE_ALL` when all roles are activated:
   
   ```java
   boolean allRoles = 
tokenRoles.contains(BasePolarisAuthenticator.PRINCIPAL_ROLE_ALL);
   ```
   
   But `BasePolarisAuhtenticator` creates a principal with an empty roles set 
in that case. You need to change `BasePolarisAuthenticator.getPrincipal()`:
   
   ```java
     if (tokenInfo.getScope() != null) {
         if (tokenInfo.getScope().equals(PRINCIPAL_ROLE_ALL)) {
           activatedPrincipalRoles.add(PRINCIPAL_ROLE_ALL); // this is missing
         } else {
           activatedPrincipalRoles.addAll(...);
         }
       }
   ```
   
   



-- 
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-unsubscr...@polaris.apache.org

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

Reply via email to