-
Notifications
You must be signed in to change notification settings - Fork 59.9k
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
Clarify if all OIDC conditions are supported on AWS (they don't appear to be) #15324
Comments
Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
@mungojam Thanks so much for opening an issue! I'll triage this for the team to take a look 👀 |
There is a little more evidence that custom claims aren't supported through At least they weren't in 2019 |
And here are the official docs on it that only list a limited number of fields: |
In the example given in the Github Actions docs:
Notice that This should probably be clarified or called out specifically in this documentation, because it's not super obvious. |
|
👋 Checking with engineering team |
Thanks @mungojam for raising this issue. As you have rightly figured out above, AWS currently supports only Can you please provide more details around your specific requirement to set condition on We are currently evaluating to provide a programatic way to customize what custom claims can be made part of the |
At least for me, I want to be able to allow Github Actions workflows that are run in my organization to assume roles in my AWS account using the OIDC connector. The simplest way for me to allow only repos in my organization to do that is to set a condition on the |
I wanted to be able to have some backstop that prevents any Github Actions access to our AWS accounts from repositories outside our GitHub org. We would expect all teams to have additional protections to limit access from only their repositories and may also enforce that, but the ultimate backstop would still be wanted. As it is, I have been able to use StringLike to achieve this instead so the impact on me is not big. I raised this particular issue as I think the GitHub docs should be explicit about the AWS limitations so that people don't waste time or accidentally make dangerous security rules like using |
👋 @mungojam - Just checking in on this one. Are you able to make a PR based on your latest comment above? |
Sorry, I only really have time for doing small documentation tweaks in PRs, or code changes. My skills at writing clear English docs aren't great so it takes me ages (despite being English!). Though on this topic, I note the new additions that allow for the org. name to be included in the audience and also the ability to set custom claims in the |
That's a shame, closing with no explanation when it is clear that plenty of people are confused and potentially use dangerous settings because of the lack of clarity |
@mungojam It looks like stalebot closed this in error, so I'm reopening it. It's currently on the |
👋🏻 @mungojam We're reviewing all open I've looked at the changes to the article since this issue was originally opened and can see that the following change has been made:
I think that the only additional change we might want to make for this issue is to expand the sentence: "The token also includes custom claims provided by GitHub." to explain that some cloud providers do not recognize GitHub's custom claims and that you should check the provider's own documentation before configuring your connection. |
@felicitymay Thanks for looking into this. A few thoughts on what could still be improved. The page you link has a pre-requisite section at the top. I'm pretty methodical so thought I should read all that it said: Specifically it says It is that page which doesn't seem to clearly state that not all token attributes will be supported by all cloud providers. So somebody could follow the advice of the pre-req and plan their enforcement based on that page, assuming that it will be possible to achieve it (I think that was the cause for me). Similarly on the AWS page, there is a clear note recommending the use of |
Thank you for getting back to us so quickly with such clear feedback 💖 I'll update the issue summary based on your feedback and this will be ready for someone to work on. We usually get a lot of contributions during Hacktoberfest. |
Based on your feedback, I've updated the issue summary to suggest a location in each file where we should note that the claims included in a GitHub token may not be supported by your cloud provider and that you should verify which claims are supported. |
👋 Hey @mungojam. Thanks for the suggestion here and for your patience while we worked through this. I've been communicating with some folks internally on this issue and we agree that the docs could use a couple changes to specify the OIDC conditions for AWS. Because these files don't accept community contributions, I'll close this and open an internal issue to fix this. |
That's cool thanks. One side note is that here and on the GitHub roadmap I've noticed githubbers not making use of the close as dupe/not-planned GitHub feature. It really helps to communicate when people are searching for closed issues or looking at notifications |
Co-authored-by: mc <[email protected]>
Code of Conduct
What article on docs.github.com is affected?
https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-amazon-web-services
What part(s) of the article would you like to see updated?
From my experience and others in this ticket, it appears that AWS don't currently support checking the various assertions that are passed through by GitHub e.g. repository_owner in my case. Their docs suggest otherwise, so it is probably a bug in AWS.
It might be worth highlighting in the docs, unless you have been able to get it to work yourselves.
Filtering on
sub
works fine.Additional information
No response
[maintainer edit]
Content changes to fix this issue
The text was updated successfully, but these errors were encountered: