dimas-b commented on code in PR #2925:
URL: https://github.com/apache/polaris/pull/2925#discussion_r2470034530
##########
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:
It is generally the responsibility of the `Authenticator` to provide role
names. However, given that it is a pluggable component and Polaris supports
identity federation, core code should be resilient to roles being not present
in the Polaris database.
I do not think the resolver should fail in this case. `PolarisAuthorizer`
may deny the request if the unresolved role is critical for reaching the
necessary permissions, though.
--
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]
