aws-samples / iam-identity-center-team

Open-source temporary elevated access solution for AWS IAM Identity Center.
https://aws-samples.github.io/iam-identity-center-team/
MIT No Attribution
250 stars 59 forks source link

teamNotifications: slackbot notifications go to disabled users #187

Closed jarrod-mg closed 4 months ago

jarrod-mg commented 4 months ago

Describe the bug A clear and concise description of what the bug is.

When TEAM sends a notification via the slackbot, the list of targets include disabled users.

To Reproduce Steps to reproduce the behavior:

  1. Have an SSO group including a disabled user (who is still active in slack - e.g. someone who has changed departments)
  2. Have an Approver policy which includes that SSO group
  3. Raise a request on an OU using that approver policy
  4. The disabled user erroneously receives a slack notification

Expected behavior A clear and concise description of what you expected to happen.

The disabled user does not get a slack message

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information): n/a

Smartphone (please complete the following information): n/a

Additional context

I suspect this may also be an issue when it comes to deciding if a user is allowed to raise or approve a request!

jarrod-mg commented 4 months ago

Probably, a check needs to be added to amplify/backend/function/teamgetUsers/src/index.py in list_idc_users.

If I am right, this is a security issue too - users who should previously have the right to a temporary access, but no longer do, will still be able to raise requests, or approve them (if their new role allows some different AWS access).

tawoyinfa commented 4 months ago

@jarrod-mg Is the group/user synced from an identity provider (e.g AzureAD)? Disabling user access in IAM Identity Center prevents them from signing in to the AWS access portal to access AWS accounts and cloud applications(including TEAM).

jarrod-mg commented 4 months ago

@tawoyinfa Yes, we are using an external IdP.

A user can be disabled in one group and not another - for example if they move departments. So, they could still have access to AWS and TEAM in their new role - but get to make requests or grant approvals as their old job role.

tawoyinfa commented 4 months ago

@jarrod-mg TEAM makes use of user and group membership data that has been provisioned and synchronised into your IAM identity store. I dont know what external IdP you are using, but i would expect that when a user is disabled in an upstream group, that information should be synchronised into IdC and eventually TEAM by association.

jarrod-mg commented 4 months ago

@tawoyinfa OK, I have done a lot of grobbling around in the internals...

Enabled or Disabled is associated with a user, not a users membership in a group - I got that wrong earlier. We got this flagged up because a user who used to have AWS access (and now does not) received the slack message asking them to approve a request. Which they shouldn't have gotten, but you are correct that they couldn't do anything with the access - so this isn't as bad as I had thought.

I still think this is a bug - TEAM should check the user is enabled before including them as an approver and sending them a message, but it isn't the massive priority I thought it was before.

tawoyinfa commented 4 months ago

I will close this for now and put it in the backlog