unfunco / terraform-aws-oidc-github

Terraform module to configure GitHub Actions as an IAM OIDC identity provider in AWS.
https://registry.terraform.io/modules/unfunco/oidc-github/aws/latest
Apache License 2.0
91 stars 51 forks source link

Enhancement - Optional variable to define allowed branch/environment? #41

Closed pww217 closed 7 months ago

pww217 commented 7 months ago

Hey @unfunco, thanks again for maintaining this. It's been real helpful.

I've been working to set up one of my developers with this to allow a series of public runners to access some internal resources - namely an S3 bucket. The engineer brought up a security concern around the fact that we define in the IAM role something like:

...
                "StringLike": {
                    "token.actions.githubusercontent.com:sub": "repo:my-org/my-repo:*"
                }
...

This is defined here as I'm sure you know: https://github.com/unfunco/terraform-aws-oidc-github/blob/main/data.tf#L28

Now, Github's own docs state that we are able to further limit what branches are allowed.

We can use specific refs or environments. I'd like to work on a PR to allow users optionally to specific this field instead of just a *. Wanted to get your take before I began the work in earnest to make sure you're interested.

I intend to make * continue to be the default, so this will be a minor/non-breaking change to end users. Purely an option/best practice.

Let me know what you think. Maybe there's some more simple way to accomplish improved security here that I'm overlooking.

pww217 commented 7 months ago

GH actually has a security guide for OIDC for some light reading :)

https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect#configuring-the-oidc-trust-with-the-cloud

Lot to process in here, but I think in most cases people would want to set this to a main branch. We have branch protection rules of course and generally consider 4 eyes on every PR sufficient before it's merged into main, at least for an internal repo.

unfunco commented 7 months ago

Hello @pww217, you should already be able to accomplish this, something like this, as shown in the README.


module "oidc_github" {
  source  = "unfunco/oidc-github/aws"
  version = "1.7.1"

  github_repositories = [
    "org/repo:ref:refs/heads/main",
  ]
}
pww217 commented 7 months ago

Ah ok! Apologies I should have looked closer. I was looking at that data.tf and it didn't seem possible in the template but let me do some testing on my end and if it's all good I'll close this. If it's not I'll consider putting in a PR.

Thanks for the quick answer!

pww217 commented 7 months ago

The way you said seemed to work fine! Sorry for the silly question then and thanks again for your time!

unfunco commented 7 months ago

Not a silly question at all, I'll improve the visibility of the feature in the docs, and it's not really obvious in the code what it's doing either, it's a simple regex that says if the org/repo name contains a : then use the name verbatim, otherwise append a :* to the repo, it could probably be made clearer.