From 7e2c2ef85d41501377a11216d81b089b2e61c5b2 Mon Sep 17 00:00:00 2001 From: James Healy Date: Thu, 30 Jan 2025 16:57:02 +1100 Subject: [PATCH 1/2] 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 50c8f01ba74ebd4f949f9cd559a55064315e14b8 Mon Sep 17 00:00:00 2001 From: James Healy Date: Thu, 6 Feb 2025 00:08:22 +1100 Subject: [PATCH 2/2] WIP: hacking and slashing my way to a Buildkite Trusted Publisher --- .../rubygem_trusted_publishers_controller.rb | 12 +- app/models/oidc/provider.rb | 3 + app/models/oidc/trusted_publisher.rb | 2 +- .../oidc/trusted_publisher/buildkite.rb | 127 ++++++++++++++++++ .../buildkite/form_component.rb | 24 ++++ .../buildkite/table_component.rb | 13 ++ .../rubygem_trusted_publishers/index_view.rb | 3 +- .../rubygem_trusted_publishers/new_view.rb | 25 ++-- .../buildkites/_buildkite.html.erb | 2 + .../github_actions/_github_action.html.erb | 2 +- config/locales/en.yml | 3 + ...create_oidc_trusted_publisher_buildkite.rb | 16 +++ db/schema.rb | 12 +- 13 files changed, 230 insertions(+), 14 deletions(-) create mode 100644 app/models/oidc/trusted_publisher/buildkite.rb create mode 100644 app/views/components/oidc/trusted_publisher/buildkite/form_component.rb create mode 100644 app/views/components/oidc/trusted_publisher/buildkite/table_component.rb create mode 100644 app/views/oidc/trusted_publisher/buildkites/_buildkite.html.erb create mode 100644 db/migrate/20250204190405_create_oidc_trusted_publisher_buildkite.rb diff --git a/app/controllers/oidc/rubygem_trusted_publishers_controller.rb b/app/controllers/oidc/rubygem_trusted_publishers_controller.rb index 781d2a88cd0..ad8dc9e47f9 100644 --- a/app/controllers/oidc/rubygem_trusted_publishers_controller.rb +++ b/app/controllers/oidc/rubygem_trusted_publishers_controller.rb @@ -12,8 +12,14 @@ def index end def new + trusted_publisher = if params[:trusted_publisher] == "buildkite" + buildkite_trusted_publisher + else + gh_actions_trusted_publisher + end + render OIDC::RubygemTrustedPublishers::NewView.new( - rubygem_trusted_publisher: @rubygem.oidc_rubygem_trusted_publishers.new(trusted_publisher: gh_actions_trusted_publisher) + rubygem_trusted_publisher: @rubygem.oidc_rubygem_trusted_publishers.new(trusted_publisher: trusted_publisher) ) end @@ -60,6 +66,10 @@ def find_rubygem_trusted_publisher @rubygem_trusted_publisher = authorize @rubygem.oidc_rubygem_trusted_publishers.find(params[:id]) end + def buildkite_trusted_publisher + OIDC::TrustedPublisher::Buildkite.new + end + def gh_actions_trusted_publisher github_params = helpers.github_params(@rubygem) diff --git a/app/models/oidc/provider.rb b/app/models/oidc/provider.rb index 04361fff603..7f79a68d5f3 100644 --- a/app/models/oidc/provider.rb +++ b/app/models/oidc/provider.rb @@ -12,6 +12,7 @@ class OIDC::Provider < ApplicationRecord has_many :audits, as: :auditable, dependent: :nullify GITHUB_ACTIONS_ISSUER = "https://token.actions.githubusercontent.com".freeze + BUILDKITE_ISSUER = "https://agent.buildkite.com".freeze def self.github_actions find_by(issuer: GITHUB_ACTIONS_ISSUER) @@ -43,6 +44,8 @@ def trusted_publisher_class case issuer when GITHUB_ACTIONS_ISSUER OIDC::TrustedPublisher::GitHubAction + when BUILDKITE_ISSUER + OIDC::TrustedPublisher::Buildkite end end diff --git a/app/models/oidc/trusted_publisher.rb b/app/models/oidc/trusted_publisher.rb index ad9a68da98a..5d72313c32f 100644 --- a/app/models/oidc/trusted_publisher.rb +++ b/app/models/oidc/trusted_publisher.rb @@ -4,6 +4,6 @@ def self.table_name_prefix end def self.all - [GitHubAction] + [GitHubAction, Buildkite] end end diff --git a/app/models/oidc/trusted_publisher/buildkite.rb b/app/models/oidc/trusted_publisher/buildkite.rb new file mode 100644 index 00000000000..004e295b4bf --- /dev/null +++ b/app/models/oidc/trusted_publisher/buildkite.rb @@ -0,0 +1,127 @@ +class OIDC::TrustedPublisher::Buildkite < ApplicationRecord + has_many :rubygem_trusted_publishers, class_name: "OIDC::RubygemTrustedPublisher", as: :trusted_publisher, dependent: :destroy, + inverse_of: :trusted_publisher + has_many :pending_trusted_publishers, class_name: "OIDC::PendingTrustedPublisher", as: :trusted_publisher, dependent: :destroy, + inverse_of: :trusted_publisher + has_many :rubygems, through: :rubygem_trusted_publishers + has_many :api_keys, dependent: :destroy, inverse_of: :owner, as: :owner + + validates :organization_slug, :pipeline_slug, + presence: true, length: { maximum: Gemcutter::MAX_FIELD_LENGTH } + + validate :unique_publisher + + def self.for_claims(claims) + organization_slug = claims.fetch(:organization_slug) + pipeline_slug = claims.fetch(:pipeline_slug) + + where(organization_slug:, pipeline_slug:).first! + end + + def self.permitted_attributes + %i[organization_slug pipeline_slug] + end + + def self.build_trusted_publisher(params) + params = params.reverse_merge(organization_slug: nil, pipeline_slug: nil) + find_or_initialize_by(params) + end + + def self.publisher_name = "Buildkite" + + def payload + { + name:, + organization_slug:, + pipeline_slug: + } + end + + delegate :as_json, to: :payload + + def organization_slug_condition + OIDC::AccessPolicy::Statement::Condition.new( + operator: "string_equals", + claim: "organization_slug", + value: organization_slug + ) + end + + def pipeline_slug_condition + OIDC::AccessPolicy::Statement::Condition.new( + operator: "string_equals", + claim: "pipeline_slug", + value: pipeline_slug + ) + end + + def audience_condition + OIDC::AccessPolicy::Statement::Condition.new( + operator: "string_equals", + claim: "aud", + value: Gemcutter::HOST + ) + end + + def to_access_policy(jwt) + # TODO what to do with jwt here? + # TODO should we be checking the audience claim? + common_conditions = [organization_slug_condition, pipeline_slug_condition].compact + OIDC::AccessPolicy.new( + statements: [ + OIDC::AccessPolicy::Statement.new( + effect: "allow", + principal: OIDC::AccessPolicy::Statement::Principal.new( + oidc: OIDC::Provider::BUILDKITE_ISSUER + ), + conditions: common_conditions + ) + ] + ) + end + + #class SigstorePolicy + # def initialize(trusted_publisher) + # @trusted_publisher = trusted_publisher + # end + + # def verify(cert) + # # 1.3.6.1.4.1.57264.1.14 is `Source Repository Ref` - AKA Branch or Tag + # ref = cert.openssl.find_extension("1.3.6.1.4.1.57264.1.14")&.value_der&.then { OpenSSL::ASN1.decode(_1).value } + # Sigstore::Policy::Identity.new( + # identity: "https://github.com/#{@trusted_publisher.repository}/#{@trusted_publisher.workflow_slug}@#{ref}", + # issuer: OIDC::Provider::BUILDKITE_ISSUER + # ).verify(cert) + # end + #end + + #def to_sigstore_identity_policy + # SigstorePolicy.new(self) + #end + + def name + "#{self.class.publisher_name} #{organization_slug}/#{pipeline_slug}" + end + + def owns_gem?(rubygem) = rubygem_trusted_publishers.exists?(rubygem: rubygem) + + def ld_context + LaunchDarkly::LDContext.create( + key: "#{model_name.singular}-key-#{id}", + kind: "trusted_publisher", + name: name + ) + end + + private + + def unique_publisher + return unless self.class.exists?( + organization_slug: organization_slug, + pipeline_slug: pipeline_slug, + ) + + errors.add(:base, "publisher already exists") + end + +end diff --git a/app/views/components/oidc/trusted_publisher/buildkite/form_component.rb b/app/views/components/oidc/trusted_publisher/buildkite/form_component.rb new file mode 100644 index 00000000000..2ed81173ce8 --- /dev/null +++ b/app/views/components/oidc/trusted_publisher/buildkite/form_component.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class OIDC::TrustedPublisher::Buildkite::FormComponent < ApplicationComponent + prop :buildkite_form, reader: :public + + def view_template + buildkite_form.fields_for :trusted_publisher do |trusted_publisher_form| + field trusted_publisher_form, :text_field, :organization_slug, autocomplete: :off + field trusted_publisher_form, :text_field, :pipeline_slug, autocomplete: :off + end + end + + private + + def field(form, type, name, optional: false, **) + form.label name, class: "form__label" do + plain form.object.class.human_attribute_name(name) + span(class: "t-text--s") { " (#{t('form.optional')})" } if optional + end + form.send(type, name, class: helpers.class_names("form__input", "tw-border tw-border-red-500" => form.object.errors.include?(name)), **) + p(class: "form__field__instructions") { t("oidc.trusted_publisher.buildkite.#{name}_help_html") } + end +end + diff --git a/app/views/components/oidc/trusted_publisher/buildkite/table_component.rb b/app/views/components/oidc/trusted_publisher/buildkite/table_component.rb new file mode 100644 index 00000000000..97725faba73 --- /dev/null +++ b/app/views/components/oidc/trusted_publisher/buildkite/table_component.rb @@ -0,0 +1,13 @@ +class OIDC::TrustedPublisher::Buildkite::TableComponent < ApplicationComponent + prop :buildkite, reader: :public + + def view_template + dl(class: "tw-flex tw-flex-col sm:tw-grid sm:tw-grid-cols-2 tw-items-baseline tw-gap-4 full-width overflow-wrap") do + dt(class: "description__heading ") { "Organization Slug" } + dd { code { buildkite.organization_slug } } + + dt(class: "description__heading ") { "Pipeline Slug" } + dd { code { buildkite.pipeline_slug } } + end + end +end diff --git a/app/views/oidc/rubygem_trusted_publishers/index_view.rb b/app/views/oidc/rubygem_trusted_publishers/index_view.rb index 42816591196..3d2e7982a44 100644 --- a/app/views/oidc/rubygem_trusted_publishers/index_view.rb +++ b/app/views/oidc/rubygem_trusted_publishers/index_view.rb @@ -18,7 +18,8 @@ def view_template end p do - button_to t(".create"), new_rubygem_trusted_publisher_path(rubygem.slug), class: "form__submit", method: :get + button_to t(".create") + " Buildkite", new_rubygem_trusted_publisher_path(rubygem.slug), params: {trusted_publisher: "buildkite"}, class: "form__submit", method: :get + button_to t(".create") + " Github Actions", new_rubygem_trusted_publisher_path(rubygem.slug), params: {trusted_publisher: "github_actions"}, class: "form__submit", method: :get end header(class: "gems__header push--s") do diff --git a/app/views/oidc/rubygem_trusted_publishers/new_view.rb b/app/views/oidc/rubygem_trusted_publishers/new_view.rb index 0c976a06ae9..fe6b1c00b4e 100644 --- a/app/views/oidc/rubygem_trusted_publishers/new_view.rb +++ b/app/views/oidc/rubygem_trusted_publishers/new_view.rb @@ -14,22 +14,31 @@ def view_template title_content div(class: "t-body") do + p do + "New Trusted Publisher: #{rubygem_trusted_publisher.trusted_publisher.class.publisher_name}" + end form_with( model: rubygem_trusted_publisher, url: rubygem_trusted_publishers_path(rubygem_trusted_publisher.rubygem.slug) ) do |f| - f.label :trusted_publisher_type, class: "form__label" - f.select :trusted_publisher_type, OIDC::TrustedPublisher.all.map { |type| - [type.publisher_name, type.polymorphic_name] - }, {}, class: "form__input form__select" - - render OIDC::TrustedPublisher::GitHubAction::FormComponent.new( - github_action_form: f - ) + f.hidden_field :trusted_publisher_type + + render form_component(f) f.submit class: "form__submit" end end end delegate :rubygem, to: :rubygem_trusted_publisher + + private + + def form_component(form) + case rubygem_trusted_publisher.trusted_publisher + when OIDC::TrustedPublisher::Buildkite then OIDC::TrustedPublisher::Buildkite::FormComponent.new(buildkite_form: form) + when OIDC::TrustedPublisher::GitHubAction then OIDC::TrustedPublisher::GitHubAction::FormComponent.new(github_action_form: form) + else + raise "oh no" + end + end end diff --git a/app/views/oidc/trusted_publisher/buildkites/_buildkite.html.erb b/app/views/oidc/trusted_publisher/buildkites/_buildkite.html.erb new file mode 100644 index 00000000000..009311b6385 --- /dev/null +++ b/app/views/oidc/trusted_publisher/buildkites/_buildkite.html.erb @@ -0,0 +1,2 @@ +<%= render OIDC::TrustedPublisher::Buildkite::TableComponent.new(buildkite:) %> + diff --git a/app/views/oidc/trusted_publisher/github_actions/_github_action.html.erb b/app/views/oidc/trusted_publisher/github_actions/_github_action.html.erb index f54667d34a5..b1832229e3d 100644 --- a/app/views/oidc/trusted_publisher/github_actions/_github_action.html.erb +++ b/app/views/oidc/trusted_publisher/github_actions/_github_action.html.erb @@ -1 +1 @@ -<%= render OIDC::TrustedPublisher::GitHubAction::TableComponent.new(github_action:) %> \ No newline at end of file +<%= render OIDC::TrustedPublisher::GitHubAction::TableComponent.new(github_action:) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index bc93bc72278..cbd42ef15f0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -887,6 +887,9 @@ en: title: "New Pending Trusted Publisher" trusted_publisher: unsupported_type: "Unsupported trusted publisher type" + buildkite: + organization_slug_help_html: "The Buildkite organization slug that owns the pipeline" + pipeline_slug_help_html: "The slug of the Buildkite Pipeline that runs the publishing workflow" github_actions: repository_owner_help_html: "The GitHub organization name or GitHub username that owns the repository" repository_name_help_html: "The name of the GitHub repository that contains the publishing workflow" diff --git a/db/migrate/20250204190405_create_oidc_trusted_publisher_buildkite.rb b/db/migrate/20250204190405_create_oidc_trusted_publisher_buildkite.rb new file mode 100644 index 00000000000..00c44fa8e51 --- /dev/null +++ b/db/migrate/20250204190405_create_oidc_trusted_publisher_buildkite.rb @@ -0,0 +1,16 @@ +class CreateOIDCTrustedPublisherBuildkite < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + create_table :oidc_trusted_publisher_buildkites do |t| + t.string :organization_slug, null: false + t.string :pipeline_slug, null: false + + t.timestamps + end + + add_index :oidc_trusted_publisher_buildkites, + %i[organization_slug pipeline_slug], + unique: true, name: "index_oidc_trusted_publisher_buildkite_claims", algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 8d0190f2dac..589e2a6fda6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,11 +10,11 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_04_065953) do +ActiveRecord::Schema[8.0].define(version: 2025_02_04_190405) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" + enable_extension "pg_catalog.plpgsql" enable_extension "pgcrypto" - enable_extension "plpgsql" create_table "admin_github_users", force: :cascade do |t| t.string "login" @@ -409,6 +409,14 @@ t.index ["trusted_publisher_type", "trusted_publisher_id"], name: "index_oidc_rubygem_trusted_publishers_on_trusted_publisher" end + create_table "oidc_trusted_publisher_buildkites", force: :cascade do |t| + t.string "organization_slug", null: false + t.string "pipeline_slug", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["organization_slug", "pipeline_slug"], name: "index_oidc_trusted_publisher_buildkite_claims", unique: true + end + create_table "oidc_trusted_publisher_github_actions", force: :cascade do |t| t.string "repository_owner", null: false t.string "repository_name", null: false