Migrating from manual pivotRole to cdkRole makes table shares unrevokable #1053
Assignees
Labels
Comments
|
Hi @zsaltys thanks for opening an issue. This is actually an error that should be solved by the automated tasks that update the dataset stack and sync tables. Let us have a look and get back to you |
1 task
|
Hi @zsaltys we confirm that this is a bug and we are working on a fix in #1055. Instead of approaching it as a migration issue, we will add the grant as a robustness measurement to ensure even manual changes or inferences from customers do not result in failed revoked tables. The PR adds the grant of |
dlpzx
added a commit
that referenced
this issue
Feb 19, 2024
### Feature or Bugfix - Feature - Bugfix ### Detail This PR should be tested and reviewed after #1016 is merged. data.all pivot role is a Data Lake admin in Lake Formation. However, to drop tables and databases "DROP" permissions are required, even for Data Lake Admins. In order for the revoke processes to work correctly, we need to ensure that these permissions are granted to the pivot role for data sharing glue resources (shared database and resource link tables) in all situations. One of this scenarios is the migration from manual to auto-created pivot roles and vice-versa as reported in #1053 . Other cases involve manual actions on existing resource links. To avoid any potential issue and make the revoke more robust, this PR explicitly grants DROP permissions on tables before deleting them. And grants "ALL" permissions on the shared_db database. Tested locally: - Create 2 environments with manually created pivot role and create, submit and approve a share request with tables. - [X] Change the configuration to use auto-created pivot roles and revoke the share. The tables are revoked successfully ### Relates - #1053 ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 task
|
Implemented sharing guardrails as part of PR #1055 - closing this issue as complete, please re-open if any additional comments / concerns |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
If you created a share using a manually created pivotRole then the resource link for that table will be owned by that role. pivotRole is the only role that can drop this resource link without additional permissions. Any other role that would want to drop this resource link would need to be granted explicit drop permissions.
If you then switch data.all from using manual pivot role to pivot role created with CDK (as we did when migrating from 2.2 to 2.3) and then attempt to revoke a table share that was created with the old role then the revoke will fail with error like this:
Failed to revoke S3 permissions to table gidmetadata from source account 123//us-east-1 with target account 123/us-east-1 due to: An error occurred (AccessDeniedException) when calling the DeleteTable operation: Insufficient Lake Formation permission(s): Required Drop on foo_tableThis happens because the new pivotRole-cdk does not have drop permissions on the table because it is not the owner of that table.
My proposal would be to either run an upgrade script on all environments to update all resource links to get a drop permission or add a check whether the new role has a drop permission and if not to grant it to itself before dropping. In general I think it's a weird glitch that LF ask LakeFormation admins to have DROP before dropping.. I think that should be reported as a bug.
How to Reproduce
Expected behavior
Revoking should work after switching from manually created pivotRole to automatically created one.
Your project
No response
Screenshots
No response
OS
Linux
Python version
3.9
AWS data.all version
2.3
Additional context
No response
The text was updated successfully, but these errors were encountered: