From 7e2c2ef85d41501377a11216d81b089b2e61c5b2 Mon Sep 17 00:00:00 2001 From: James Healy Date: Thu, 30 Jan 2025 16:57:02 +1100 Subject: [PATCH 1/3] Add integration test for assuming an API Key Role using a Buildkite OIDC 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 #5412 --- test/factories/oidc/api_key_role.rb | 18 ++++++ test/factories/oidc/provider.rb | 51 +++++++++++++++++ .../api/v1/oidc/api_key_roles_test.rb | 56 +++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/test/factories/oidc/api_key_role.rb b/test/factories/oidc/api_key_role.rb index 97d9615ddfa..95e68c449cd 100644 --- a/test/factories/oidc/api_key_role.rb +++ b/test/factories/oidc/api_key_role.rb @@ -19,5 +19,23 @@ ] } end + + trait :buildkite do + provider factory: :oidc_provider_buildkite + sequence(:name) { |n| "Buildkite Pusher #{n}" } + access_policy do + { + statements: [ + { effect: "allow", + principal: { oidc: provider.issuer }, + conditions: [ + { operator: "string_equals", claim: "organization_slug", value: "example-org" } + ] } + ] + } + end + end + + factory :oidc_api_key_role_buildkite, traits: [:buildkite] end end diff --git a/test/factories/oidc/provider.rb b/test/factories/oidc/provider.rb index f745ff2cc63..03ee6ffa844 100644 --- a/test/factories/oidc/provider.rb +++ b/test/factories/oidc/provider.rb @@ -63,5 +63,56 @@ transient do pkey { OpenSSL::PKey::RSA.generate(2048) } end + + trait :buildkite do + sequence(:issuer) { |n| "https://#{n}.agent.buildkite.com" } + configuration do + { + issuer: issuer, + jwks_uri: "#{issuer}/.well-known/jwks", + id_token_signing_alg_values_supported: [ + "RS256" + ], + response_types_supported: [ + "id_token" + ], + scopes_supported: [ + "openid" + ], + subject_types_supported: %w[ + public + pairwise + ], + claims_supported: %w[ + sub + aud + exp + iat + iss + nbf + jti + organization_id + organization_slug + pipeline_id + pipeline_slug + build_number + build_branch + build_tag + build_commit + build_source + step_key + job_id + agent_id + cluster_id + cluster_name + queue_id + queue_key + runner_environment + ] + } + end + end + + factory :oidc_provider_buildkite, traits: [:buildkite] end end diff --git a/test/integration/api/v1/oidc/api_key_roles_test.rb b/test/integration/api/v1/oidc/api_key_roles_test.rb index b2f9a08e2b5..035097cc3e9 100644 --- a/test/integration/api/v1/oidc/api_key_roles_test.rb +++ b/test/integration/api/v1/oidc/api_key_roles_test.rb @@ -383,5 +383,61 @@ def jwt(claims = @claims, key: @pkey) ) end end + + context "with a Buildkite OIDC token" do + setup do + @role = create(:oidc_api_key_role_buildkite, provider: build(:oidc_provider_buildkite, issuer: "https://agent.buildkite.com", pkey: @pkey)) + @user = @role.user + + @claims = { + "aud" => "rubygems.org", + "exp" => 1_680_020_837, + "iat" => 1_680_020_537, + "iss" => "https://agent.buildkite.com", + "jti" => "0194b014-8517-7cef-b232-76a827315f08", + "nbf" => 1_680_019_937, + "sub" => "organization:example-org:pipeline:example-pipeline:ref:refs/heads/main:commit:b5ffe3aeea51cec6c41aef16e45ee6bce47d8810:step:", + "organization_slug" => "example-org", + "pipeline_slug" => "example-pipeline", + "build_number" => 5, + "build_branch" => "main", + "build_tag" => nil, + "build_commit" => "b5ffe3aeea51cec6c41aef16e45ee6bce47d8810", + "step_key" => nil, + "job_id" => "01945ecf-80f0-41e8-9b83-a2970a9305a1", + "agent_id" => "01945ecf-8bcf-40a6-9d70-a765db9a0928", + "build_source" => "ui", + "runner_environment" => "buildkite-hosted" + } + + travel_to Time.zone.at(1_680_020_830) # after the JWT iat, before the exp + end + + context "with matching conditions" do + should "return API key" do + post assume_role_api_v1_oidc_api_key_role_path(@role.token), + params: { + jwt: jwt.to_s + }, + headers: {} + + assert_response :created + + resp = response.parsed_body + + assert_match(/^rubygems_/, resp["rubygems_api_key"]) + assert_equal_hash( + { "rubygems_api_key" => resp["rubygems_api_key"], + "name" => "#{@role.name}-0194b014-8517-7cef-b232-76a827315f08", + "scopes" => ["push_rubygem"], + "expires_at" => 30.minutes.from_now }, + resp + ) + hashed_key = @user.api_keys.sole.hashed_key + + assert_equal hashed_key, Digest::SHA256.hexdigest(resp["rubygems_api_key"]) + end + end + end end end From 3a7260804e3e344fd225993a40cc5837d6f429bf Mon Sep 17 00:00:00 2001 From: James Healy Date: Wed, 5 Feb 2025 22:00:31 +1100 Subject: [PATCH 2/3] Add a failing system test for creating an API Key Role for Buildkite The form and POSTing to create works, but the resulting API Key Role has a condition that expects a principal of "https://token.actions.githubusercontent.com" --- test/system/oidc_test.rb | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/test/system/oidc_test.rb b/test/system/oidc_test.rb index 62524a77d78..9d754fe5864 100644 --- a/test/system/oidc_test.rb +++ b/test/system/oidc_test.rb @@ -218,6 +218,88 @@ def verify_session # rubocop:disable Minitest/TestMethodName assert_equal_hash(expected, role.reload.as_json.slice(*expected.keys)) end + test "creating an api key role for a provider who isn't GitHub" do + provider_two = create(:oidc_provider_buildkite, issuer: "https://agent.buildkite.com") + rubygem = create(:rubygem, owners: [@user]) + + # We intentionally use a gem that hosts source on GitHub, but a different CI/CD provider for pushing + create(:version, rubygem: rubygem, metadata: { "source_code_uri" => "https://github.com/example/repo" }) + + sign_in + page.assert_selector "h1", text: "Dashboard" + visit rubygem_path(rubygem.slug) + click_link "OIDC: Create" + verify_session + + page.assert_selector "h1", text: "New OIDC API Key Role" + + # The page loads defaulting to Github Actions shaped config + assert_field "Name", with: "Push #{rubygem.name}" + assert_select "OIDC provider", options: ["https://token.actions.githubusercontent.com","https://agent.buildkite.com"], selected: "https://token.actions.githubusercontent.com" + assert_checked_field "Push rubygem" + assert_field "Valid for", with: "PT30M" + assert_select "Gem Scope", options: ["All Gems", rubygem.name], selected: rubygem.name + + assert_select "Effect", options: %w[allow deny], selected: "allow", + id: "oidc_api_key_role_access_policy_statements_attributes_0_effect" + assert_field "Claim", with: "aud", + id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_0_claim" + assert_select "Operator", options: ["String Equals", "String Matches"], selected: "String Equals", + id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_0_operator" + assert_field "Value", with: Gemcutter::HOST, + id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_0_value" + assert_field "Claim", with: "repository", + id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_1_claim" + assert_select "Operator", options: ["String Equals", "String Matches"], selected: "String Equals", + id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_1_operator" + assert_field "Value", with: "example/repo", + id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_1_value" + + # Adjust the form to align with Buildkite OIDC tokens + page.select "https://agent.buildkite.com", from: "OIDC provider" + + last_condition = page.find_all(id: /oidc_api_key_role_access_policy_statements_attributes_\d+_conditions_attributes_\d+_wrapper/).last + last_condition.fill_in "Claim", with: "organization_slug" + last_condition.fill_in "Value", with: "example-org" + + page.click_button "Add condition" + new_condition = page.find_all(id: /oidc_api_key_role_access_policy_statements_attributes_\d+_conditions_attributes_\d+_wrapper/).last + new_condition.fill_in "Claim", with: "pipeline_slug" + new_condition.fill_in "Value", with: "example-pipeline" + + click_button "Create Api key role" + + page.assert_selector "h1", text: "API Key Role Push #{rubygem.name}" + + role = OIDC::ApiKeyRole.where(name: "Push #{rubygem.name}", user: @user, provider: provider_two).sole + + token = role.token + expected = { + "name" => "Push #{rubygem.name}", + "token" => token, + "api_key_permissions" => { + "scopes" => ["push_rubygem"], + "valid_for" => 1800, + "gems" => [rubygem.name] + }, + "access_policy" => { + "statements" => [ + { + "effect" => "allow", + "principal" => { "oidc" => "https://agent.buildkite.com" }, + "conditions" => [ + { "operator" => "string_equals", "claim" => "aud", "value" => "localhost" }, + { "operator" => "string_equals", "claim" => "organization_slug", "value" => "example-org" }, + { "operator" => "string_equals", "claim" => "pipeline_slug", "value" => "example-pipeline" } + ] + } + ] + } + } + + assert_equal_hash(expected, role.as_json.slice(*expected.keys)) + end + test "creating rubygem trusted publishers" do rubygem = create(:rubygem, name: "rubygem0") create(:version, rubygem: rubygem, metadata: { "source_code_uri" => "https://github.com/example/rubygem0" }) From 62605dbf83b74c9a23ac96c4c0001e368e969cf4 Mon Sep 17 00:00:00 2001 From: James Healy Date: Wed, 5 Feb 2025 22:14:20 +1100 Subject: [PATCH 3/3] Fix the spec creating an API Key Role for Buildkite The conditional in add_default_params is always try (it's a single =) so statement.principal is set to the GitHub Actions principal (https://token.actions.githubusercontent.com) every time the new API Key Role form is loaded. That field is hidden on the form so the user doesn't have a chance to edit it, and after saving the created role has a provider of Buildkite with an expected principal for GitHub Actions. An alternative solution would be to remove the statement.principal assignment completely. It's not required - when the form is submitted the OIDC::ApiKeyRole#set_statement_principals callback will set the correct principal for both GitHub Actionas *and* Buildkite. --- app/controllers/oidc/api_key_roles_controller.rb | 2 +- test/system/oidc_test.rb | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/app/controllers/oidc/api_key_roles_controller.rb b/app/controllers/oidc/api_key_roles_controller.rb index b0322f5899d..29d065b07aa 100644 --- a/app/controllers/oidc/api_key_roles_controller.rb +++ b/app/controllers/oidc/api_key_roles_controller.rb @@ -134,7 +134,7 @@ def add_default_params(rubygem, statement, condition) return unless rubygem return unless (gh = helpers.link_to_github(rubygem)).presence - return unless (@api_key_role.provider = OIDC::Provider.github_actions) + return unless (@api_key_role.provider == OIDC::Provider.github_actions) statement.principal = { oidc: @api_key_role.provider.issuer } diff --git a/test/system/oidc_test.rb b/test/system/oidc_test.rb index 9d754fe5864..af2e016e6cc 100644 --- a/test/system/oidc_test.rb +++ b/test/system/oidc_test.rb @@ -248,19 +248,14 @@ def verify_session # rubocop:disable Minitest/TestMethodName id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_0_operator" assert_field "Value", with: Gemcutter::HOST, id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_0_value" - assert_field "Claim", with: "repository", - id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_1_claim" - assert_select "Operator", options: ["String Equals", "String Matches"], selected: "String Equals", - id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_1_operator" - assert_field "Value", with: "example/repo", - id: "oidc_api_key_role_access_policy_statements_attributes_0_conditions_attributes_1_value" # Adjust the form to align with Buildkite OIDC tokens page.select "https://agent.buildkite.com", from: "OIDC provider" - last_condition = page.find_all(id: /oidc_api_key_role_access_policy_statements_attributes_\d+_conditions_attributes_\d+_wrapper/).last - last_condition.fill_in "Claim", with: "organization_slug" - last_condition.fill_in "Value", with: "example-org" + page.click_button "Add condition" + new_condition = page.find_all(id: /oidc_api_key_role_access_policy_statements_attributes_\d+_conditions_attributes_\d+_wrapper/).last + new_condition.fill_in "Claim", with: "organization_slug" + new_condition.fill_in "Value", with: "example-org" page.click_button "Add condition" new_condition = page.find_all(id: /oidc_api_key_role_access_policy_statements_attributes_\d+_conditions_attributes_\d+_wrapper/).last