eric-maynard commented on code in PR #361:
URL: https://github.com/apache/polaris/pull/361#discussion_r1793834727


##########
polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisServiceImplIntegrationTest.java:
##########
@@ -1775,4 +1790,479 @@ public void testAssignListAndRevokeCatalogRoles() {
       assertThat(response).returns(204, Response::getStatus);
     }
   }
+
+  @Test
+  public void testCatalogAdminGrantAndRevokeCatalogRoles() {
+    // Create a PrincipalRole and a new catalog. Grant the catalog_admin role 
to the new principal
+    // role
+    String principalRoleName = "mypr33";
+    PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
+    createPrincipalRole(principalRole1);
+
+    String catalogName = "myuniquetestcatalog";
+    Catalog catalog =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName)
+            .setStorageConfigInfo(
+                new AwsStorageConfigInfo(
+                    "arn:aws:iam::012345678901:role/jdoe", 
StorageConfigInfo.StorageTypeEnum.S3))
+            .setProperties(new CatalogProperties("s3://bucket1/"))
+            .build();
+    createCatalog(catalog);
+
+    CatalogRole catalogAdminRole = readCatalogRole(catalogName, 
"catalog_admin");
+    grantCatalogRoleToPrincipalRole(principalRoleName, catalogName, 
catalogAdminRole, userToken);
+
+    PrincipalWithCredentials catalogAdminPrincipal = 
createPrincipal("principal1");
+
+    
grantPrincipalRoleToPrincipal(catalogAdminPrincipal.getPrincipal().getName(), 
principalRole1);
+
+    String catalogAdminToken =
+        TokenUtils.getTokenFromSecrets(
+            EXT.client(),
+            EXT.getLocalPort(),
+            catalogAdminPrincipal.getCredentials().getClientId(),
+            catalogAdminPrincipal.getCredentials().getClientSecret(),
+            realm);
+
+    // Create a second principal role. Use the catalog admin principal to list 
principal roles and
+    // grant a catalog role to the new principal role
+    String principalRoleName2 = "mypr2";
+    PrincipalRole principalRole2 = new PrincipalRole(principalRoleName2);
+    createPrincipalRole(principalRole2);
+
+    // create a catalog role and grant it manage_content privilege
+    String catalogRoleName = "mycr1";
+    createCatalogRole(catalogName, catalogRoleName, catalogAdminToken);
+
+    CatalogPrivilege privilege = CatalogPrivilege.CATALOG_MANAGE_CONTENT;
+    grantPrivilegeToCatalogRole(
+        catalogName,
+        catalogRoleName,
+        new CatalogGrant(privilege, GrantResource.TypeEnum.CATALOG),
+        catalogAdminToken,
+        Response.Status.CREATED);
+
+    // The catalog admin can grant the new catalog role to the mypr2 principal 
role
+    grantCatalogRoleToPrincipalRole(
+        principalRoleName2, catalogName, new CatalogRole(catalogRoleName), 
catalogAdminToken);
+
+    // But the catalog admin cannot revoke the role because it requires
+    // PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE
+    try (Response response =
+        newRequest(
+                "http://localhost:%d/api/management/v1/principal-roles/";
+                    + principalRoleName
+                    + "/catalog-roles/"
+                    + catalogName
+                    + "/"
+                    + catalogRoleName,
+                catalogAdminToken)
+            .delete()) {
+      assertThat(response).returns(Response.Status.FORBIDDEN.getStatusCode(), 
Response::getStatus);
+    }
+
+    // The service admin can revoke the role because it has the
+    // PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE privilege
+    try (Response response =
+        newRequest(
+                "http://localhost:%d/api/management/v1/principal-roles/";
+                    + principalRoleName
+                    + "/catalog-roles/"
+                    + catalogName
+                    + "/"
+                    + catalogRoleName,
+                userToken)
+            .delete()) {
+      assertThat(response).returns(Response.Status.NO_CONTENT.getStatusCode(), 
Response::getStatus);
+    }
+  }
+
+  @Test
+  public void testServiceAdminCanTransferCatalogAdmin() {
+    // Create a PrincipalRole and a new catalog. Grant the catalog_admin role 
to the new principal
+    // role
+    String principalRoleName = "mypr33";
+    PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
+    createPrincipalRole(principalRole1);
+
+    String catalogName = "myothertestcatalog";
+    Catalog catalog =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName)
+            .setStorageConfigInfo(
+                new AwsStorageConfigInfo(
+                    "arn:aws:iam::012345678901:role/jdoe", 
StorageConfigInfo.StorageTypeEnum.S3))
+            .setProperties(new CatalogProperties("s3://bucket1/"))
+            .build();
+    createCatalog(catalog);
+
+    CatalogRole catalogAdminRole = readCatalogRole(catalogName, 
"catalog_admin");
+    grantCatalogRoleToPrincipalRole(principalRoleName, catalogName, 
catalogAdminRole, userToken);
+
+    PrincipalWithCredentials catalogAdminPrincipal = 
createPrincipal("principal1");
+
+    
grantPrincipalRoleToPrincipal(catalogAdminPrincipal.getPrincipal().getName(), 
principalRole1);
+
+    String catalogAdminToken =
+        TokenUtils.getTokenFromSecrets(
+            EXT.client(),
+            EXT.getLocalPort(),
+            catalogAdminPrincipal.getCredentials().getClientId(),
+            catalogAdminPrincipal.getCredentials().getClientSecret(),
+            realm);
+
+    // service_admin revokes the catalog_admin privilege from its principal 
role
+    try {
+      try (Response response =
+          newRequest(
+                  
"http://localhost:%d/api/management/v1/principal-roles/service_admin/catalog-roles/";
+                      + catalogName
+                      + "/catalog_admin",
+                  userToken)
+              .delete()) {
+        assertThat(response)
+            .returns(Response.Status.NO_CONTENT.getStatusCode(), 
Response::getStatus);
+      }
+
+      // the service_admin can not revoke the catalog_admin privilege from the 
new principal role
+      try (Response response =
+          newRequest(
+                  "http://localhost:%d/api/management/v1/principal-roles/";
+                      + principalRoleName
+                      + "/catalog-roles/"
+                      + catalogName
+                      + "/catalog_admin",
+                  catalogAdminToken)
+              .delete()) {
+        assertThat(response)
+            .returns(Response.Status.FORBIDDEN.getStatusCode(), 
Response::getStatus);
+      }
+    } finally {
+      // grant the admin role back to service_admin so that cleanup can happen
+      grantCatalogRoleToPrincipalRole(
+          "service_admin", catalogName, catalogAdminRole, catalogAdminToken);
+    }
+  }
+
+  @Test
+  public void testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog() {
+    // Create a PrincipalRole and a new catalog. Grant the catalog_admin role 
to the new principal
+    // role
+    String principalRoleName = "mypr33";
+    PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
+    createPrincipalRole(principalRole1);
+
+    // create a catalog
+    String catalogName = "mytestcatalog";
+    Catalog catalog =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName)
+            .setStorageConfigInfo(
+                new AwsStorageConfigInfo(
+                    "arn:aws:iam::012345678901:role/jdoe", 
StorageConfigInfo.StorageTypeEnum.S3))
+            .setProperties(new CatalogProperties("s3://bucket1/"))
+            .build();
+    createCatalog(catalog);
+
+    // create a second catalog
+    String catalogName2 = "anothercatalog";
+    Catalog catalog2 =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName2)
+            .setStorageConfigInfo(
+                new AwsStorageConfigInfo(
+                    "arn:aws:iam::012345678901:role/jdoe", 
StorageConfigInfo.StorageTypeEnum.S3))
+            .setProperties(new CatalogProperties("s3://bucket1/"))
+            .build();
+    createCatalog(catalog2);
+
+    // create a catalog role *in the second catalog* and grant it 
manage_content privilege
+    String catalogRoleName = "mycr1";
+    createCatalogRole(catalogName2, catalogRoleName, userToken);
+
+    // Get the catalog admin role from the *first* catalog and grant that role 
to the principal role
+    CatalogRole catalogAdminRole = readCatalogRole(catalogName, 
"catalog_admin");
+    grantCatalogRoleToPrincipalRole(principalRoleName, catalogName, 
catalogAdminRole, userToken);
+
+    // Create a principal and grant the principal role to it
+    PrincipalWithCredentials catalogAdminPrincipal = 
createPrincipal("principal1");
+    
grantPrincipalRoleToPrincipal(catalogAdminPrincipal.getPrincipal().getName(), 
principalRole1);
+
+    String catalogAdminToken =
+        TokenUtils.getTokenFromSecrets(
+            EXT.client(),
+            EXT.getLocalPort(),
+            catalogAdminPrincipal.getCredentials().getClientId(),
+            catalogAdminPrincipal.getCredentials().getClientSecret(),
+            realm);
+
+    // Create a second principal role.
+    String principalRoleName2 = "mypr2";
+    PrincipalRole principalRole2 = new PrincipalRole(principalRoleName2);
+    createPrincipalRole(principalRole2);
+
+    // The catalog admin cannot grant the new catalog role to the mypr2 
principal role because the
+    // catalog role is in the wrong catalog
+    try (Response response =
+        newRequest(
+                "http://localhost:%d/api/management/v1/principal-roles/";
+                    + principalRoleName
+                    + "/catalog-roles/"
+                    + catalogName2,
+                catalogAdminToken)
+            .put(Entity.json(new GrantCatalogRoleRequest(new 
CatalogRole(catalogRoleName))))) {
+      assertThat(response).returns(Response.Status.FORBIDDEN.getStatusCode(), 
Response::getStatus);
+    }
+  }
+
+  @Test
+  public void testTableManageAccessCanGrantAndRevokeFromCatalogRoles() {
+    // Create a PrincipalRole and a new catalog.
+    String principalRoleName = "mypr33";
+    PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
+    createPrincipalRole(principalRole1);
+
+    // create a catalog
+    String catalogName = "mytablemanagecatalog";
+    Catalog catalog =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName)
+            .setStorageConfigInfo(
+                new AwsStorageConfigInfo(
+                    "arn:aws:iam::012345678901:role/jdoe", 
StorageConfigInfo.StorageTypeEnum.S3))
+            .setProperties(new CatalogProperties("s3://bucket1/"))
+            .build();
+    createCatalog(catalog);
+
+    // create a valid target CatalogRole in this catalog
+    createCatalogRole(catalogName, "target_catalog_role", userToken);
+
+    // create a second catalog
+    String catalogName2 = "anothertablemanagecatalog";

Review Comment:
   ditto



-- 
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]

Reply via email to