collado-mike opened a new pull request, #361:
URL: https://github.com/apache/polaris/pull/361

   # Description
   
   This updates the authorization model to allow `catalog_admin` roles to grant 
other CatalogRoles within their catalog to a PrincipalRole. This addresses 
https://github.com/apache/polaris/issues/359 , in which it is stated that 
neither the `catalog_admin` or the `service_admin` roles alone are sufficient 
to grant catalog access to principal roles.
   
   The fix is implemented by removing the requirement for 
`PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE` privilege from the 
`ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE` operation. Implicitly, this states that 
a user only needs privileges to manage the grants on the catalog as a 
securable, but does not need privilege to manage the grants on a PrincipalRole 
as a grantee.
   
   A caller _does_ require `PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE` to revoke 
a privilege, which, admittedly, does make this operation a little bit lopsided. 
However, having the ability to create grants does not implicitly mean the user 
has the privilege to revoke grants for the same securable. For example, a 
`service_admin` may delegate `catalog_admin` to another user/role, but may wish 
to retain `catalog_admin` privileges. Without requiring 
`PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE`, a `catalog_admin` could revoke 
grants from any PrincipalRole, even if the grants weren't originally created by 
the `catalog_admin` itself.
   
   For consistency, I also removed the corresponding 
`*_MANAGE_GRANTS_FOR_GRANTEE` requirements for other `ASSIGN*` operations. So a 
user, having privilege to manage grants on a securable, can create a grant for 
that securable for any grantee without needing special privilege on that 
grantee. As an example, a user having `CATALOG_MANAGE_ACCESS` at a namespace 
level can create grants for any entity at that namespace or below and grant it 
to any CatalogRole in the catalog. The revoke authorization is also consistent 
with the `ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE` operation.
   
   I confirmed with tests that the `catalog_admin` and other roles cannot 
create grant records that cross catalogs (i.e., can't grant `CREATE_TABLE` 
privilege to a catalog role in another catalog). 
   
   Fixes #359
   
   ## Type of change
   
   Please delete options that are not relevant.
   
   - [X] Bug fix (non-breaking change which fixes an issue)
   - [ ] Documentation update
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to not work as expected)
   - [ ] This change requires a documentation update
   
   # How Has This Been Tested?
   
   Added tests to confirm that a `service_admin` can grant `catalog_admin` to 
another principal role and a principal with that role can create CatalogRoles 
and grant them to other PrincipalRoles. Also confirmed that callers that have 
the `CATALOG_MANAGE_ACCESSS` privilege can create grants on CatalogRoles within 
their own catalog, but across catalogs.
   
   - [ ] Test A
   - [ ] Test B
   
   **Test Configuration**:
   * Hardware:
   * Toolchain:
   * SDK:
   
   # Checklist:
   
   Please delete options that are not relevant.
   
   - [ ] I have performed a self-review of my code
   - [ ] I have commented my code, particularly in hard-to-understand areas
   - [ ] I have made corresponding changes to the documentation
   - [ ] My changes generate no new warnings
   - [ ] If adding new functionality, I have discussed my implementation with 
the community using the linked GitHub issue
   


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