Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add account and sso to not_actions in DenyRegions and RestrictToSpecifiedRegions #29

Closed
wants to merge 7 commits into from

Conversation

BFarand
Copy link

@BFarand BFarand commented Jun 13, 2023

what

  • Add account:* and sso:* as not_actions in the DenyRegions and RestrictToSpecifiedRegions SCPs.

why

  • Like the other not_actions resources in the SCPs, these resources are provisioned in a specific AWS region that may not necessarily be the one explicitly allow/deny listed.
  • sso:* being denied results in the entire AWS Identity Centre console being unusable.
  • account:* being denied results in, for example, GetAlternateContact requests failing.

@BFarand BFarand requested review from a team as code owners June 13, 2023 16:01
@goruha
Copy link
Member

goruha commented Jun 21, 2023

@Nuru @aknysh could you review this PR pls

@hans-d hans-d requested review from goruha, Nuru and aknysh and removed request for dotCipher and srhopkins March 8, 2024 12:11
@hans-d hans-d added the stale This PR has gone stale label Mar 8, 2024
Copy link

mergify bot commented Mar 9, 2024

Thanks @BFarand for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

@mergify mergify bot added triage Needs triage and removed stale This PR has gone stale labels Mar 15, 2024
@Nuru
Copy link
Sponsor Contributor

Nuru commented Apr 15, 2024

@BFarand Thank you for this contribution.

I am going to close this in favor of PR #50, which incorporates some, but not all, of your changes.

SCPs are supposed to be restrictive, so I want to error on the side of them being overly restrictive. People are free to copy and modify our SCPs if they want to relax some restrictions; I am uncomfortable doing that for them. I did, however, add account and artifact as service exceptions for region restrictions.

  • sso can be provisioned in a region, so I did not add that.
  • Lambda region restrictions are tricky and I do not want to spring that on someone by changing the existing policies.
  • Many S3 actions are region-specific, including creating a bucket, so I did not relax those restrictions.
  • Access Analyzer and SSM are also region-specific and so I did not add those services to the list of exceptions.

I think, in general, it may be asking too much to try to prevent people from provisioning resources in us-east-1 since there are so many special cases of resources that must be provisioned there (e.g. ACM certificates, Lambda@Edge). So if the region restriction policy is too restrictive for those special cases, just add us-east-1 to the allow list. Otherwise we may need a whole new set of policies to deal with all the special cases.

After PR #50 merges, if you still want changes, I suggest you create a separate set of new policies where the global restrictions are relaxed and then individually tightened up, as you did here with Lambdas. Maybe just create a separate statement for the services that need us-east-1 that ensures they are enabled in that region, and then people can choose.

@Nuru Nuru closed this Apr 15, 2024
@mergify mergify bot removed the triage Needs triage label Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants