adutra commented on code in PR #1397:
URL: https://github.com/apache/polaris/pull/1397#discussion_r2070230336
##########
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:
That's a very good question. You'll notice that for now only one impl of
`ActiveRolesProvider` exists, which will invariably lookup the grants in the
database.
When we have federated principals, my suggestion would be to change this
Augmentor by either:
1. Creating one different Augmentor for each authentication type
(internal/external)
2. Make the Augmentor use a different impl of `ActiveRolesProvider`
depending on the authentication type.
My preference would be for 2, but in any case, my intention was to tackle
that once we have federated principals. Wdyt? Does this plan work for you?
--
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]
