XN137 commented on code in PR #2925:
URL: https://github.com/apache/polaris/pull/2925#discussion_r2469964677


##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java:
##########
@@ -791,22 +788,22 @@ private List<ResolvedPolarisEntity> 
resolveAllPrincipalRoles(
                     PolarisEntityType.PRINCIPAL_ROLE,
                     PolarisEntityConstants.getRootEntityId(),
                     gr.getSecurableId()))
+        .filter(Objects::nonNull)
         .collect(Collectors.toList());
   }
 
   /**
-   * Resolve the specified list of principal roles. The SecurityContext is 
used to determine whether
-   * the principal actually has the roles specified.
+   * Resolve the specified list of principal roles. The PolarisPrincipal is 
used to determine
+   * whether the principal actually has the roles specified.
    *
-   * @param toValidate
-   * @param roleNames
    * @return the filtered list of resolved principal roles
    */
   private List<ResolvedPolarisEntity> resolvePrincipalRolesByName(
       List<ResolvedPolarisEntity> toValidate, Set<String> roleNames) {
     return roleNames.stream()
-        .filter(securityContext::isUserInRole)
+        .filter(roleName -> polarisPrincipal.getRoles().contains(roleName))
         .map(roleName -> resolveByName(toValidate, 
PolarisEntityType.PRINCIPAL_ROLE, roleName))
+        .filter(Objects::nonNull)

Review Comment:
   
https://github.com/apache/polaris/blob/2c77fbf530a7fa3e31b1001713bab47007c65a63/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java#L182-L183
   
   this test was failing without null filtering... tbh i am not sure i 
understand it... its creating a Principal with an invalid role... shouldnt the 
Resolver be unsuccessful in that case?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to