Prefer PolarisPrincipal.getRoles in Resolver (#2925)

it should be sufficient to rely on `SecurityContext.getUserPrincipal`
alone, we dont need to call `isUserInRole` explicitly.

note due to the `ResolverTest` testing with non-existent roles we have
to add null-filtering to the `Resolver`.

Author: XN137

polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java

@@ -28,6 +28,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.polaris.core.PolarisCallContext;
@@ -69,7 +70,6 @@ public class Resolver {
 
   // the id of the principal making the call or 0 if unknown
   private final @Nonnull PolarisPrincipal polarisPrincipal;
-  private final @Nonnull SecurityContext securityContext;
 
   // reference catalog name for name resolution
   private final String referenceCatalogName;
@@ -137,7 +137,6 @@ public Resolver(
     this.diagnostics = diagnostics;
     this.polarisMetaStoreManager = polarisMetaStoreManager;
     this.cache = cache;
-    this.securityContext = securityContext;
     this.referenceCatalogName = referenceCatalogName;
 
     // validate inputs
@@ -467,11 +466,11 @@ private void updateResolved() {
 
     // update all principal roles with latest
     if (!this.resolvedCallerPrincipalRoles.isEmpty()) {
-      List<ResolvedPolarisEntity> refreshedResolvedCallerPrincipalRoles =
-          new ArrayList<>(this.resolvedCallerPrincipalRoles.size());
-      this.resolvedCallerPrincipalRoles.forEach(
-          ce -> refreshedResolvedCallerPrincipalRoles.add(this.getFreshlyResolved(ce)));
-      this.resolvedCallerPrincipalRoles = refreshedResolvedCallerPrincipalRoles;
+      this.resolvedCallerPrincipalRoles =
+          resolvedCallerPrincipalRoles.stream()
+              .map(this::getFreshlyResolved)
+              .filter(Objects::nonNull)
+              .collect(Collectors.toList());
     }
 
     // update referenced catalog
@@ -776,13 +775,11 @@ private ResolverStatus resolveCallerPrincipalAndPrincipalRoles(
   /**
    * Resolve all principal roles that the principal has grants for
    *
-   * @param toValidate
-   * @param resolvedCallerPrincipal1
    * @return the list of resolved principal roles the principal has grants for
    */
   private List<ResolvedPolarisEntity> resolveAllPrincipalRoles(
-      List<ResolvedPolarisEntity> toValidate, ResolvedPolarisEntity resolvedCallerPrincipal1) {
-    return resolvedCallerPrincipal1.getGrantRecordsAsGrantee().stream()
+      List<ResolvedPolarisEntity> toValidate, ResolvedPolarisEntity callerPrincipal) {
+    return callerPrincipal.getGrantRecordsAsGrantee().stream()
         .filter(gr -> gr.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode())
         .map(
             gr ->
@@ -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)
         .collect(Collectors.toList());
   }
 

runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java

@@ -38,7 +38,6 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
-import java.util.stream.Collectors;
 import org.apache.iceberg.CatalogProperties;
 import org.apache.iceberg.Schema;
 import org.apache.iceberg.catalog.Catalog;
@@ -69,15 +68,13 @@
 import org.apache.polaris.core.entity.CatalogEntity;
 import org.apache.polaris.core.entity.CatalogRoleEntity;
 import org.apache.polaris.core.entity.PolarisBaseEntity;
-import org.apache.polaris.core.entity.PolarisEntityType;
 import org.apache.polaris.core.entity.PolarisPrivilege;
 import org.apache.polaris.core.entity.PrincipalEntity;
 import org.apache.polaris.core.entity.PrincipalRoleEntity;
 import org.apache.polaris.core.identity.provider.ServiceIdentityProvider;
 import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
 import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
 import org.apache.polaris.core.persistence.dao.entity.BaseResult;
-import org.apache.polaris.core.persistence.dao.entity.EntityResult;
 import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
 import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest;
 import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory;
@@ -464,34 +461,9 @@ protected static void assertSuccess(BaseResult result) {
   protected @Nonnull SecurityContext securityContext(PolarisPrincipal p) {
     SecurityContext securityContext = Mockito.mock(SecurityContext.class);
     Mockito.when(securityContext.getUserPrincipal()).thenReturn(p);
-    Set<String> principalRoleNames = loadPrincipalRolesNames(p);
-    Mockito.when(securityContext.isUserInRole(Mockito.anyString()))
-        .thenAnswer(invocation -> principalRoleNames.contains(invocation.getArgument(0)));
     return securityContext;
   }
 
-  protected @Nonnull Set<String> loadPrincipalRolesNames(PolarisPrincipal p) {
-    PolarisBaseEntity principal =
-        metaStoreManager
-            .findPrincipalByName(callContext.getPolarisCallContext(), p.getName())
-            .orElseThrow();
-    return metaStoreManager
-        .loadGrantsToGrantee(callContext.getPolarisCallContext(), principal)
-        .getGrantRecords()
-        .stream()
-        .filter(gr -> gr.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode())
-        .map(
-            gr ->
-                metaStoreManager.loadEntity(
-                    callContext.getPolarisCallContext(),
-                    0L,
-                    gr.getSecurableId(),
-                    PolarisEntityType.PRINCIPAL_ROLE))
-        .map(EntityResult::getEntity)
-        .map(PolarisBaseEntity::getName)
-        .collect(Collectors.toSet());
-  }
-
   protected @Nonnull PrincipalEntity rotateAndRefreshPrincipal(
       PolarisMetaStoreManager metaStoreManager,
       String principalName,
@@ -524,7 +496,6 @@ private void initBaseCatalog() {
     }
     SecurityContext securityContext = Mockito.mock(SecurityContext.class);
     Mockito.when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot);
-    Mockito.when(securityContext.isUserInRole(Mockito.anyString())).thenReturn(true);
     PolarisPassthroughResolutionView passthroughView =
         new PolarisPassthroughResolutionView(
             resolutionManifestFactory, securityContext, CATALOG_NAME);

runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/AbstractPolarisGenericTableCatalogTest.java

@@ -167,7 +167,6 @@ public void before(TestInfo testInfo) {
 
     securityContext = Mockito.mock(SecurityContext.class);
     when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot);
-    when(securityContext.isUserInRole(isA(String.class))).thenReturn(true);
 
     PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig);
     ReservedProperties reservedProperties = ReservedProperties.NONE;

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogTest.java

@@ -316,7 +316,6 @@ public void before(TestInfo testInfo) {
 
     securityContext = Mockito.mock(SecurityContext.class);
     when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot);
-    when(securityContext.isUserInRole(isA(String.class))).thenReturn(true);
 
     PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig);
     reservedProperties = new ReservedProperties() {};

runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogViewTest.java

@@ -173,7 +173,6 @@ public void before(TestInfo testInfo) {
 
     SecurityContext securityContext = Mockito.mock(SecurityContext.class);
     when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot);
-    when(securityContext.isUserInRole(Mockito.anyString())).thenReturn(true);
 
     PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig);
     ReservedProperties reservedProperties = ReservedProperties.NONE;

runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/AbstractPolicyCatalogTest.java

@@ -188,7 +188,6 @@ public void before(TestInfo testInfo) {
 
     securityContext = Mockito.mock(SecurityContext.class);
     when(securityContext.getUserPrincipal()).thenReturn(authenticatedRoot);
-    when(securityContext.isUserInRole(isA(String.class))).thenReturn(true);
 
     PolarisAuthorizer authorizer = new PolarisAuthorizerImpl(realmConfig);
     ReservedProperties reservedProperties = ReservedProperties.NONE;