adutra commented on code in PR #1397:
URL: https://github.com/apache/polaris/pull/1397#discussion_r2070234629
##########
quarkus/service/src/main/java/org/apache/polaris/service/quarkus/auth/ActiveRolesAugmentor.java:
##########
@@ -32,33 +32,42 @@
/**
* A custom {@link SecurityIdentityAugmentor} that adds active roles to the
{@link
- * SecurityIdentity}. This is used to augment the identity with active roles
after authentication.
+ * SecurityIdentity}. This is used to augment the identity with valid active
roles after
+ * authentication.
*/
@ApplicationScoped
public class ActiveRolesAugmentor implements SecurityIdentityAugmentor {
+ // must run after AuthenticatingAugmentor
+ public static final int PRIORITY = AuthenticatingAugmentor.PRIORITY - 1;
+
@Inject ActiveRolesProvider activeRolesProvider;
+ @Override
+ public int priority() {
+ return PRIORITY;
+ }
+
@Override
public Uni<SecurityIdentity> augment(
SecurityIdentity identity, AuthenticationRequestContext context) {
if (identity.isAnonymous()) {
return Uni.createFrom().item(identity);
}
- return context.runBlocking(() -> augmentWithActiveRoles(identity));
+ return context.runBlocking(() -> validateActiveRoles(identity));
}
- private SecurityIdentity augmentWithActiveRoles(SecurityIdentity identity) {
- AuthenticatedPolarisPrincipal polarisPrincipal =
- identity.getPrincipal(AuthenticatedPolarisPrincipal.class);
- if (polarisPrincipal == null) {
+ private SecurityIdentity validateActiveRoles(SecurityIdentity identity) {
+ if (!(identity.getPrincipal() instanceof AuthenticatedPolarisPrincipal)) {
throw new AuthenticationFailedException("No Polaris principal found");
}
+ AuthenticatedPolarisPrincipal polarisPrincipal =
+ identity.getPrincipal(AuthenticatedPolarisPrincipal.class);
Set<String> validRoleNames =
activeRolesProvider.getActiveRoles(polarisPrincipal);
Review Comment:
We also need a more robust story around roles mapping. For now, most of the
work is done by `PrincipalRolesMapper`, but some further processing is being
done in `DefaultAuthenticator`, for historical reasons. And
`AuthenticatedPolarisPrincipal` wouldn't need to expose roles anymore. But
again I'm leaving that for later, as this PR is already big.
--
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]
