1141 revoke share fix

[SofiaSazonova]

Feature or Bugfix

• Bugfix

Detail

• Health check is now started with check if the principal role exists
• If Role is missing: -- add share item will fail (Share failed) and then the user will be able delete it -- revoke of the item will be successful ( SHARE_SUCCEEDED)
• If Role was missing, but then restored with the same name and attached policies (to keep arn), the share can be revoked and applied again successfuly

Relates

• 1141

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.

[SofiaSazonova]

How to test:

• [ ] Create consumption role for the env and share request for this role
• [ ] After all items are shared successfully, delete IAM role via AWS Console
• [ ] Check share health. Shares must become unhealthy with the message 'Principal Role is not found'
• [ ] Revoke shares and delete them
• [ ] Delete share request and consumption role
• [ ] Create another consumption role
• [ ] Create share for this new role
• [ ] Share all items, they must be shared successfully

[dlpzx]

Let me first confirm my understanding on how this PR interacts with the current workflows.

Sharing Statuses workflows

Initial state For data sharing, the Share.Status and ShareItem.Status should move in this statemachine:

current state We only verify the principal IAM role in the ECS tasks, so in the start workflows. (ECS tasks)

• if the task is a approve_share task - if IAM role does not exist --> failure
• if the task is a revoke_share - make sure the revoke succeeds and clean everything

target state

• AddItem = No changes, it is alright if the users add items, it is just info in RDS in pending state
• Submit => Should we allow requesters to submit a request when the IAM role does not exist?
• Approve => Should we allow approvers to approve a request when the IAM role does not exist?
• Reject => No changes, it is alright if the users reject, it is just info in RDS in rejected
• One the share ECS task starts => yes, as first item it should check if the principal IAM role exists: - if the task is a approve_share task and the IAM role does not exist => treat is as another Failure - if the task is a revoke_share => clean-up the rest of the share and ensure the revoke completes => This is the most important part of the PR. If needed we could just implement this item. 👀
• RevokeItems = No changes, it is alright if the users revokes items, data.all must ensure the share revoke won't fail, same for DeleteItems

ShareItems health state workflow

initial state We do not have a diagrams because it is an easier workflow:

• in the initial shares is share is successful: set item HealthStatus = Healthy
• in shareItemService verify_items_share_object: From Healthy/ Unhealthy to PendingVerify
• in ECS task share_verify: From PendingVerify to Healthy/ Unhealthy
• in shareItemService reapply_items_share_object: From UnhealthytoPendingReApply`
• in ECS task share_reapply: From PendingReApply to Healthy/ Unhealthy

current state

• in ECS task share_verify: Add check of IAM role, it updates the healthstatus in the verification function -> Unhealthy
• in ECS task share_reapply: Add check of IAM role, it updates the healthstatus in the verification function -> Unhealthy
• In ECS taks share_approve and share_revoke it is also marking the healthStatus as Unhealthy although they are different processes

target state No changes except for:

• in ECS task share_verify: Add check of IAM role and mark as Unhealthy if role does not exist
• in ECS task share_reapply: Add check of IAM role. Health status is already Unhealthy. Should shareItemStatus go to ShareFailed? The initial share succeeded, the problem is that it is unhealthy and the IAM role does not exist. They can still revoke the share, so in my opinion, no, we should just focus on health status here.

[SofiaSazonova]

@dlpzx

• Submit => Should we allow requesters to submit a request when the IAM role does not exist?
• Approve => Should we allow approvers to approve a request when the IAM role does not exist?

Good point, we can terminate it right there.

All other cases are already covered in PR.

[dlpzx]

I think we need to add some test cases

• [x] Create consumption role for the env and share request for this role ---> all items are shared successfully
• [x] Now, delete the IAM role and click verify items and verify some of the items. ---> The verified items only must become unhealthy with the message 'Principal Role is not found'
• [x] Revoke all items from the share ---> the unhealthy as well as the healthy items must be revoked successfully - [ ] Removed from KMS policy - [ ] Removed from S3 policy - [ ] Managed IAM policies deleted
• [x] Re-add the items and submit the share request --> When share is submitted it should throw an error
• [x] Re-create the consumption role, re-add the items, submit the request, delete the consumption role and the approvers click on approve --> When share is approved, it should throw an error
• [x] Re-create the consumption role, re-add the items, submit the request, approve, quickly delete IAM role before ECS task starts ----> When the share is executed, it should put the items in Share_Failed

[SofiaSazonova]

all these tests were made successfully)