Fix Check other share exists before clean up

[noah-paige]

Feature or Bugfix

• Bugfix

Detail

• Fix method to detect if other share objects exist on the environment before cleaning up environment-level shared resources (i.e. RAM invitation and PivotRole permissions)

  • Originally, if TeamA in EnvA had 2 shares approved and succeeded on DatasetB and TeamA rejects 1 of the pre-existing shares, the method other_approved_share_object_exists was returning Falseand deleting necessary permissions for the other existing Share
  • Also disables the other existing shares ability to Revoke the still existing share since pivotRole no longer has permissions

• Also fixes the removal of dataall QS Group permissions if there are still existing shares to EnvA

Security

NA

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.

[noah-paige]

Tested in AWS Deployment:

Pre-requisites

• Dataset X owned by Team X in Env X
• Team A and Team B in Env Y (w/ dashboards enabled)

Testing Scenarios:

• Team A Requests Access to Dataset X Table and Folder in Env Y

  • Share Succeeds

• Team B Requests Access to Dataset X Table and Folder in Env Y

  • Share Succeeds

• Team B Revokes Share in Env Y

  • Revoke Succeeds
  • Team B loses access to data
  • Team A still has access to table and folder

• Team A Revokes Share in Env Y

  • Revoke Succeeds
  • Access Revoked to External Account (Env Y) since no other shares exist
  • Works only when Requestor Enviornment has Dashboards Disabled

Upon testing, another issue seems to come up when trying to revoke the QS dataall Group for the second time (for the second revoke in the above testing scenario). It fails with the following error message:

An error occurred (InvalidInputException) when calling the RevokePermissions operation: No permissions revoked. Grantee has no permissions and no grantable permissions on resource.

With the current implementation of QuickSight using 1 QS Group for all EnvGroups in the environment - I believe we will have to avoid revoking any permissions from the source database/table on target QS Group unless there are no other existing shares in the environment for that dataset - @dlpzx let me know your thoughts on this

[noah-paige]

To test this fix again for the following:

• 1 Table Shared 1 Time Cross Account to EnvA (QS enabled)

  • Share Succeeds with Proper LF Permissions
  • Share is Revoked Successfully (no more permissions for Team Role, QS Group, + Pivot Role in EnvA)

• 1 Table Shared 2 Times Cross Account to EnvA (QS enabled)

  • Both Shares Succeed
  • When 1 Share Revoke - no permissions for Team1 but QS Group and Team2 still can access Table in EnvA
  • Second Share Revoked Successfully

• 2 Tables in Same Dataset Shared 1 Time Each to Cross Account to EnvA (QS enabled)

  • Both Shares Succeed
  • When 1 Share Revoke - no more permissions for Team Role, QS Group, + Pivot Role in EnvA for Table1 (still permissions for other table)
  • Second Share Revoked Successfully

[noah-paige]

There is one remaining thing to fix - we currently only revoke external account access for cross account shares when there are no other share objects with the same target env and source dataset URIs

If that is the case we revoke external account access to the target env for all of the self.revoked_tables in the revoke request

The above does not work when there is 2 table shared from the same dataset to the same env (either to the same or different teams). Ultimately this leaves remaining permissions in LakeFormation when revoking a table

Instead, we can revoke external access on a tabular level and check for other existing share items with the same tableUri that are shared to the same envUri. Then if there are no other existing shares on that table we can easily revoke external access to the env for that table

[noah-paige]

Final Testing - all works below, PR ready for review

• 1 Table Shared 1 Time Cross Account to EnvA (QS enabled)

  • Share Succeeds with Proper LF Permissions
  • Share is Revoked Successfully (no more permissions for Team Role, QS Group, + Pivot Role in EnvA)

• 1 Table Shared 2 Times Cross Account to EnvA (QS enabled)

  • Both Shares Succeed
  • When 1 Share Revoke - no permissions for Team1 but QS Group and Team2 still can access Table in EnvA
  • Second Share Revoked Successfully

• 2 Tables in Same Dataset Shared 1 Time Each to Cross Account to EnvA (QS enabled)

  • Both Shares Succeed
  • When 1 Share Revoke - no more permissions for Team Role, QS Group, + Pivot Role in EnvA for Table1 (still permissions for other table)
  • Second Share Revoked Successfully

[dlpzx]

Looks good, just a couple of minor comments

[dlpzx]

I deployed the feature in AWS and I confirm that:

• CICD completes successfully

With Quicksight enabled in envA (dataset A with 2 tables) and envB (groupB1 and groupB2)

• 1 table, 1 group
  • share table1 with groupB1 succeeds, proper LF permissions
  • revoke table1 for groupB1 cleans up shared-db and permissions source to target account
• 1 table, 2 groups
  • share table1 with groupB1 and B2 succeeds, proper LF permissions (permissions source to target account, shared-db1, shared-db2, permissions in shared-dbs)
  • revoke table1 for groupB1 keeps shared resources for B2 (permissions source to target account, shared-db2, permissions in shared-db2)
  • revoke table1 for groupB2 cleans up everything
• 2 tables, 1 group
  • share table1 and table2 with groupB1 succeeds, proper LF permissions (permissions source to target account for table1 and table2, shared-db1, permissions in shared-db1 for table1 and 2)
  • revoke table1 for groupB1 keeps shared resources for TABLE2 (permissions source to target account to table2 only, shared-db1, permissions in shared-db1 for table2 only)
  • revoke table2 for groupB1 cleans up everything

@noah-paige after the small cosmetic changes I think this PR is ready to be merged! Nice job

[dlpzx]

@noah-paige I think it is good to merge :) Should we go for v2m1m0 or directly to main? If it is merged to main it needs to include #816 to avoid npm-audit issues