Handle Error on clean up share and not get stuck in IN_PROGRESS status

[noah-paige]

Feature or Bugfix

• Bugfix

Detail

• Scenario

  • Folder share item exists in an existing share object in DA<=v2.2
  • DA upgrade to v2.3
  • Before updating environment/pivot role - user removes folder item from existing share object

• Issue

  • Share tries to create the managed policies in def clean_up_share() and remove the inline policies
  • Function fails since pivotRole does not have proper IAM permissions
  • Exception raised and share gets stuck in a REVOKE_IN_PROGRESS state + Dataset Lock Table never releases Lock for next share to process

• Resolution

  • Wrap def clean_up_share() method for revoke folder item in a try/except block
  • ++ Remove some unused additional code

Security

Please answer the questions below briefly where applicable, or write N/A. Based on
OWASP 10.

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

This is a quick fix - we may want to make folder revoke better and add the clean up steps as part of the def process_revoked_share()

Otherwise, revoking a folder item may not clean up all of the resources before reaching a REVOKE_SUCCEEDED state

For now leaving design as is and just implementing fix so we don't run into larger issues when upgrading DA and revoking items

[dlpzx]

Testing it in AWS and provoking a failure by removing pivot role permissions.
When checking the logs of the ECS share task I can see the error: 12-Mar-24 13:57:25 - INFO - clean_up_share - Failed to clean up folder share resources due to: Data.all Environment Pivot Role does not have permissions to update policy dataall-env-u22f6tn3-share-policy-dataall-test-environmentb1-u22f6tn3: An error occurred (AccessDenied) when calling the CreatePolicyVersion operation: User: arn:aws:sts::XXXXXXX:assumed-role/dataallPivotRole-cdk/dataallPivotRole-cdk is not authorized to perform: iam:CreatePolicyVersion on resource: policy arn:aws:iam::XXXXXXX:policy/dataall-env-u22f6tn3-share-policy-dataall-test-environmentb1-u22f6tn3 because no identity-based policy allows the iam:CreatePolicyVersion action and Clean up S3 successful = False

But then, the revoke as a whole succeeds and the share item appears as revoke succeeded (because the revoke for each folder individually was indeed a success in revoked_folders_succeed. I think @noah-paige you are right at pointing out that we need to review how the clean-up is done. Maybe we should open an issue wdyt?

But in short, this PR solves the main issue = shares stuck in in-progress 🆗

[noah-paige]

New Issue to pick up later to improve the clean up of folders - #1101