-
-
Notifications
You must be signed in to change notification settings - Fork 935
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 integration test for assuming an API Key Role using a Buildkite OIDC token #5416
base: master
Are you sure you want to change the base?
Conversation
…IDC token Until recently, Buildkite OIDC tokens did not contain a `jti` claim. At some point in early 2024 it was possible to assume an API Key Role using Buildkite OIDC tokens, but when testing in January 2025 we found the assume role request was failing with an error: > Missing/invalid jti Buildkite has addressed that by adding a `jti` claim to tokens - it's a good claim to include. However, to reduce the risk of regressions in the future, this proposes adding an integration test with a Buildkite-shaped OIDC token. The trait added to the OIDC::Provider factory is based on a real token that I generated then anonymized. I only test the happy path with this token - there's a buncha existing tests for various unhappy paths (expired token, etc) using the Github Actions shaped OIDC token and there's little value in replicating them. Most of the added test is copy-pasted from the happy-path Github Actions test further up the file. Fixes rubygems#5412
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my first contribution to rubygems.org, so I'm very happy to be guided on style and approach ❤️
context "with matching conditions" do | ||
should "return API key" do | ||
# remove the Github Actions shaped condition in the default fixture | ||
@role.access_policy.statements.first.conditions.clear |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a hack - would you prefer the test setup create just a Buildkite shaped policy? Maybe via a new trait?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please 🙏
runner_environment | ||
] | ||
} | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This config was created based on https://agent.buildkite.com/.well-known/openid-configuration
Until recently, Buildkite OIDC tokens did not contain a
jti
claim. At some point in early 2024 it was possible to assume an API Key Role using Buildkite OIDC tokens, but when testing in January 2025 we found the assume role request was failing with an error:Buildkite has addressed that by adding a
jti
claim to tokens - it's a good claim to include. However, to reduce the risk of regressions in the future, this PR proposes adding an integration test with a Buildkite-shaped OIDC token.The trait added to the OIDC::Provider factory is based on a real token that I generated then anonymized. I only test the happy path with this token - there's a buncha existing tests for various unhappy paths (expired token, etc) using the Github Actions shaped OIDC token and there's little value in replicating them.
Most of the added test is copy-pasted from the happy-path Github Actions test further up the file.
Fixes #5412