From 6990db263f53cee27b33738d711a66f468130337 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Fri, 26 Jan 2024 21:36:03 +0100 Subject: [PATCH 1/2] Support additional claims in Gitlab::Ci::JwtV2 --- app/models/ci/build.rb | 2 +- lib/gitlab/ci/jwt_v2.rb | 23 ++++++++++++----------- spec/lib/gitlab/ci/jwt_spec.rb | 2 +- spec/lib/gitlab/ci/jwt_v2_spec.rb | 24 +++++++++++++++++++++++- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 5c29d5e28c4ccd..b36375a16cb77b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -1178,7 +1178,7 @@ def predefined_jwt_variables def id_tokens_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| id_tokens.each do |var_name, token_data| - token = Gitlab::Ci::JwtV2.for_build(self, aud: expanded_id_token_aud(token_data['aud'])) + token = Gitlab::Ci::JwtV2.for_build(self, additional_claims: { aud: expanded_id_token_aud(token_data['aud']) }) variables.append(key: var_name, value: token, public: false, masked: true) end diff --git a/lib/gitlab/ci/jwt_v2.rb b/lib/gitlab/ci/jwt_v2.rb index 90db9d13d85908..0dd7e2d84c879e 100644 --- a/lib/gitlab/ci/jwt_v2.rb +++ b/lib/gitlab/ci/jwt_v2.rb @@ -9,14 +9,14 @@ class JwtV2 < Jwt GITLAB_HOSTED_RUNNER = 'gitlab-hosted' SELF_HOSTED_RUNNER = 'self-hosted' - def self.for_build(build, aud: DEFAULT_AUD) - new(build, ttl: build.metadata_timeout, aud: aud).encoded + def self.for_build(build, additional_claims: { aud: DEFAULT_AUD }) + new(build, ttl: build.metadata_timeout, additional_claims: additional_claims).encoded end - def initialize(build, ttl:, aud:) + def initialize(build, ttl:, additional_claims:) super(build, ttl: ttl) - @aud = aud + @additional_claims = additional_claims || {} end private @@ -32,13 +32,14 @@ def reserved_claims end def custom_claims - additional_custom_claims = { - runner_id: runner&.id, - runner_environment: runner_environment, - sha: pipeline.sha, - project_visibility: Gitlab::VisibilityLevel.string_level(project.visibility_level), - user_identities: user_identities - }.compact + additional_custom_claims = + @additional_claims.merge( + runner_id: runner&.id, + runner_environment: runner_environment, + sha: pipeline.sha, + project_visibility: Gitlab::VisibilityLevel.string_level(project.visibility_level), + user_identities: user_identities + ).compact mapper = ClaimMapper.new(project_config, pipeline) diff --git a/spec/lib/gitlab/ci/jwt_spec.rb b/spec/lib/gitlab/ci/jwt_spec.rb index f0b203961b40c6..ee0b7e8c5d07f5 100644 --- a/spec/lib/gitlab/ci/jwt_spec.rb +++ b/spec/lib/gitlab/ci/jwt_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Jwt do +RSpec.describe Gitlab::Ci::Jwt, feature_category: :secrets_management do let(:namespace) { build_stubbed(:namespace) } let(:project) { build_stubbed(:project, namespace: namespace) } let(:user) { build_stubbed(:user) } diff --git a/spec/lib/gitlab/ci/jwt_v2_spec.rb b/spec/lib/gitlab/ci/jwt_v2_spec.rb index 1093e6331cdaaf..84ed8a3c60ec12 100644 --- a/spec/lib/gitlab/ci/jwt_v2_spec.rb +++ b/spec/lib/gitlab/ci/jwt_v2_spec.rb @@ -15,6 +15,7 @@ let(:pipeline) { build_stubbed(:ci_pipeline, ref: 'auto-deploy-2020-03-19') } let(:runner) { build_stubbed(:ci_runner) } let(:aud) { described_class::DEFAULT_AUD } + let(:kwargs) { { additional_claims: { aud: aud } } } let(:build) do build_stubbed( @@ -26,7 +27,7 @@ ) end - subject(:ci_job_jwt_v2) { described_class.new(build, ttl: 30, aud: aud) } + subject(:ci_job_jwt_v2) { described_class.new(build, ttl: 30, **kwargs) } it { is_expected.to be_a Gitlab::Ci::Jwt } @@ -81,6 +82,27 @@ end end + describe 'additional claims' do + let(:wlif) { '//iam.googleapis.com/foo' } + let(:kwargs) do + { additional_claims: { aud: aud, wlif: wlif } } + end + + it 'uses specified wlif in the payload' do + expect(payload[:wlif]).to eq(wlif) + end + + context 'when restricted claim is overridden' do + let(:kwargs) do + { additional_claims: { sub: 'override' } } + end + + it 'uses the original subject in the payload' do + expect(payload[:sub]).to eq("project_path:#{project.full_path}:ref_type:branch:ref:#{pipeline.source_ref}") + end + end + end + describe 'custom claims' do let(:project_config) do instance_double( -- GitLab From 092dbfb0318b997a7427810e5632592a78f89e08 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 29 Jan 2024 16:31:22 +0100 Subject: [PATCH 2/2] Support only wlif --- app/models/ci/build.rb | 2 +- lib/gitlab/ci/jwt_v2.rb | 29 +++++++++++++++-------------- spec/lib/gitlab/ci/jwt_v2_spec.rb | 23 +++++++---------------- 3 files changed, 23 insertions(+), 31 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b36375a16cb77b..5c29d5e28c4ccd 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -1178,7 +1178,7 @@ def predefined_jwt_variables def id_tokens_variables Gitlab::Ci::Variables::Collection.new.tap do |variables| id_tokens.each do |var_name, token_data| - token = Gitlab::Ci::JwtV2.for_build(self, additional_claims: { aud: expanded_id_token_aud(token_data['aud']) }) + token = Gitlab::Ci::JwtV2.for_build(self, aud: expanded_id_token_aud(token_data['aud'])) variables.append(key: var_name, value: token, public: false, masked: true) end diff --git a/lib/gitlab/ci/jwt_v2.rb b/lib/gitlab/ci/jwt_v2.rb index 0dd7e2d84c879e..e559615b7a643b 100644 --- a/lib/gitlab/ci/jwt_v2.rb +++ b/lib/gitlab/ci/jwt_v2.rb @@ -9,37 +9,38 @@ class JwtV2 < Jwt GITLAB_HOSTED_RUNNER = 'gitlab-hosted' SELF_HOSTED_RUNNER = 'self-hosted' - def self.for_build(build, additional_claims: { aud: DEFAULT_AUD }) - new(build, ttl: build.metadata_timeout, additional_claims: additional_claims).encoded + def self.for_build(build, aud: DEFAULT_AUD, wlif: nil) + new(build, ttl: build.metadata_timeout, aud: aud, wlif: wlif).encoded end - def initialize(build, ttl:, additional_claims:) + def initialize(build, ttl:, aud:, wlif:) super(build, ttl: ttl) - @additional_claims = additional_claims || {} + @aud = aud + @wlif = wlif end private - attr_reader :aud + attr_reader :aud, :wlif def reserved_claims super.merge({ iss: Feature.enabled?(:oidc_issuer_url) ? Gitlab.config.gitlab.url : Settings.gitlab.base_url, sub: "project_path:#{project.full_path}:ref_type:#{ref_type}:ref:#{source_ref}", - aud: aud + aud: aud, + wlif: wlif }.compact) end def custom_claims - additional_custom_claims = - @additional_claims.merge( - runner_id: runner&.id, - runner_environment: runner_environment, - sha: pipeline.sha, - project_visibility: Gitlab::VisibilityLevel.string_level(project.visibility_level), - user_identities: user_identities - ).compact + additional_custom_claims = { + runner_id: runner&.id, + runner_environment: runner_environment, + sha: pipeline.sha, + project_visibility: Gitlab::VisibilityLevel.string_level(project.visibility_level), + user_identities: user_identities + }.compact mapper = ClaimMapper.new(project_config, pipeline) diff --git a/spec/lib/gitlab/ci/jwt_v2_spec.rb b/spec/lib/gitlab/ci/jwt_v2_spec.rb index 84ed8a3c60ec12..7a51e905a14fdf 100644 --- a/spec/lib/gitlab/ci/jwt_v2_spec.rb +++ b/spec/lib/gitlab/ci/jwt_v2_spec.rb @@ -15,7 +15,7 @@ let(:pipeline) { build_stubbed(:ci_pipeline, ref: 'auto-deploy-2020-03-19') } let(:runner) { build_stubbed(:ci_runner) } let(:aud) { described_class::DEFAULT_AUD } - let(:kwargs) { { additional_claims: { aud: aud } } } + let(:wlif) { nil } let(:build) do build_stubbed( @@ -27,7 +27,7 @@ ) end - subject(:ci_job_jwt_v2) { described_class.new(build, ttl: 30, **kwargs) } + subject(:ci_job_jwt_v2) { described_class.new(build, ttl: 30, aud: aud, wlif: wlif) } it { is_expected.to be_a Gitlab::Ci::Jwt } @@ -80,27 +80,18 @@ it 'uses that aud in the payload' do expect(payload[:aud]).to eq('AWS') end + + it 'does not use wlif claim in the payload' do + expect(payload.include?(:wlif)).to be_falsey + end end - describe 'additional claims' do + context 'when given an wlif claim' do let(:wlif) { '//iam.googleapis.com/foo' } - let(:kwargs) do - { additional_claims: { aud: aud, wlif: wlif } } - end it 'uses specified wlif in the payload' do expect(payload[:wlif]).to eq(wlif) end - - context 'when restricted claim is overridden' do - let(:kwargs) do - { additional_claims: { sub: 'override' } } - end - - it 'uses the original subject in the payload' do - expect(payload[:sub]).to eq("project_path:#{project.full_path}:ref_type:branch:ref:#{pipeline.source_ref}") - end - end end describe 'custom claims' do -- GitLab