From fcb8c4e13dfc78ac1a0cc86d3456c39d70ab7aea Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sun, 9 Mar 2025 12:10:10 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/rspec/feature_category.yml | 2 - ...endency_proxy_for_containers_controller.rb | 10 +-- .../dependency_proxy/group_settings/update.rb | 17 +++++ .../dependency_proxy/group_setting_type.rb | 9 +++ app/models/dependency_proxy/group_setting.rb | 15 ++++ .../group_settings/update_service.rb | 2 +- .../dependency_proxy/request_token_service.rb | 9 ++- ...roxy_containers_docker_hub_credentials.yml | 9 +++ .../query_analyzer_gitlab_schema_metrics.yml | 2 +- ...olumn_information_on_statement_invalid.yml | 2 +- config/initializers/omniauth.rb | 8 +- ...cret_to_dependency_proxy_group_settings.rb | 26 +++++++ db/schema_migrations/20250225153919 | 1 + db/structure.sql | 5 +- doc/api/graphql/reference/_index.md | 3 + ...est_phase_oauth_login_counter_increment.rb | 23 ++++++ .../dependency_proxy/group_settings.rb | 2 + .../group_settings/update_spec.rb | 16 +++- .../group_setting_type_spec.rb | 4 +- ...hase_oauth_login_counter_increment_spec.rb | 32 ++++++++ .../dependency_proxy/group_setting_spec.rb | 76 ++++++++++++++++++- .../dependency_proxy_group_setting_spec.rb | 29 +++++-- .../rack_middlewares/omniauth_spec.rb | 10 ++- .../group_settings/update_service_spec.rb | 6 +- .../request_token_service_spec.rb | 35 +++++++-- .../helpers/dependency_proxy_helpers.rb | 7 +- ...pendency_proxy_settings_shared_examples.rb | 17 +++++ 27 files changed, 329 insertions(+), 48 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/dependency_proxy_containers_docker_hub_credentials.yml create mode 100644 db/migrate/20250225153919_add_identity_secret_to_dependency_proxy_group_settings.rb create mode 100644 db/schema_migrations/20250225153919 create mode 100644 lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment.rb create mode 100644 spec/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment_spec.rb diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index 169452e3ddd..7ceee9c6bd0 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -1560,7 +1560,6 @@ RSpec/FeatureCategory: - 'spec/graphql/types/customer_relations/organization_state_counts_type_spec.rb' - 'spec/graphql/types/customer_relations/organization_type_spec.rb' - 'spec/graphql/types/dependency_proxy/blob_type_spec.rb' - - 'spec/graphql/types/dependency_proxy/group_setting_type_spec.rb' - 'spec/graphql/types/dependency_proxy/manifest_type_spec.rb' - 'spec/graphql/types/deployment_tier_enum_spec.rb' - 'spec/graphql/types/design_management/design_collection_copy_state_enum_spec.rb' @@ -3243,7 +3242,6 @@ RSpec/FeatureCategory: - 'spec/models/customer_relations/contact_state_counts_spec.rb' - 'spec/models/cycle_analytics/project_level_stage_adapter_spec.rb' - 'spec/models/dependency_proxy/blob_spec.rb' - - 'spec/models/dependency_proxy/group_setting_spec.rb' - 'spec/models/dependency_proxy/image_ttl_group_policy_spec.rb' - 'spec/models/dependency_proxy/manifest_spec.rb' - 'spec/models/dependency_proxy/registry_spec.rb' diff --git a/app/controllers/groups/dependency_proxy_for_containers_controller.rb b/app/controllers/groups/dependency_proxy_for_containers_controller.rb index 1d640688004..0ad09c20420 100644 --- a/app/controllers/groups/dependency_proxy_for_containers_controller.rb +++ b/app/controllers/groups/dependency_proxy_for_containers_controller.rb @@ -8,8 +8,8 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy include Gitlab::Utils::StrongMemoize before_action :ensure_group - before_action :ensure_token_granted!, only: [:blob, :manifest] before_action :ensure_feature_enabled! + before_action :ensure_token_granted!, only: [:blob, :manifest] before_action :verify_workhorse_api!, only: [:authorize_upload_blob, :upload_blob, :authorize_upload_manifest, :upload_manifest] @@ -154,8 +154,8 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy event_name end - def dependency_proxy - @dependency_proxy ||= group.dependency_proxy_setting + def dependency_proxy_setting + @dependency_proxy_setting ||= group.dependency_proxy_setting end def ensure_group @@ -163,11 +163,11 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy end def ensure_feature_enabled! - render_404 unless dependency_proxy.enabled + render_404 unless dependency_proxy_setting.enabled end def ensure_token_granted! - result = DependencyProxy::RequestTokenService.new(image).execute + result = DependencyProxy::RequestTokenService.new(image:, dependency_proxy_setting:).execute if result[:status] == :success @token = result[:token] diff --git a/app/graphql/mutations/dependency_proxy/group_settings/update.rb b/app/graphql/mutations/dependency_proxy/group_settings/update.rb index 8107043f57f..9267d8af383 100644 --- a/app/graphql/mutations/dependency_proxy/group_settings/update.rb +++ b/app/graphql/mutations/dependency_proxy/group_settings/update.rb @@ -22,6 +22,18 @@ module Mutations required: false, description: copy_field_description(Types::DependencyProxy::ImageTtlGroupPolicyType, :enabled) + argument :identity, GraphQL::Types::String, required: false, + experiment: { milestone: '17.10' }, + description: 'Identity credential used to authenticate with Docker Hub when pulling images. ' \ + 'Can be a username (for password or PAT) or organization name (for OAT). ' \ + 'Ignored if `dependency_proxy_containers_docker_hub_credentials` is disabled.' + + argument :secret, GraphQL::Types::String, required: false, + experiment: { milestone: '17.10' }, + description: 'Secret credential used to authenticate with Docker Hub when pulling images. ' \ + 'Can be a password, Personal Access Token (PAT), or Organization Access Token (OAT). ' \ + 'Ignored if `dependency_proxy_containers_docker_hub_credentials` is disabled.' + field :dependency_proxy_setting, Types::DependencyProxy::GroupSettingType, null: true, @@ -30,6 +42,11 @@ module Mutations def resolve(group_path:, **args) group = authorized_find!(group_path: group_path) + unless Feature.enabled?(:dependency_proxy_containers_docker_hub_credentials, group) + args.delete(:identity) + args.delete(:secret) + end + result = ::DependencyProxy::GroupSettings::UpdateService .new(container: group, current_user: current_user, params: args) .execute diff --git a/app/graphql/types/dependency_proxy/group_setting_type.rb b/app/graphql/types/dependency_proxy/group_setting_type.rb index 6c6f848d019..b51d3d8a13f 100644 --- a/app/graphql/types/dependency_proxy/group_setting_type.rb +++ b/app/graphql/types/dependency_proxy/group_setting_type.rb @@ -9,5 +9,14 @@ module Types authorize :admin_dependency_proxy field :enabled, GraphQL::Types::Boolean, null: false, description: 'Indicates whether the dependency proxy is enabled for the group.' + field :identity, GraphQL::Types::String, null: true, + experiment: { milestone: '17.10' }, + description: 'Identity credential used to authenticate with Docker Hub when pulling images. ' \ + 'Can be a username (for password or PAT) or organization name (for OAT). ' \ + 'Returns null if `dependency_proxy_containers_docker_hub_credentials` feature flag is disabled.' + + def identity + object.identity if Feature.enabled?(:dependency_proxy_containers_docker_hub_credentials, object.group) + end end end diff --git a/app/models/dependency_proxy/group_setting.rb b/app/models/dependency_proxy/group_setting.rb index b39ea36644a..a9c865b8090 100644 --- a/app/models/dependency_proxy/group_setting.rb +++ b/app/models/dependency_proxy/group_setting.rb @@ -3,5 +3,20 @@ class DependencyProxy::GroupSetting < ApplicationRecord belongs_to :group + encrypts :identity + encrypts :secret + validates :group, presence: true + validates :identity, presence: true, if: :secret? + validates :secret, presence: true, if: :identity? + validates :identity, :secret, length: { maximum: 255 } + + def authorization_header + return {} unless Feature.enabled?(:dependency_proxy_containers_docker_hub_credentials, group) + return {} unless identity? && secret? + + authorization = ActionController::HttpAuthentication::Basic.encode_credentials(identity, secret) + + { Authorization: authorization } + end end diff --git a/app/services/dependency_proxy/group_settings/update_service.rb b/app/services/dependency_proxy/group_settings/update_service.rb index ba43452def3..26c9e28cf25 100644 --- a/app/services/dependency_proxy/group_settings/update_service.rb +++ b/app/services/dependency_proxy/group_settings/update_service.rb @@ -3,7 +3,7 @@ module DependencyProxy module GroupSettings class UpdateService < BaseContainerService - ALLOWED_ATTRIBUTES = %i[enabled].freeze + ALLOWED_ATTRIBUTES = %i[enabled identity secret].freeze def execute return ServiceResponse.error(message: 'Access Denied', http_status: 403) unless allowed? diff --git a/app/services/dependency_proxy/request_token_service.rb b/app/services/dependency_proxy/request_token_service.rb index 4ca7239b9f6..4876ca78bd8 100644 --- a/app/services/dependency_proxy/request_token_service.rb +++ b/app/services/dependency_proxy/request_token_service.rb @@ -2,12 +2,17 @@ module DependencyProxy class RequestTokenService < DependencyProxy::BaseService - def initialize(image) + def initialize(image:, dependency_proxy_setting:) @image = image + @dependency_proxy_setting = dependency_proxy_setting end def execute - response = Gitlab::HTTP.get(auth_url) + response = Gitlab::HTTP.get( + auth_url, + headers: @dependency_proxy_setting&.authorization_header || {}, + follow_redirects: true + ) if response.success? success(token: Gitlab::Json.parse(response.body)['token']) diff --git a/config/feature_flags/gitlab_com_derisk/dependency_proxy_containers_docker_hub_credentials.yml b/config/feature_flags/gitlab_com_derisk/dependency_proxy_containers_docker_hub_credentials.yml new file mode 100644 index 00000000000..b7b930c07f1 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/dependency_proxy_containers_docker_hub_credentials.yml @@ -0,0 +1,9 @@ +--- +name: dependency_proxy_containers_docker_hub_credentials +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/331741 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182748 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/521312 +milestone: '17.10' +group: group::container registry +type: gitlab_com_derisk +default_enabled: false diff --git a/config/feature_flags/ops/query_analyzer_gitlab_schema_metrics.yml b/config/feature_flags/ops/query_analyzer_gitlab_schema_metrics.yml index 83476bcc65e..123b266a2a3 100644 --- a/config/feature_flags/ops/query_analyzer_gitlab_schema_metrics.yml +++ b/config/feature_flags/ops/query_analyzer_gitlab_schema_metrics.yml @@ -3,5 +3,5 @@ name: query_analyzer_gitlab_schema_metrics introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73839 milestone: '14.5' type: ops -group: group::cells infrastructure +group: group::database frameworks default_enabled: false diff --git a/config/feature_flags/ops/reset_column_information_on_statement_invalid.yml b/config/feature_flags/ops/reset_column_information_on_statement_invalid.yml index 384a18caeaf..a199574ec2c 100644 --- a/config/feature_flags/ops/reset_column_information_on_statement_invalid.yml +++ b/config/feature_flags/ops/reset_column_information_on_statement_invalid.yml @@ -4,5 +4,5 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121957 rollout_issue_url: milestone: '16.1' type: ops -group: group::cells infrastructure +group: group::database frameworks default_enabled: true diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 3ff48cbad1e..3ea1157adc8 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -20,12 +20,6 @@ end OmniAuth.config.logger = Gitlab::AppLogger -omniauth_login_counter = - Gitlab::Metrics.counter( - :gitlab_omniauth_login_total, - 'Counter of initiated OmniAuth login attempts') - OmniAuth.config.before_request_phase do |env| - provider = env['omniauth.strategy']&.name - omniauth_login_counter.increment(omniauth_provider: provider, status: 'initiated') + Gitlab::Auth::OAuth::BeforeRequestPhaseOauthLoginCounterIncrement.call(env) end diff --git a/db/migrate/20250225153919_add_identity_secret_to_dependency_proxy_group_settings.rb b/db/migrate/20250225153919_add_identity_secret_to_dependency_proxy_group_settings.rb new file mode 100644 index 00000000000..c9c1e970e31 --- /dev/null +++ b/db/migrate/20250225153919_add_identity_secret_to_dependency_proxy_group_settings.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class AddIdentitySecretToDependencyProxyGroupSettings < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + TABLE_NAME = :dependency_proxy_group_settings + + def up + with_lock_retries do + add_column TABLE_NAME, :identity, :jsonb, null: true, if_not_exists: true + add_column TABLE_NAME, :secret, :jsonb, null: true, if_not_exists: true + end + + add_check_constraint TABLE_NAME, + 'num_nonnulls(identity, secret) = 2 OR num_nulls(identity, secret) = 2', + check_constraint_name(TABLE_NAME, 'identity_and_secret', 'both_set_or_null') + end + + def down + with_lock_retries do + remove_column(TABLE_NAME, :identity, if_exists: true) + remove_column(TABLE_NAME, :secret, if_exists: true) + end + end +end diff --git a/db/schema_migrations/20250225153919 b/db/schema_migrations/20250225153919 new file mode 100644 index 00000000000..ec10e9114ad --- /dev/null +++ b/db/schema_migrations/20250225153919 @@ -0,0 +1 @@ +c97190eadf5bbcb643d7b69745c435ca95ce300ee332f91dfb194ba478b2c4a2 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index cd204bc0256..59d61110962 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12757,7 +12757,10 @@ CREATE TABLE dependency_proxy_group_settings ( group_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, - enabled boolean DEFAULT true NOT NULL + enabled boolean DEFAULT true NOT NULL, + identity jsonb, + secret jsonb, + CONSTRAINT check_7ed6c2f608 CHECK (((num_nonnulls(identity, secret) = 2) OR (num_nulls(identity, secret) = 2))) ); CREATE SEQUENCE dependency_proxy_group_settings_id_seq diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 0bf83caa4ea..bd41f9f4fc1 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -11239,6 +11239,8 @@ Input type: `UpdateDependencyProxySettingsInput` | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `enabled` | [`Boolean`](#boolean) | Indicates whether the policy is enabled or disabled. | | `groupPath` | [`ID!`](#id) | Group path for the group dependency proxy. | +| `identity` {{< icon name="warning-solid" >}} | [`String`](#string) | **Deprecated:** **Status**: Experiment. Introduced in GitLab 17.10. | +| `secret` {{< icon name="warning-solid" >}} | [`String`](#string) | **Deprecated:** **Status**: Experiment. Introduced in GitLab 17.10. | #### Fields @@ -24418,6 +24420,7 @@ Group-level Dependency Proxy settings. | Name | Type | Description | | ---- | ---- | ----------- | | `enabled` | [`Boolean!`](#boolean) | Indicates whether the dependency proxy is enabled for the group. | +| `identity` {{< icon name="warning-solid" >}} | [`String`](#string) | **Introduced** in GitLab 17.10. **Status**: Experiment. Identity credential used to authenticate with Docker Hub when pulling images. Can be a username (for password or PAT) or organization name (for OAT). Returns null if `dependency_proxy_containers_docker_hub_credentials` feature flag is disabled. | ### `Deployment` diff --git a/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment.rb b/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment.rb new file mode 100644 index 00000000000..0e2904a437c --- /dev/null +++ b/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Gitlab + module Auth + module OAuth + module BeforeRequestPhaseOauthLoginCounterIncrement + OMNIAUTH_LOGIN_TOTAL_COUNTER = + Gitlab::Metrics.counter(:gitlab_omniauth_login_total, 'Counter of initiated OmniAuth login attempts') + + def self.call(env) + provider = current_provider_name_from(env) + return unless provider + + OMNIAUTH_LOGIN_TOTAL_COUNTER.increment(omniauth_provider: provider, status: 'initiated') + end + + private_class_method def self.current_provider_name_from(env) + env['omniauth.strategy']&.name + end + end + end + end +end diff --git a/spec/factories/dependency_proxy/group_settings.rb b/spec/factories/dependency_proxy/group_settings.rb index c15cddf7430..36a3669f049 100644 --- a/spec/factories/dependency_proxy/group_settings.rb +++ b/spec/factories/dependency_proxy/group_settings.rb @@ -5,5 +5,7 @@ FactoryBot.define do group enabled { true } + identity { 'username' } + secret { 'secret' } end end diff --git a/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb b/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb index 6cae85ab56c..3fef8cf6cfd 100644 --- a/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb +++ b/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Mutations::DependencyProxy::GroupSettings::Update, feature_catego let_it_be_with_reload(:group) { create(:group) } let_it_be_with_reload(:group_settings) { create(:dependency_proxy_group_setting, group: group) } let_it_be(:current_user) { create(:user) } - let(:params) { { group_path: group.full_path, enabled: false } } + let(:params) { { group_path: group.full_path, enabled: false, identity: 'i', secret: 's' } } specify { expect(described_class).to require_graphql_authorizations(:admin_dependency_proxy) } @@ -18,8 +18,8 @@ RSpec.describe Mutations::DependencyProxy::GroupSettings::Update, feature_catego shared_examples 'updating the dependency proxy group settings' do it_behaves_like 'updating the dependency proxy group settings attributes', - from: { enabled: true }, - to: { enabled: false } + from: { enabled: true, identity: 'username', secret: 'secret' }, + to: { enabled: false, identity: 'i', secret: 's' } it 'returns the dependency proxy settings no errors' do expect(subject).to eq( @@ -27,6 +27,16 @@ RSpec.describe Mutations::DependencyProxy::GroupSettings::Update, feature_catego errors: [] ) end + + context 'with dependency_proxy_containers_docker_hub_credentials disabled' do + before do + stub_feature_flags(dependency_proxy_containers_docker_hub_credentials: false) + end + + it_behaves_like 'updating the dependency proxy group settings attributes', + from: { enabled: true, identity: 'username', secret: 'secret' }, + to: { enabled: false, identity: 'username', secret: 'secret' } + end end shared_examples 'denying access to dependency proxy group settings' do diff --git a/spec/graphql/types/dependency_proxy/group_setting_type_spec.rb b/spec/graphql/types/dependency_proxy/group_setting_type_spec.rb index cd648cf4b4d..f91c81a837e 100644 --- a/spec/graphql/types/dependency_proxy/group_setting_type_spec.rb +++ b/spec/graphql/types/dependency_proxy/group_setting_type_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['DependencyProxySetting'] do +RSpec.describe GitlabSchema.types['DependencyProxySetting'], feature_category: :virtual_registry do it 'includes dependency proxy blob fields' do expected_fields = %w[ - enabled + enabled identity ] expect(described_class).to include_graphql_fields(*expected_fields) diff --git a/spec/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment_spec.rb b/spec/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment_spec.rb new file mode 100644 index 00000000000..a1ddfa862a1 --- /dev/null +++ b/spec/lib/gitlab/auth/o_auth/before_request_phase_oauth_login_counter_increment_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Auth::OAuth::BeforeRequestPhaseOauthLoginCounterIncrement, feature_category: :system_access do + describe '.call' do + let(:env) { { 'omniauth.strategy' => omniauth_strategy } } + let(:omniauth_strategy) { instance_double(OmniAuth::Strategies::GoogleOauth2, name: 'google_oauth2') } + + it 'increments Prometheus counter for the given provider' do + expect { described_class.call(env) } + .to change { gitlab_metric_omniauth_login_total_for('google_oauth2', 'initiated') }.by(1) + end + + context 'when omniauth strategy is nil' do + let(:omniauth_strategy) { nil } + + it 'does not increment counter' do + expect { described_class.call(env) } + .to change { gitlab_metric_omniauth_login_total_for('google_oauth2', 'initiated') }.by(0) + + expect(gitlab_metric_omniauth_login_total_for(nil, 'initiated')).to eq 0 + end + end + + def gitlab_metric_omniauth_login_total_for(omniauth_provider, status) + Gitlab::Metrics.registry.get(:gitlab_omniauth_login_total) + &.get(omniauth_provider: omniauth_provider, status: status) + .to_f + end + end +end diff --git a/spec/models/dependency_proxy/group_setting_spec.rb b/spec/models/dependency_proxy/group_setting_spec.rb index 4da1fe42ff2..e3bea555ba6 100644 --- a/spec/models/dependency_proxy/group_setting_spec.rb +++ b/spec/models/dependency_proxy/group_setting_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe DependencyProxy::GroupSetting, type: :model do +RSpec.describe DependencyProxy::GroupSetting, type: :model, feature_category: :virtual_registry do + subject(:setting) { build(:dependency_proxy_group_setting) } + describe 'relationships' do it { is_expected.to belong_to(:group) } end @@ -14,5 +16,77 @@ RSpec.describe DependencyProxy::GroupSetting, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:group) } + it { is_expected.to validate_presence_of(:identity) } + it { is_expected.to validate_presence_of(:secret) } + it { is_expected.to validate_length_of(:identity).is_at_most(255) } + it { is_expected.to validate_length_of(:secret).is_at_most(255) } + + context 'for identity and secret cross validation' do + using RSpec::Parameterized::TableSyntax + + where(:identity, :secret, :valid) do + nil | nil | true + 'i' | nil | false + nil | 's' | false + 'i' | 's' | true + end + + with_them do + it 'works as expected' do + setting.identity = identity + setting.secret = secret + + if valid + expect(setting).to be_valid + else + expect(setting).not_to be_valid + end + end + end + end + end + + describe '#authorization_header' do + let_it_be_with_reload(:dependency_proxy_setting) { create(:dependency_proxy_group_setting) } + + subject { dependency_proxy_setting.authorization_header } + + shared_examples 'empty authorization headers' do + it { is_expected.to eq({}) } + end + + context 'with identity and secret set' do + let(:expected_headers) { { Authorization: 'Basic aTpz' } } + + before do + dependency_proxy_setting.update!(identity: 'i', secret: 's') + end + + it { is_expected.to eq(expected_headers) } + + context 'when dependency_proxy_containers_docker_hub_credentials is disabled' do + before do + stub_feature_flags(dependency_proxy_containers_docker_hub_credentials: false) + end + + it_behaves_like 'empty authorization headers' + end + end + + context 'with identity and secret not set' do + before do + dependency_proxy_setting.update!(identity: nil, secret: nil) + end + + it_behaves_like 'empty authorization headers' + + context 'when dependency_proxy_containers_docker_hub_credentials is disabled' do + before do + stub_feature_flags(dependency_proxy_containers_docker_hub_credentials: false) + end + + it_behaves_like 'empty authorization headers' + end + end end end diff --git a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb index 3ec2d7633bb..7e5d98abf77 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb @@ -35,12 +35,12 @@ RSpec.describe 'getting dependency proxy settings for a group', feature_category stub_config(dependency_proxy: { enabled: true }) end - subject { post_graphql(query, current_user: user, variables: variables) } + subject(:post_query) { post_graphql(query, current_user: user, variables: variables) } shared_examples 'dependency proxy group setting query' do it_behaves_like 'a working graphql query' do before do - subject + post_query end end @@ -68,10 +68,13 @@ RSpec.describe 'getting dependency proxy settings for a group', feature_category end it 'return the proper response' do - subject + post_query if access_granted - expect(dependency_proxy_group_setting_response).to eq('enabled' => true) + expect(dependency_proxy_group_setting_response).to eq( + 'enabled' => true, + 'identity' => group.dependency_proxy_setting.identity + ) else expect(dependency_proxy_group_setting_response).to be_blank end @@ -82,10 +85,26 @@ RSpec.describe 'getting dependency proxy settings for a group', feature_category context 'with the settings model created' do before do - group.create_dependency_proxy_setting!(enabled: true) + group.create_dependency_proxy_setting!(enabled: true, identity: 'i', secret: 's') end it_behaves_like 'dependency proxy group setting query' + + context 'with dependency_proxy_containers_docker_hub_credentials disabled' do + before do + stub_feature_flags(dependency_proxy_containers_docker_hub_credentials: false) + end + + it 'does not return the identity' do + group.add_owner(user) + post_query + + expect(dependency_proxy_group_setting_response).to eq( + 'enabled' => true, + 'identity' => nil + ) + end + end end context 'without the settings model created' do diff --git a/spec/requests/rack_middlewares/omniauth_spec.rb b/spec/requests/rack_middlewares/omniauth_spec.rb index ac10845bb1a..ae5fddb0ec0 100644 --- a/spec/requests/rack_middlewares/omniauth_spec.rb +++ b/spec/requests/rack_middlewares/omniauth_spec.rb @@ -5,10 +5,12 @@ require 'spec_helper' RSpec.describe 'OmniAuth Rack middlewares', feature_category: :system_access do describe 'OmniAuth before_request_phase callback' do it 'increments Prometheus counter' do - post('/users/auth/google_oauth2') - - counter = Gitlab::Metrics.registry.get(:gitlab_omniauth_login_total) - expect(counter.get(omniauth_provider: 'google_oauth2', status: 'initiated')).to eq(1) + expect { post('/users/auth/google_oauth2') } + .to change { + Gitlab::Metrics.registry.get(:gitlab_omniauth_login_total) + &.get(omniauth_provider: 'google_oauth2', status: 'initiated') + .to_f + }.by(1) end end end diff --git a/spec/services/dependency_proxy/group_settings/update_service_spec.rb b/spec/services/dependency_proxy/group_settings/update_service_spec.rb index 0d83d2eaca1..c3cd8b5a3d9 100644 --- a/spec/services/dependency_proxy/group_settings/update_service_spec.rb +++ b/spec/services/dependency_proxy/group_settings/update_service_spec.rb @@ -8,15 +8,15 @@ RSpec.describe ::DependencyProxy::GroupSettings::UpdateService, feature_category let_it_be_with_reload(:group) { create(:group) } let_it_be_with_reload(:group_settings) { create(:dependency_proxy_group_setting, group: group) } let_it_be(:user) { create(:user) } - let_it_be(:params) { { enabled: false } } + let_it_be(:params) { { enabled: false, identity: 'i', secret: 's' } } describe '#execute' do subject { described_class.new(container: group, current_user: user, params: params).execute } shared_examples 'updating the dependency proxy group settings' do it_behaves_like 'updating the dependency proxy group settings attributes', - from: { enabled: true }, - to: { enabled: false } + from: { enabled: true, identity: 'username', secret: 'secret' }, + to: { enabled: false, identity: 'i', secret: 's' } it 'returns a success' do result = subject diff --git a/spec/services/dependency_proxy/request_token_service_spec.rb b/spec/services/dependency_proxy/request_token_service_spec.rb index 259e07841d2..9843d710d7e 100644 --- a/spec/services/dependency_proxy/request_token_service_spec.rb +++ b/spec/services/dependency_proxy/request_token_service_spec.rb @@ -4,23 +4,38 @@ require 'spec_helper' RSpec.describe DependencyProxy::RequestTokenService, feature_category: :virtual_registry do include DependencyProxyHelpers + let_it_be_with_reload(:dependency_proxy_setting) { create(:dependency_proxy_group_setting) } + let(:image) { 'alpine:3.9' } let(:token) { Digest::SHA256.hexdigest('123') } - subject { described_class.new(image).execute } + subject { described_class.new(image:, dependency_proxy_setting:).execute } context 'remote request is successful' do - before do - stub_registry_auth(image, token) + context 'with identity and secret set' do + before do + dependency_proxy_setting.update!(identity: 'i', secret: 's') + stub_registry_auth(image, token, request_headers: dependency_proxy_setting.authorization_header) + end + + it { expect(subject[:status]).to eq(:success) } + it { expect(subject[:token]).to eq(token) } end - it { expect(subject[:status]).to eq(:success) } - it { expect(subject[:token]).to eq(token) } + context 'with identity and secret are not set' do + before do + dependency_proxy_setting.update!(identity: nil, secret: nil) + stub_registry_auth(image, token, request_headers: dependency_proxy_setting.authorization_header) + end + + it { expect(subject[:status]).to eq(:success) } + it { expect(subject[:token]).to eq(token) } + end end context 'remote request is not found' do before do - stub_registry_auth(image, token, 404) + stub_registry_auth(image, token, status: 404, request_headers: dependency_proxy_setting.authorization_header) end it { expect(subject[:status]).to eq(:error) } @@ -30,7 +45,13 @@ RSpec.describe DependencyProxy::RequestTokenService, feature_category: :virtual_ context 'failed to parse response body' do before do - stub_registry_auth(image, token, 200, 'dasd1321: wow') + stub_registry_auth( + image, + token, + status: 200, + body: 'dasd1321: wow', + request_headers: dependency_proxy_setting.authorization_header + ) end it { expect(subject[:status]).to eq(:error) } diff --git a/spec/support/helpers/dependency_proxy_helpers.rb b/spec/support/helpers/dependency_proxy_helpers.rb index 91912ffe2cd..37cc7905eb6 100644 --- a/spec/support/helpers/dependency_proxy_helpers.rb +++ b/spec/support/helpers/dependency_proxy_helpers.rb @@ -3,12 +3,13 @@ module DependencyProxyHelpers include StubRequests - def stub_registry_auth(image, token, status = 200, body = nil) + def stub_registry_auth(image, token, status: 200, body: nil, request_headers: {}) auth_body = { 'token' => token }.to_json auth_link = registry.auth_url(image) - stub_full_request(auth_link) - .to_return(status: status, body: body || auth_body) + stub = stub_full_request(auth_link) + stub = stub.with(headers: request_headers) unless request_headers.empty? + stub.to_return(status: status, body: body || auth_body) end def stub_manifest_download(image, tag, status: 200, body: nil, headers: {}) diff --git a/spec/support/shared_examples/services/dependency_proxy_settings_shared_examples.rb b/spec/support/shared_examples/services/dependency_proxy_settings_shared_examples.rb index 2c1dc2da560..ebcbcd6f6ff 100644 --- a/spec/support/shared_examples/services/dependency_proxy_settings_shared_examples.rb +++ b/spec/support/shared_examples/services/dependency_proxy_settings_shared_examples.rb @@ -2,7 +2,24 @@ RSpec.shared_examples 'updating the dependency proxy group settings attributes' do |from: {}, to: {}| it 'updates the dependency proxy settings' do + old_identity = group_settings.identity + old_secret = group_settings.secret + expect { subject } .to change { group_settings.reload.enabled }.from(from[:enabled]).to(to[:enabled]) + + if from[:identity] && to[:identity] + expect(old_identity).to eq(from[:identity]) + expect(group_settings.identity).to eq(to[:identity]) + else + expect(group_settings.identity).to eq(old_identity) + end + + if from[:secret] && to[:secret] + expect(old_secret).to eq(from[:secret]) + expect(group_settings.secret).to eq(to[:secret]) + else + expect(group_settings.secret).to eq(old_secret) + end end end