githubwebhooks: allow infosec-prod account to access unfiltered GitHub stream#72
githubwebhooks: allow infosec-prod account to access unfiltered GitHub stream#72indygreg wants to merge 1 commit intomozilla-platform-ops:masterfrom
Conversation
50482f1 to
6b869cc
Compare
| sid = "github_webhooks_all_infosec_subscribe" | ||
| effect = "Allow" | ||
| actions = [ | ||
| "SNS:Subscribe", |
There was a problem hiding this comment.
Could you also grant ListSubscriptions and Unsubscribe so we can determine our subscription ARN and unsubscribe if needed?
githubwebhooks/iam-roles.tf
Outdated
|
|
||
| data "aws_iam_policy_document" "sns_webhooks_all" { | ||
| # Give self full access. | ||
| statement = { |
There was a problem hiding this comment.
I'm not too familiar with TerraForm but if this is you granting your own account rights via a resource policy (AKA SNS Topic Policy), I'd recommend against it unless there's a good reason. I'd recommend governing access to your SNS topics in your own account with an identity-based policy (aka an IAM policy).
There was a problem hiding this comment.
When I loaded up the AWS web console, the topic had a default policy which contained the rules I implemented here. Not sure why that is or if anything is relying on it.
We already have a separate IAM policy allowing the Lambda function to SNS:Publish to this topic. That's the only thing that matters. So I'll remove this.
If we lock ourselves out of the topic, we can restore access via an IAM policy elsewhere.
6b869cc to
f7d7791
Compare
|
I was unable to add I /think/ the error means that we can't grant this action to an entire account: only a service. So if we changed the principal to a service within the infosec AWS account rather than the account itself, it may work. Maybe we could wildcard the principal to all instances of a particular subscriber type? |
…b stream This was requested by @gene1wood so infosec can monitor GitHub activity.
f7d7791 to
f652f8d
Compare
|
The latest PR is rebased on master and is the active configuration. We should probably merge it so Git repo state is in sync with terraform state in production. |
|
Yes, sorry I should have chimed in here. Indeed that makes sense. Could we instead have |
|
I /think/ If your account wants to audit subscriptions for all SNS topics in our AWS account, then we'd grant |
|
The key to Lemme do this to avoid ambiguity, I'll run a couple tests today to validate the exact permissions we need and update the ticket then.
No, no need to do that. |
This was requested by @gene1wood.
There is a default policy on SNS topics. The first portion of the
added policy copies what was present on the topic.