dimas-b commented on code in PR #2925:
URL: https://github.com/apache/polaris/pull/2925#discussion_r2470044888
##########
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))
Review Comment:
I agree that from the `polaris-core` perspective using `Principal` roles is
preferable to using `securityContext::isUserInRole` as it reduces the scope of
components that need to be in sync WRT auth data (note: `SecurityContext` is
well-defined only for Web / REST applications).
--
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]
