diff --git a/app/models/ci/job_token/scope.rb b/app/models/ci/job_token/scope.rb index e2493e4e3b6b5ab107fa12aae185684e9addc9b3..98b944941af0595e72281f0b66e6afb2dee81c12 100644 --- a/app/models/ci/job_token/scope.rb +++ b/app/models/ci/job_token/scope.rb @@ -51,6 +51,10 @@ def groups_count groups.count end + def self_referential?(accessed_project) + current_project.id == accessed_project.id + end + private def outbound_accessible?(accessed_project) @@ -94,10 +98,6 @@ def inbound_allowlist def outbound_allowlist Ci::JobToken::Allowlist.new(current_project, direction: :outbound) end - - def self_referential?(accessed_project) - current_project.id == accessed_project.id - end end end end diff --git a/app/models/project.rb b/app/models/project.rb index 0485b4b93e56b7eb0ea225be836af08193aceb9f..626e67af608132a9fe195cc3f093c1436207639b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -543,6 +543,7 @@ def self.integration_association_name(name) with_options prefix: :ci do delegate :pipeline_variables_minimum_override_role, :pipeline_variables_minimum_override_role= + delegate :push_repository_for_job_token_allowed, :push_repository_for_job_token_allowed= delegate :default_git_depth, :default_git_depth= delegate :forward_deployment_enabled, :forward_deployment_enabled= delegate :forward_deployment_rollback_allowed, :forward_deployment_rollback_allowed= @@ -3108,6 +3109,12 @@ def override_pipeline_variables_allowed?(access_level) ci_cd_settings.override_pipeline_variables_allowed?(access_level) end + def ci_push_repository_for_job_token_allowed? + return false unless ci_cd_settings + + ci_cd_settings.push_repository_for_job_token_allowed? + end + def keep_latest_artifacts_available? return false unless ci_cd_settings diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f47861ca9148f9018549fc1b847e1d361b6eb3fa..eaaa397d9e2601ecc6814bdcb2cd6db3ba69894a 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -250,6 +250,10 @@ class ProjectPolicy < BasePolicy end end + condition(:push_repository_for_job_token_allowed) do + @user&.from_ci_job_token? && project.ci_push_repository_for_job_token_allowed? && @user.ci_job_token_scope.self_referential?(project) + end + condition(:packages_disabled, scope: :subject) { !@subject.packages_enabled } condition(:runner_registration_token_enabled, scope: :subject) { @subject.namespace.allow_runner_registration_token? } @@ -707,6 +711,7 @@ class ProjectPolicy < BasePolicy end rule { repository_disabled }.policy do + prevent :build_push_code prevent :push_code prevent :download_code prevent :build_download_code @@ -747,6 +752,10 @@ class ProjectPolicy < BasePolicy prevent :public_user_access end + rule { can?(:developer_access) & push_repository_for_job_token_allowed }.policy do + enable :build_push_code + end + rule { public_or_internal & job_token_container_registry }.policy do enable :build_read_container_image enable :read_container_image diff --git a/db/migrate/20240506164707_add_push_repository_for_job_token_allowed_to_project_ci_cd_settings.rb b/db/migrate/20240506164707_add_push_repository_for_job_token_allowed_to_project_ci_cd_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..8f040c6c6949e00545f677964a33073b65627466 --- /dev/null +++ b/db/migrate/20240506164707_add_push_repository_for_job_token_allowed_to_project_ci_cd_settings.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddPushRepositoryForJobTokenAllowedToProjectCiCdSettings < Gitlab::Database::Migration[2.2] + milestone '17.1' + + def change + add_column :project_ci_cd_settings, :push_repository_for_job_token_allowed, + :boolean, default: false, null: false + end +end diff --git a/db/schema_migrations/20240506164707 b/db/schema_migrations/20240506164707 new file mode 100644 index 0000000000000000000000000000000000000000..ebea646306737c0748fb32f70b641614288dc1c7 --- /dev/null +++ b/db/schema_migrations/20240506164707 @@ -0,0 +1 @@ +c9db446f16f9e00f80daf6fdb94a9010d7fd10af6369fd101acfc9962a751fef \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9117ab647feffa6ce149a7b88f435034488b0a27..0be5965c867d12a6218a8c4321b24f9f1a949ce0 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14508,7 +14508,8 @@ CREATE TABLE project_ci_cd_settings ( forward_deployment_rollback_allowed boolean DEFAULT true NOT NULL, merge_trains_skip_train_allowed boolean DEFAULT false NOT NULL, restrict_pipeline_cancellation_role smallint DEFAULT 0 NOT NULL, - pipeline_variables_minimum_override_role smallint DEFAULT 3 NOT NULL + pipeline_variables_minimum_override_role smallint DEFAULT 3 NOT NULL, + push_repository_for_job_token_allowed boolean DEFAULT false NOT NULL ); CREATE SEQUENCE project_ci_cd_settings_id_seq diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index c822099fba63cf1f09aa5dd23116998a8d469a54..fc4b00881ccc2387abd483a09cefd9e4750bdfc0 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -103,6 +103,7 @@ 'separated_caches' => 'ci_', 'allow_fork_pipelines_to_run_in_parent_project' => 'ci_', 'inbound_job_token_scope_enabled' => 'ci_', + 'push_repository_for_job_token_allowed' => 'ci_', 'job_token_scope_enabled' => 'ci_outbound_', # EE only 'auto_rollback_enabled' => '', diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index ec9a514e89895acc81a66fefd45dec6533ecf7e1..ee77ebe59c911eaad4dc2c432a1c80291f2e8610 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -359,6 +359,7 @@ def build_authentication_abilities [ :read_project, :build_download_code, + :build_push_code, :build_read_container_image, :build_create_container_image, :build_destroy_container_image diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 0cfef4ed802eb56ef7e3950fef80474562d03cd7..b37f29a208318870292d781bd7f3132695b4623e 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -144,6 +144,10 @@ def build_can_download_code? authentication_abilities.include?(:build_download_code) && user_access.can_do_action?(:build_download_code) end + def build_can_push? + authentication_abilities.include?(:build_push_code) && user_access.can_do_action?(:build_push_code) + end + def build_can_download? build_can_download_code? end @@ -231,7 +235,7 @@ def check_authentication_abilities! raise ForbiddenError, error_message(:auth_download) end when *PUSH_COMMANDS - unless authentication_abilities.include?(:push_code) + unless authentication_abilities.include?(:push_code) || authentication_abilities.include?(:build_push_code) raise ForbiddenError, error_message(:auth_upload) end end @@ -334,12 +338,14 @@ def check_push_access! end def user_can_push? - user_access.can_do_action?(push_ability) + authentication_abilities.include?(:push_code) && + user_access.can_do_action?(push_ability) end def check_change_access! if changes == ANY can_push = deploy_key? || + build_can_push? || user_can_push? || project&.any_branch_allows_collaboration?(user_access.user) diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 6498428b642f54e97bfda33deff4d4dca6e7bb19..e2feda95b7754f3227c0c5f571a47423b1eeff30 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -6,6 +6,7 @@ include TermsHelper include AdminModeHelper include ExternalAuthorizationServiceHelpers + include Ci::JobTokenScopeHelpers let(:user) { create(:user) } let(:actor) { user } @@ -1279,6 +1280,54 @@ def self.run_permission_checks(permissions_matrix) end end end + + describe 'when request is made from CI' do + let(:auth_result_type) { :build } + let(:job) { build_stubbed(:ci_build, project: project, user: user) } + + before do + accept_terms(user) + project.add_maintainer(user) + project.ci_push_repository_for_job_token_allowed = ci_push_repository_for_job_token_allowed + project.save! + + allow(user).to receive(:ci_job_token_scope).and_return(user.set_ci_job_token_scope!(job)) + end + + context 'when push to repositry is allowed by project settings' do + let(:ci_push_repository_for_job_token_allowed) { true } + + it "doesn't block push" do + expect { push_access_check_build(project, changes) }.not_to raise_error + end + + context 'when push is requested to a different project' do + let(:another_project) do + create(:project, :repository, :public).tap do |accessible_project| + add_inbound_accessible_linkage(project, accessible_project) + end + end + + before do + another_project.add_maintainer(user) + another_project.ci_push_repository_for_job_token_allowed = ci_push_repository_for_job_token_allowed + another_project.save! + end + + it 'raises forbidden exception on push' do + expect { push_access_check_build(another_project, changes) }.to raise_error(Gitlab::GitAccess::ForbiddenError) + end + end + end + + context 'when push to repositry is not allowed by project settings' do + let(:ci_push_repository_for_job_token_allowed) { false } + + it 'raises forbidden exception on push' do + expect { push_access_check_build(project, changes) }.to raise_error(Gitlab::GitAccess::ForbiddenError) + end + end + end end private @@ -1290,6 +1339,14 @@ def access redirected_path: redirected_path, auth_result_type: auth_result_type) end + def push_access_check_build(access_project, changes) + access_class.new(actor, access_project, protocol, + authentication_abilities: build_authentication_abilities_allowed_push, + repository_path: "#{access_project.full_path}.git", + redirected_path: redirected_path, auth_result_type: auth_result_type) + .check('git-receive-pack', changes) + end + def push_changes(changes) access.check('git-receive-pack', changes) end @@ -1309,6 +1366,14 @@ def build_authentication_abilities ] end + def build_authentication_abilities_allowed_push + [ + :read_project, + :build_download_code, + :build_push_code + ] + end + def full_authentication_abilities [ :read_project, diff --git a/spec/models/ci/job_token/scope_spec.rb b/spec/models/ci/job_token/scope_spec.rb index 8a3e8011c9ff723b71bf6dc5fbd3eabbaad8fcd0..e0257e05bda888e46ed35c229fcb4343f6ac3e8f 100644 --- a/spec/models/ci/job_token/scope_spec.rb +++ b/spec/models/ci/job_token/scope_spec.rb @@ -176,4 +176,20 @@ end end end + + describe '#self_referential?' do + subject { scope.self_referential?(access_project) } + + context 'when a current project requested' do + let(:access_project) { current_project } + + it { is_expected.to be_truthy } + end + + context 'when a different project requested' do + let_it_be(:access_project) { create(:project) } + + it { is_expected.to be_falsey } + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 50baa68a8082947d9eff19b68423c3765888d88c..0ce3e1fe033582cca7c8384b4d5d2daf78b33a29 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1246,6 +1246,7 @@ 'separated_caches' => 'ci_', 'allow_fork_pipelines_to_run_in_parent_project' => 'ci_', 'inbound_job_token_scope_enabled' => 'ci_', + 'push_repository_for_job_token_allowed' => 'ci_', 'job_token_scope_enabled' => 'ci_outbound_' } end @@ -1298,6 +1299,12 @@ end end + describe '#ci_push_repository_for_job_token_allowed?' do + it_behaves_like 'a ci_cd_settings predicate method', prefix: 'ci_' do + let(:delegated_method) { :push_repository_for_job_token_allowed? } + end + end + describe '#keep_latest_artifacts_available?' do it_behaves_like 'a ci_cd_settings predicate method' do let(:delegated_method) { :keep_latest_artifacts_available? } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index cf849e9d3d981c858e84f38ef0e77ae1457b84ab..428648e3bed8916f744c9886889bd8a25f5aa659 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -3780,6 +3780,69 @@ def permissions_abilities(role) end end + describe 'build_push_code' do + using RSpec::Parameterized::TableSyntax + + let(:policy) { :build_push_code } + + where(:user_role, :project_visibility, :push_repository_for_job_token_allowed, :self_referential_project, :allowed) do + :maintainer | :public | true | true | true + :owner | :public | true | true | true + :maintainer | :private | true | true | true + :developer | :public | true | true | true + :reporter | :public | true | true | false + :guest | :public | true | true | false + :guest | :private | true | true | false + :guest | :internal | true | true | false + :anonymous | :public | true | true | false + :maintainer | :public | false | true | false + :maintainer | :public | true | false | false + :maintainer | :public | false | false | false + end + + with_them do + let(:current_user) do + public_send(user_role) + end + + let(:job) { build_stubbed(:ci_build, project: scope_project, user: current_user) } + let(:project) { public_send("#{project_visibility}_project") } + let(:self_referential_job) { build_stubbed(:ci_build, project: project, user: current_user) } + let(:scope_project) { public_send(:private_project) } + + before do + project.add_guest(guest) + project.add_reporter(reporter) + project.add_developer(developer) + project.add_maintainer(maintainer) + project.add_maintainer(owner) + + project.ci_inbound_job_token_scope_enabled = true + project.save! + + ci_cd_settings = project.ci_cd_settings + ci_cd_settings.push_repository_for_job_token_allowed = push_repository_for_job_token_allowed + ci_cd_settings.save! + + if user_role != :anonymous + if self_referential_project + allow(current_user).to receive(:ci_job_token_scope).and_return(current_user.set_ci_job_token_scope!(self_referential_job)) + else + allow(current_user).to receive(:ci_job_token_scope).and_return(current_user.set_ci_job_token_scope!(job)) + end + end + end + + it 'allows/disallows build_push_code' do + if allowed + is_expected.to be_allowed(:build_push_code) + else + is_expected.to be_disallowed(:build_push_code) + end + end + end + end + private def project_subject(project_type) diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 60aa7d51060e00d2bfbdd4fd98927f1a4c2f60b5..c5d798f004f11dad5ea554946c3be7adc07b0005 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -100,6 +100,7 @@ ci_cd_settings: - auto_rollback_enabled - inbound_job_token_scope_enabled - restrict_pipeline_cancellation_role + - push_repository_for_job_token_allowed remapped_attributes: pipeline_variables_minimum_override_role: ci_pipeline_variables_minimum_override_role default_git_depth: ci_default_git_depth diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 68d18200df705e7e892d65ecc3956636c84f9273..a080e121b9f5302c8f0637d599a48de56ee5d1a7 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -904,7 +904,7 @@ def attempt_login(include_password) it 'rejects pushes' do push_get(path, **env) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) end def pull @@ -938,7 +938,7 @@ def pull push_get(path, **env) expect(response).to have_gitlab_http_status(:forbidden) - expect(response.body).to eq(git_access_error(:auth_upload)) + expect(response.body).to eq(git_access_error(:push_code)) end end @@ -1509,7 +1509,7 @@ def attempt_login(include_password) it 'rejects pushes' do push_get(path, **env) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) end end @@ -1539,7 +1539,7 @@ def attempt_login(include_password) push_get(path, **env) expect(response).to have_gitlab_http_status(:forbidden) - expect(response.body).to eq(git_access_error(:auth_upload)) + expect(response.body).to eq(git_access_error(:push_code)) end end