adutra commented on code in PR #1397:
URL: https://github.com/apache/polaris/pull/1397#discussion_r2060400365
##########
service/common/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java:
##########
@@ -88,17 +92,19 @@ protected Optional<AuthenticatedPolarisPrincipal>
getPrincipal(DecodedToken toke
.log("Unable to authenticate user with token");
throw new ServiceFailureException("Unable to fetch principal entity");
}
- if (principal == null) {
- LOGGER.warn(
- "Failed to resolve principal from tokenInfo client_id={}",
tokenInfo.getClientId());
+ if (principal == null || principal.getType() !=
PolarisEntityType.PRINCIPAL) {
+ LOGGER.warn("Failed to resolve principal from credentials={}",
credentials);
throw new NotAuthorizedException("Unable to authenticate");
}
+ LOGGER.debug("Resolved principal: {}", principal);
+
+ boolean allRoles =
credentials.getPrincipalRoles().contains(PRINCIPAL_ROLE_ALL);
+
Set<String> activatedPrincipalRoles = new HashSet<>();
- // TODO: Consolidate the divergent "scopes" logic between
test-bearer-token and token-exchange.
- if (tokenInfo.getScope() != null &&
!tokenInfo.getScope().equals(PRINCIPAL_ROLE_ALL)) {
+ if (!allRoles) {
activatedPrincipalRoles.addAll(
- Arrays.stream(tokenInfo.getScope().split(" "))
+ credentials.getPrincipalRoles().stream()
.map(
s -> // strip the principal_role prefix, if present
s.startsWith(PRINCIPAL_ROLE_PREFIX)
Review Comment:
BTW I also think that the responsibility of stripping off the prefix should
be transferred from this class to `DefaultPrincipalRolesMapper`. But we can do
this in a follow-up PR.
--
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
