diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 85bdeb07b00b8ca26bd2b93969887fa802e9918b..c675e4bb61c7ab3d84bc2a88a5f638e7e002fbcc 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -39,6 +39,8 @@ def download return render_404 unless artifact_file log_artifacts_filesize(artifact_file.model) + audit_download(build, artifact_file.filename) + send_upload(artifact_file, attachment: artifact_file.filename, proxy: params[:proxy]) end @@ -106,6 +108,10 @@ def latest_succeeded private + def audit_download(build, filename) + # overridden in EE + end + def extract_ref_name_and_path return unless params[:ref_name_and_path] @@ -184,3 +190,5 @@ def authorize_read_job_artifacts! return access_denied! unless can?(current_user, :read_job_artifacts, job_artifact) end end + +Projects::ArtifactsController.prepend_mod diff --git a/doc/administration/audit_event_streaming/audit_event_types.md b/doc/administration/audit_event_streaming/audit_event_types.md index ff99fc1f6c64e874e64e3d3415354ed9c89cb1f3..d89b0587e9e42d56e1292f4dc09179286270bee3 100644 --- a/doc/administration/audit_event_streaming/audit_event_types.md +++ b/doc/administration/audit_event_streaming/audit_event_types.md @@ -64,6 +64,12 @@ Audit event types belong to the following product categories. | [`update_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74632) | Event triggered when an external audit event destination is updated| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/344664) | | [`update_instance_event_streaming_destination`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125846) | Event triggered when an instance level external audit event destination is updated| **{check-circle}** Yes | **{check-circle}** Yes | GitLab [16.2](https://gitlab.com/gitlab-org/gitlab/-/issues/404730) | +### Build artifacts + +| Name | Description | Saved to database | Streamed | Introduced in | +|:-----|:------------|:------------------|:---------|:--------------| +| [`job_artifact_downloaded`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129608) | Triggered when a user download a job artifact from a project| **{dotted-circle}** No | **{check-circle}** Yes | GitLab [16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/250663) | + ### Code review | Name | Description | Saved to database | Streamed | Introduced in | diff --git a/ee/app/controllers/ee/projects/artifacts_controller.rb b/ee/app/controllers/ee/projects/artifacts_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..b9ed2812655c2036502e6d86f7feba16bc0dec12 --- /dev/null +++ b/ee/app/controllers/ee/projects/artifacts_controller.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module EE + module Projects + module ArtifactsController + extend ::Gitlab::Utils::Override + + private + + override :audit_download + def audit_download(build, filename) + super + + Audit::Ci::ArtifactDownloadAuditor.new( + current_user: current_user, + build: build, + artifact: job_artifact, + filename: filename + ).execute + end + end + end +end diff --git a/ee/config/audit_events/types/job_artifact_downloaded.yml b/ee/config/audit_events/types/job_artifact_downloaded.yml new file mode 100644 index 0000000000000000000000000000000000000000..e38728ab085faeb422508b61f1e456ac1d3b9139 --- /dev/null +++ b/ee/config/audit_events/types/job_artifact_downloaded.yml @@ -0,0 +1,9 @@ +--- +name: job_artifact_downloaded +description: Triggered when a user download a job artifact from a project +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/250663 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129608 +feature_category: build_artifacts +milestone: '16.8' +saved_to_database: false +streamed: true diff --git a/ee/lib/audit/ci/artifact_download_auditor.rb b/ee/lib/audit/ci/artifact_download_auditor.rb new file mode 100644 index 0000000000000000000000000000000000000000..effabe09d9207c04526036ac70bd27e5867282cc --- /dev/null +++ b/ee/lib/audit/ci/artifact_download_auditor.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Audit + module Ci + class ArtifactDownloadAuditor + NAME = 'job_artifact_downloaded' + UNKNOWN_FILENAME = 'unknown' + NEVER = 'never' + + attr_reader :current_user, :build + + def initialize(build:, filename:, artifact: nil, current_user: nil) + @current_user = current_user + @build = build + @filename = filename + @artifact = artifact + end + + def execute + return unless job_artifact + + ::Gitlab::Audit::Auditor.audit( + name: NAME, + author: author, + scope: build.project, + target: target, + message: message, + additional_details: { + expire_at: expiry, + filename: filename, + artifact_id: job_artifact.id, + artifact_type: job_artifact.class.name + } + ) + end + + private + + def author + return Gitlab::Audit::UnauthenticatedAuthor.new if current_user.nil? + + current_user + end + + def filename + @filename.presence || UNKNOWN_FILENAME + end + + def message + "Downloaded artifact #{filename} (expiration: #{expiry})" + end + + def job_artifact + @artifact || build.job_artifacts_archive + end + + def expiry + expire_at = job_artifact&.expire_at.presence + + return NEVER unless expire_at + + expire_at.iso8601 + end + + def target + build.pipeline || build + end + end + end +end diff --git a/ee/lib/ee/api/ci/helpers/runner.rb b/ee/lib/ee/api/ci/helpers/runner.rb index bcea232dd497e34b203bc4294b1d5e2d1c8b5733..c029a0858a9a041c24f7907a87edd89496e6ccbd 100644 --- a/ee/lib/ee/api/ci/helpers/runner.rb +++ b/ee/lib/ee/api/ci/helpers/runner.rb @@ -11,6 +11,17 @@ module Runner def track_ci_minutes_usage!(build, runner) ::Ci::Minutes::TrackLiveConsumptionService.new(build).execute end + + override :audit_download + def audit_download(build, filename) + super + + Audit::Ci::ArtifactDownloadAuditor.new( + current_user: current_user, + build: build, + filename: filename + ).execute + end end end end diff --git a/ee/lib/ee/api/ci/job_artifacts.rb b/ee/lib/ee/api/ci/job_artifacts.rb index c4752f2a6bfd5cee9acb7d08b337bdaf3841420d..a49ad9d50d3ccd25164e4afe3c1442a4f7a1599f 100644 --- a/ee/lib/ee/api/ci/job_artifacts.rb +++ b/ee/lib/ee/api/ci/job_artifacts.rb @@ -8,8 +8,19 @@ module JobArtifacts prepended do helpers do + def audit_download(build, filename) + super + + Audit::Ci::ArtifactDownloadAuditor.new( + current_user: current_user, + build: build, + filename: filename + ).execute + end + def authorize_download_artifacts! super + check_cross_project_pipelines_feature! end diff --git a/ee/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/ee/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9a98aeddcaaa040d45682ad1179478ad9831372d --- /dev/null +++ b/ee/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "User downloads artifacts", feature_category: :build_artifacts do + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:pipeline) { create(:ci_empty_pipeline, status: :success, sha: project.commit.id, project: project) } + let_it_be(:job) { create(:ci_build, :artifacts, :success, pipeline: pipeline) } + + shared_examples "downloading" do + it "audits the download" do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(name: 'job_artifact_downloaded')) + + visit(url) + end + end + + context "when downloading" do + before do + stub_licensed_features( + admin_audit_log: true, + audit_events: true, + extended_audit_events: true, + external_audit_events: true) + end + + context "with job id" do + let(:url) { download_project_job_artifacts_path(project, job) } + + it_behaves_like "downloading" + end + + context "with branch name and job name" do + let(:url) { latest_succeeded_project_artifacts_path(project, "#{pipeline.ref}/download", job: job.name) } + + it_behaves_like "downloading" + end + + context "with SHA" do + let(:url) { latest_succeeded_project_artifacts_path(project, "#{pipeline.sha}/download", job: job.name) } + + it_behaves_like "downloading" + end + + context "when unauthorized" do + let(:url) { download_project_job_artifacts_path(project, job) } + + before do + job.job_artifacts.update_all(accessibility: 'private') + end + + it 'does not audit when no-one can download' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + visit(url) + end + end + end +end diff --git a/ee/spec/lib/audit/ci/artifact_download_auditor_spec.rb b/ee/spec/lib/audit/ci/artifact_download_auditor_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b18342597937eebf244bd4eaeaf3b465ff3e250a --- /dev/null +++ b/ee/spec/lib/audit/ci/artifact_download_auditor_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Audit::Ci::ArtifactDownloadAuditor, feature_category: :compliance_management do + let_it_be(:build) { create :ci_build, :artifacts } + + describe '#execute' do + context 'with filename' do + it 'includes given filename in additional_details' do + filename = 'ci_build_artifacts.zip' + + expect(Gitlab::Audit::Auditor).to receive(:audit) + .with(hash_including(additional_details: hash_including({ filename: filename }))) + + described_class.new(build: build, filename: build.job_artifacts_archive.filename).execute + end + end + + context 'without filename' do + it 'defaults to UNKNOWN_FILENAME in additional_details' do + expect(Gitlab::Audit::Auditor).to receive(:audit) + .with(hash_including(additional_details: hash_including({ filename: described_class::UNKNOWN_FILENAME }))) + + described_class.new(build: build, filename: nil).execute + end + end + + context 'without expiry' do + it 'includes message with unlimited expiration' do + expect(Gitlab::Audit::Auditor).to receive(:audit) + .with(hash_including(name: described_class::NAME, + message: 'Downloaded artifact ci_build_artifacts.zip (expiration: never)', + additional_details: hash_including(expire_at: 'never') + )).and_call_original + + described_class.new(build: build, filename: build.job_artifacts_archive.filename).execute + end + end + + context 'with expiry', time_travel_to: Time.zone.parse("2022-02-22 00:00:00 UTC+0") do + let(:artifact) { create :ci_job_artifact, :archive, expire_at: Time.zone.now } + let(:build) { artifact.job } + + it 'includes message with unlimited expiration' do + artifact + never_expire_msg = 'Downloaded artifact ci_build_artifacts.zip (expiration: 2022-02-22T00:00:00Z)' + + expect(Gitlab::Audit::Auditor).to receive(:audit).with(hash_including(message: never_expire_msg)) + + described_class.new(build: build, filename: build.job_artifacts_archive.filename).execute + end + end + end +end diff --git a/ee/spec/lib/ee/api/helpers_spec.rb b/ee/spec/lib/ee/api/helpers_spec.rb index f553fe824cd853ae18b9e54b1b4aebee2e5bf190..1fa7883662ad84e5ad9cf14282705080d59732a9 100644 --- a/ee/spec/lib/ee/api/helpers_spec.rb +++ b/ee/spec/lib/ee/api/helpers_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe EE::API::Helpers do +RSpec.describe EE::API::Helpers, feature_category: :shared do include Rack::Test::Methods let(:helper) do diff --git a/ee/spec/requests/api/ci/job_artifacts_spec.rb b/ee/spec/requests/api/ci/job_artifacts_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c7ac9a0b84f3c7b7efd04d8f117829383a0802d1 --- /dev/null +++ b/ee/spec/requests/api/ci/job_artifacts_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do + include HttpBasicAuthHelpers + include DependencyProxyHelpers + include Ci::JobTokenScopeHelpers + include HttpIOHelpers + + let_it_be(:project, reload: true) do + create(:project, :repository, :in_group, public_builds: false) + end + + let_it_be(:pipeline, reload: true) do + create(:ci_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) + end + + let(:user) { create(:user) } + let(:api_user) { user } + let(:guest) { create(:project_member, :guest, project: project).user } + let!(:job) { create(:ci_build, :artifacts, pipeline: pipeline, project: project) } + + before do + project.add_developer(user) + end + + describe 'GET /projects/:id/jobs/:job_id/artifacts' do + context 'with job artifacts' do + context 'with audit events enabled', :aggregate_failures do + before do + project.group.root_ancestor.external_audit_event_destinations.create!(destination_url: 'http://example.com') + stub_licensed_features(admin_audit_log: true, extended_audit_events: true, external_audit_events: true) + end + + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, project: project) } + + subject(:request_artifact) { get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) } + + it 'audits downloads' do + expect(::Gitlab::Audit::Auditor).to( + receive(:audit).with(hash_including(name: 'job_artifact_downloaded')).and_call_original + ) + + request_artifact + end + end + end + end +end diff --git a/ee/spec/requests/api/ci/runner_spec.rb b/ee/spec/requests/api/ci/runner_spec.rb index 1e1e403e3440f4f5bb7715aa225a6ac775163a56..48153d7e8c7b2fa2fe50ba589d658695577eef9f 100644 --- a/ee/spec/requests/api/ci/runner_spec.rb +++ b/ee/spec/requests/api/ci/runner_spec.rb @@ -5,7 +5,7 @@ RSpec.describe API::Ci::Runner, feature_category: :runner do include Ci::JobTokenScopeHelpers - let_it_be_with_reload(:project) { create(:project, :repository) } + let_it_be_with_reload(:project) { create(:project, :repository, :in_group) } let_it_be(:user) { create(:user) } let_it_be(:ref) { 'master' } @@ -136,7 +136,21 @@ def request_job(token = runner.token, **params) end shared_examples 'successful artifact download' do + before do + project.group.root_ancestor.external_audit_event_destinations.create!(destination_url: 'http://example.com') + stub_licensed_features(admin_audit_log: true, extended_audit_events: true, external_audit_events: true) + end + it 'downloads artifacts' do + expect(::Gitlab::Audit::Auditor).to( + receive(:audit).with(hash_including(name: 'job_artifact_downloaded')).and_call_original + ) + expect(AuditEvents::AuditEventStreamingWorker).to( + receive(:perform_async) + .with('job_artifact_downloaded', nil, a_string_including("Downloaded artifact")) + .and_call_original + ) + download_artifact expect(response).to have_gitlab_http_status(:ok) diff --git a/lib/api/ci/helpers/runner.rb b/lib/api/ci/helpers/runner.rb index 382528c814c41195324d3636ab8dff0c6a9530e4..02a0e6bd7221608af12641ead8ae8bd1df9c20ef 100644 --- a/lib/api/ci/helpers/runner.rb +++ b/lib/api/ci/helpers/runner.rb @@ -140,6 +140,10 @@ def track_ci_minutes_usage!(_build, _runner) # noop: overridden in EE end + def audit_download(build, filename) + # noop: overridden in EE + end + def check_if_backoff_required! return unless Gitlab::Database::Migrations::RunnerBackoff::Communicator.backoff_runner? diff --git a/lib/api/ci/job_artifacts.rb b/lib/api/ci/job_artifacts.rb index 3788f5bec410dfaa315c65e3690216b9e431e98e..cdc318894080e138e29ccf346a3106f620e2d8a0 100644 --- a/lib/api/ci/job_artifacts.rb +++ b/lib/api/ci/job_artifacts.rb @@ -14,6 +14,8 @@ class JobArtifacts < ::API::Base def authorize_download_artifacts! authorize_read_builds! end + + def audit_download(build, filename); end end params do @@ -44,7 +46,7 @@ def authorize_download_artifacts! latest_build = user_project.latest_successful_build_for_ref!(params[:job], params[:ref_name]) authorize_read_job_artifacts!(latest_build) - + audit_download(latest_build, latest_build.artifacts_file.filename) present_artifacts_file!(latest_build.artifacts_file) end @@ -104,7 +106,7 @@ def authorize_download_artifacts! build = find_build!(params[:job_id]) authorize_read_job_artifacts!(build) - + audit_download(build, build.artifacts_file&.filename) if build.artifacts_file present_artifacts_file!(build.artifacts_file) end diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index 585e9f962a3ff9b6e4934a11be5922a9b910e17e..1ea62e03fbab69cab770b704ee277575d8f11d8f 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -386,6 +386,7 @@ class Runner < ::API::Base get '/:id/artifacts', feature_category: :build_artifacts do authenticate_job_via_dependent_job! + audit_download(current_job, current_job.artifacts_file&.filename) if current_job.artifacts_file present_artifacts_file!(current_job.artifacts_file, supports_direct_download: params[:direct_download]) end end diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index a0548e847a0f7ddc2652fa50f71b8f0464742750..78cd32de5603b441657f0691b01a2f3196a24291 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -104,7 +104,8 @@ def download_artifact(extra_params = {}) download_artifact - expect(response.headers['Content-Disposition']).to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + expect(response.headers['Content-Disposition']) + .to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end @@ -135,7 +136,8 @@ def download_artifact(extra_params = {}) download_artifact(file_type: 'archive') expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Content-Disposition']).to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + expect(response.headers['Content-Disposition']) + .to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end end @@ -168,7 +170,8 @@ def download_artifact(extra_params = {}) download_artifact(file_type: file_type) - expect(response.headers['Content-Disposition']).to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) + expect(response.headers['Content-Disposition']) + .to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end end @@ -182,7 +185,8 @@ def download_artifact(extra_params = {}) end it 'sends the codequality report' do - expect(Gitlab::ApplicationContext).to receive(:push).with(artifact: an_instance_of(Ci::JobArtifact)).and_call_original + expect(Gitlab::ApplicationContext) + .to receive(:push).with(artifact: an_instance_of(Ci::JobArtifact)).and_call_original expect(controller).to receive(:redirect_to).and_call_original @@ -212,7 +216,8 @@ def download_artifact(extra_params = {}) end it 'redirects to a Google CDN request' do - expect(Gitlab::ApplicationContext).to receive(:push).with(artifact: an_instance_of(Ci::JobArtifact)).and_call_original + expect(Gitlab::ApplicationContext) + .to receive(:push).with(artifact: an_instance_of(Ci::JobArtifact)).and_call_original expect(Gitlab::ApplicationContext).to receive(:push).with(artifact_used_cdn: true).and_call_original download_artifact(file_type: file_type) diff --git a/spec/lib/api/ci/helpers/runner_spec.rb b/spec/lib/api/ci/helpers/runner_spec.rb index ee0a58a4e5308262db03acfcd8e155e7556e8418..2e4551f5eb901d4f0fd8cd8e13c431e3406a6338 100644 --- a/spec/lib/api/ci/helpers/runner_spec.rb +++ b/spec/lib/api/ci/helpers/runner_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Ci::Helpers::Runner do +RSpec.describe API::Ci::Helpers::Runner, feature_category: :runner do let(:helper) do Class.new do include API::Ci::Helpers::Runner