-
Notifications
You must be signed in to change notification settings - Fork 17
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
Stale GitHub thumbprints #24
Stale GitHub thumbprints #24
Comments
The CDK Construct OpenIdConnectProvider downloads the thumbprint based on the issuer URL, if the thumbprint is not supplied to it as a prop. I wonder whether it would be better to omit any specific thumbprint from this Construct’s source, and let the OpenIdConnectProvider figure out the right thumbprint based on the issuer URL. Then we would not have to periodically update the thumbprint in this Construct. |
There was a past PR that would've addressed this: #18 |
For anyone using Python you can override these values in the CFN output like this:
|
Fix in the spirit of #18 - removing the entire const provider = new GithubActionsIdentityProvider(this, 'GitHubProvider')
/**
* Removes hard-coded `ThumbprintList` list, because this list is volatile, and the
* parent construct is not being updated.
*
* @see https://github.com/aripalo/aws-cdk-github-oidc/issues/24
* @see https://github.com/aripalo/aws-cdk-github-oidc/pull/18
*/
Aspects.of(provider).add({
visit(node) {
if (node instanceof CfnResource && node.node.id === 'Default') {
node.addPropertyDeletionOverride('ThumbprintList')
}
},
}) |
Automatic updates in Python, based on @moltar's and @bryan-queryai's snippets: provider = GithubActionsIdentityProvider(self, "provider")
for child in provider.node.default_child.node.children:
if isinstance(child, CfnResource):
child.add_property_deletion_override("ThumbprintList") |
Good news! Automatic notice from AWS:
|
First of all, apologies for not handling this issue as it came out. I've been on summer vacation since June 22nd, and though I have not been completely offline / off-the-grid, I haven't really worked with any code, used GitHub, or read any code/work related emails, etc – hence I missed this issue. Secondly, thanks for reporting the issue and for the workarounds posted to this issue (I've heard from multiple people that those have helped during the time this issue persisted. Third, luckily AWS handled this whole ordeal in their end 🎉 That being said, I think I should just remove the existing thumbprint definitions from this construct. Originally I knew that they are optional and IAM "can figure them out", but I saw them as an extra layer of security – on hindsight – they should've been optional even with this construct and leave the addition (and updates) of those for the end-user. But as said above (and as I've also now seen in my AWS accounts), AWS is now handling the verification using trusted root CA. |
GitHub has updated their certificates:
https://github.blog/changelog/2023-06-27-github-actions-update-on-oidc-integration-with-aws/
You can see the thread of challenges for people here:
aws-actions/configure-aws-credentials#357
For temporary workaround:
I cannot make a PR against origin, but essentially here is the fix:
pmoghaddam#1
I extended the code to more natively support overriding going forward.
The text was updated successfully, but these errors were encountered: