Fix/remove allow all bucket statement

[dlpzx]

Feature or Bugfix

• Bugfix

Detail

• Remove unnecessary AllowAllToAdmin statement in bucket policy and in access point policies
• For the deletion of access points in existing shares I kept a reference to AllowAllToAdmin. We could change this, I am not strong opinionated.
• For existing bucket policies and access point policies AllowAllToAdmin will be deleted when all folders are revoked. It is unnecessary but it does not harm anyone to leave it as it is for existing shares.

[UPDATE]
Since the testing uncovered issues derived from removing AllowAllToAdmin with the current clean-up actions I took the issue #1101 and included it in this PR. On top of the aforementioned changes, this PR:

• Remove unnecessary clean_up functions from revoke_folders and move the clean_up logic into the process_revoked_folders function (from the data_sharing_service.py)
• Break delete_access_point_policy into smaller functions with less responsibilities: revoke_access_in_access_point_policy that generates the new access point policy; attach_new_access_point_policy that attaches a given policy to an access point; delete_access_point that deletes an access point.
• Logic on what should be deleted moved to backend/dataall/modules/dataset_sharing/services/share_processors/s3_access_point_process_share.py. Only if the resulting policy contains no statements the access point is deleted, the IAM permissions revoked from the requester IAM role and the KMS key policy revoked.
• Bonus point: fixed small bug on the clean-up (it was assuming prefix_list is a list, but for a single folder shared it can be a string)

Relates

• remove AllowAllToAdmin statement from bucket policies #997
• Handle Entire Folder Revoke + Clean Up Before Share Process Succeeds #1101

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.

[dlpzx]

Testing (I waited to have the PR for linting merged before doing proper testing)

• Revoking all folders deletes access point with AllowAllToAdmins
• Revoke Bucket does not change the AllowAllToAdmin, but works as expected
• Sharing folder creates new access point and access point policy without AllowAllToAdmins statement
• Revoking the folder from previous step deletes access point --> error Failed to revoke S3 permissions to folder folder1 from source account 496481071459//eu-west-1 with target account 443950722698/eu-west-1 due to: An error occurred (MalformedPolicy) when calling the PutAccessPointPolicy operation: Could not parse the policy: Statement is empty!
• Share bucket with empty policy succeeds and adds SSL secure transport statement only

[dlpzx]

I am picking up #1101 alongside this issue because they are very much related

[dlpzx]

Testing again, after picking up #1101 Tested locally.

• Revoking all folders deletes access point with AllowAllToAdmins
• Revoke Bucket does not change the AllowAllToAdmin, but works as expected
• Sharing folder creates new access point and access point policy without AllowAllToAdmins statement
• Revoking the folder from previous step deletes access point and also revokes IAM permissions and KMS permissions
• Sharing 2 folders creates new access point and access point policy without AllowAllToAdmins statement
• Revoking the 2 folders from previous step deletes access point and also revokes IAM permissions and KMS permissions
• Share bucket with empty policy succeeds and adds SSL secure transport statement only

[petrkalos]

lgtm