Merge branch 'ci_job_token_delete_registry_images' into 'master'

Allow access to registry API of the current project using the job token [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!49750

changelogs/unreleased/ci_job_token_delete_registry_images.yml

+---
+title: Allow access to registry API of the current project using the job token
+merge_request: 49750
+author: Mathieu Parent
+type: added

config/feature_flags/development/ci_job_token_scope.yml

+---
+name: ci_job_token_scope
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49750
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300821
+milestone: '13.12'
+type: development
+group: group::package
+default_enabled: false

doc/api/README.md

@@ -208,6 +208,7 @@ You can use a GitLab CI/CD job token to authenticate with specific API endpoints
     Package Registry, you can use [deploy tokens](../user/project/deploy_tokens/index.md).
   - [Container Registry](../user/packages/container_registry/index.md)
     (the `$CI_REGISTRY_PASSWORD` is `$CI_JOB_TOKEN`).
+  - [Container Registry API](container_registry.md) (scoped to the job's project, when the `ci_job_token_scope` feature flag is enabled)
 - [Get job artifacts](job_artifacts.md#get-job-artifacts).
 - [Get job token's job](jobs.md#get-job-tokens-job).
 - [Pipeline triggers](pipeline_triggers.md), using the `token=` parameter.

doc/api/container_registry.md

@@ -6,10 +6,30 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 # Container Registry API
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/55978) in GitLab 11.8.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/55978) in GitLab 11.8.
+> - The use of `CI_JOB_TOKEN` scoped to the current project was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49750) in GitLab 13.12.
 
 This is the API documentation of the [GitLab Container Registry](../user/packages/container_registry/index.md).
 
+When the `ci_job_token_scope` feature flag is enabled (it is **disabled by default**), you can use the below endpoints
+from a CI/CD job, by passing the `$CI_JOB_TOKEN` variable as the `JOB-TOKEN` header.
+The job token will only have access to its own project.
+
+[GitLab administrators with access to the GitLab Rails console](../administration/feature_flags.md)
+can opt to enable it.
+
+To enable it:
+
+```ruby
+Feature.enable(:ci_job_token_scope)
+```
+
+To disable it:
+
+```ruby
+Feature.disable(:ci_job_token_scope)
+```
+
 ## List registry repositories
 
 ### Within a project

ee/lib/ee/api/helpers.rb

@@ -87,6 +87,8 @@ module EE
       def find_project!(id)
         project = find_project(id)
 
+        return forbidden! unless authorized_project_scope?(project)
+
         # CI job token authentication:
         # this method grants limited privileged for admin users
         # admin users can only access project if they are direct member

lib/api/helpers.rb

@@ -124,12 +124,22 @@ module API
     def find_project!(id)
       project = find_project(id)
 
+      return forbidden! unless authorized_project_scope?(project)
+
       return project if can?(current_user, :read_project, project)
       return unauthorized! if authenticate_non_public?
 
       not_found!('Project')
     end
 
+    def authorized_project_scope?(project)
+      return true unless job_token_authentication?
+      return true unless route_authentication_setting[:job_token_scope] == :project
+
+      ::Feature.enabled?(:ci_job_token_scope, project, default_enabled: :yaml) &&
+        current_authenticated_job.project == project
+    end
+
     # rubocop: disable CodeReuse/ActiveRecord
     def find_group(id)
       if id.to_s =~ /^\d+$/

lib/api/project_container_repositories.rb

@@ -15,6 +15,7 @@ module API
     params do
       requires :id, type: String, desc: 'The ID of a project'
     end
+    route_setting :authentication, job_token_allowed: true, job_token_scope: :project
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       desc 'Get a project container repositories' do
         detail 'This feature was introduced in GitLab 11.8.'

spec/lib/api/helpers_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe API::Helpers do
+  using RSpec::Parameterized::TableSyntax
+
   subject { Class.new.include(described_class).new }
 
   describe '#find_project' do
@@ -99,6 +101,59 @@ RSpec.describe API::Helpers do
     end
   end
 
+  describe '#find_project!' do
+    let_it_be(:project) { create(:project) }
+
+    let(:user) { project.owner}
+
+    before do
+      allow(subject).to receive(:current_user).and_return(user)
+      allow(subject).to receive(:authorized_project_scope?).and_return(true)
+      allow(subject).to receive(:job_token_authentication?).and_return(false)
+      allow(subject).to receive(:authenticate_non_public?).and_return(false)
+    end
+
+    shared_examples 'project finder' do
+      context 'when project exists' do
+        it 'returns requested project' do
+          expect(subject.find_project!(existing_id)).to eq(project)
+        end
+
+        it 'returns nil' do
+          expect(subject).to receive(:render_api_error!).with('404 Project Not Found', 404)
+          expect(subject.find_project!(non_existing_id)).to be_nil
+        end
+      end
+    end
+
+    context 'when ID is used as an argument' do
+      let(:existing_id) { project.id }
+      let(:non_existing_id) { non_existing_record_id }
+
+      it_behaves_like 'project finder'
+    end
+
+    context 'when PATH is used as an argument' do
+      let(:existing_id) { project.full_path }
+      let(:non_existing_id) { 'something/else' }
+
+      it_behaves_like 'project finder'
+
+      context 'with an invalid PATH' do
+        let(:non_existing_id) { 'undefined' } # path without slash
+
+        it_behaves_like 'project finder'
+
+        it 'does not hit the database' do
+          expect(Project).not_to receive(:find_by_full_path)
+          expect(subject).to receive(:render_api_error!).with('404 Project Not Found', 404)
+
+          subject.find_project!(non_existing_id)
+        end
+      end
+    end
+  end
+
   describe '#find_namespace' do
     let(:namespace) { create(:namespace) }
 
@@ -191,6 +246,49 @@ RSpec.describe API::Helpers do
     it_behaves_like 'user namespace finder'
   end
 
+  describe '#authorized_project_scope?' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:other_project) { create(:project) }
+    let_it_be(:job) { create(:ci_build) }
+
+    let(:send_authorized_project_scope) { subject.authorized_project_scope?(project) }
+
+    where(:job_token_authentication, :route_setting, :feature_flag, :same_job_project, :expected_result) do
+      false | false | false | false | true
+      false | false | false | true  | true
+      false | false | true  | false | true
+      false | false | true  | true  | true
+      false | true  | false | false | true
+      false | true  | false | true  | true
+      false | true  | true  | false | true
+      false | true  | true  | true  | true
+      true  | false | false | false | true
+      true  | false | false | true  | true
+      true  | false | true  | false | true
+      true  | false | true  | true  | true
+      true  | true  | false | false | false
+      true  | true  | false | true  | false
+      true  | true  | true  | false | false
+      true  | true  | true  | true  | true
+    end
+
+    with_them do
+      before do
+        allow(subject).to receive(:job_token_authentication?).and_return(job_token_authentication)
+        allow(subject).to receive(:route_authentication_setting).and_return(job_token_scope: route_setting ? :project : nil)
+        allow(subject).to receive(:current_authenticated_job).and_return(job)
+        allow(job).to receive(:project).and_return(same_job_project ? project : other_project)
+
+        stub_feature_flags(ci_job_token_scope: false)
+        stub_feature_flags(ci_job_token_scope: project) if feature_flag
+      end
+
+      it 'returns the expected result' do
+        expect(send_authorized_project_scope).to eq(expected_result)
+      end
+    end
+  end
+
   describe '#send_git_blob' do
     let(:repository) { double }
     let(:blob) { double(name: 'foobar') }

spec/requests/api/project_container_repositories_spec.rb

@@ -6,12 +6,14 @@ RSpec.describe API::ProjectContainerRepositories do
   include ExclusiveLeaseHelpers
 
   let_it_be(:project) { create(:project, :private) }
+  let_it_be(:project2) { create(:project, :public) }
   let_it_be(:maintainer) { create(:user) }
   let_it_be(:developer) { create(:user) }
   let_it_be(:reporter) { create(:user) }
   let_it_be(:guest) { create(:user) }
   let(:root_repository) { create(:container_repository, :root, project: project) }
   let(:test_repository) { create(:container_repository, project: project) }
+  let(:root_repository2) { create(:container_repository, :root, project: project2) }
 
   let(:users) do
     {
@@ -24,25 +26,80 @@ RSpec.describe API::ProjectContainerRepositories do
   end
 
   let(:api_user) { maintainer }
+  let(:job) { create(:ci_build, :running, user: api_user, project: project) }
+  let(:job2) { create(:ci_build, :running, user: api_user, project: project2) }
 
-  before do
+  let(:method) { :get }
+  let(:params) { {} }
+
+  before_all do
     project.add_maintainer(maintainer)
     project.add_developer(developer)
     project.add_reporter(reporter)
     project.add_guest(guest)
 
-    stub_container_registry_config(enabled: true)
+    project2.add_maintainer(maintainer)
+    project2.add_developer(developer)
+    project2.add_reporter(reporter)
+    project2.add_guest(guest)
+  end
 
+  before do
     root_repository
     test_repository
+
+    stub_container_registry_config(enabled: true)
+  end
+
+  shared_context 'using API user' do
+    subject { public_send(method, api(url, api_user), params: params) }
+  end
+
+  shared_context 'using job token' do
+    before do
+      stub_exclusive_lease
+      stub_feature_flags(ci_job_token_scope: true)
+    end
+
+    subject { public_send(method, api(url), params: params.merge({ job_token: job.token })) }
+  end
+
+  shared_context 'using job token from another project' do
+    before do
+      stub_exclusive_lease
+      stub_feature_flags(ci_job_token_scope: true)
+    end
+
+    subject { public_send(method, api(url), params: { job_token: job2.token }) }
+  end
+
+  shared_context 'using job token while ci_job_token_scope feature flag is disabled' do
+    before do
+      stub_exclusive_lease
+      stub_feature_flags(ci_job_token_scope: false)
+    end
+
+    subject { public_send(method, api(url), params: params.merge({ job_token: job.token })) }
+  end
+
+  shared_examples 'rejected job token scopes' do
+    include_context 'using job token from another project' do
+      it_behaves_like 'rejected container repository access', :maintainer, :forbidden
+    end
+
+    include_context 'using job token while ci_job_token_scope feature flag is disabled' do
+      it_behaves_like 'rejected container repository access', :maintainer, :forbidden
+    end
   end
 
   describe 'GET /projects/:id/registry/repositories' do
     let(:url) { "/projects/#{project.id}/registry/repositories" }
 
-    subject { get api(url, api_user) }
+    ['using API user', 'using job token'].each do |context|
+      context context do
+        include_context context
 
-    it_behaves_like 'rejected container repository access', :guest, :forbidden
+        it_behaves_like 'rejected container repository access', :guest, :forbidden unless context == 'using job token'
         it_behaves_like 'rejected container repository access', :anonymous, :not_found
         it_behaves_like 'a package tracking event', described_class.name, 'list_repositories'
 
@@ -50,9 +107,18 @@ RSpec.describe API::ProjectContainerRepositories do
           let(:object) { project }
         end
       end
+    end
+
+    include_examples 'rejected job token scopes'
+  end
 
   describe 'DELETE /projects/:id/registry/repositories/:repository_id' do
-    subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}", api_user) }
+    let(:method) { :delete }
+    let(:url) { "/projects/#{project.id}/registry/repositories/#{root_repository.id}" }
+
+    ['using API user', 'using job token'].each do |context|
+      context context do
+        include_context context
 
         it_behaves_like 'rejected container repository access', :developer, :forbidden
         it_behaves_like 'rejected container repository access', :anonymous, :not_found
@@ -71,11 +137,19 @@ RSpec.describe API::ProjectContainerRepositories do
           end
         end
       end
+    end
+
+    include_examples 'rejected job token scopes'
+  end
 
   describe 'GET /projects/:id/registry/repositories/:repository_id/tags' do
-    subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user) }
+    let(:url) { "/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags" }
+
+    ['using API user', 'using job token'].each do |context|
+      context context do
+        include_context context
 
-    it_behaves_like 'rejected container repository access', :guest, :forbidden
+        it_behaves_like 'rejected container repository access', :guest, :forbidden unless context == 'using job token'
         it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
         context 'for reporter' do
@@ -102,9 +176,18 @@ RSpec.describe API::ProjectContainerRepositories do
           end
         end
       end
+    end
+
+    include_examples 'rejected job token scopes'
+  end
 
   describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags' do
-    subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user), params: params }
+    let(:method) { :delete }
+    let(:url) { "/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags" }
+
+    ['using API user', 'using job token'].each do |context|
+      context context do
+        include_context context
 
         context 'disallowed' do
           let(:params) do
@@ -120,8 +203,6 @@ RSpec.describe API::ProjectContainerRepositories do
           let(:api_user) { maintainer }
 
           context 'without required parameters' do
-        let(:params) { }
-
             it 'returns bad request' do
               subject
 
@@ -165,7 +246,6 @@ RSpec.describe API::ProjectContainerRepositories do
 
             it 'schedules cleanup of tags repository' do
               stub_last_activity_update
-          stub_exclusive_lease(lease_key, timeout: 1.hour)
               expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
                 .with(maintainer.id, root_repository.id, worker_params)
 
@@ -209,11 +289,8 @@ RSpec.describe API::ProjectContainerRepositories do
                 container_expiration_policy: false }
             end
 
-        let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" }
-
             it 'schedules cleanup of tags repository' do
               stub_last_activity_update
-          stub_exclusive_lease(lease_key, timeout: 1.hour)
               expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
                 .with(maintainer.id, root_repository.id, worker_params)
 
@@ -225,7 +302,6 @@ RSpec.describe API::ProjectContainerRepositories do
 
           context 'with invalid regex' do
             let(:invalid_regex) { '*v10.' }
-        let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" }
 
             RSpec.shared_examples 'rejecting the invalid regex' do |param_name|
               it 'does not enqueue a job' do
@@ -245,7 +321,6 @@ RSpec.describe API::ProjectContainerRepositories do
 
             before do
               stub_last_activity_update
-          stub_exclusive_lease(lease_key, timeout: 1.hour)
             end
 
             %i[name_regex_delete name_regex name_regex_keep].each do |param_name|
@@ -258,11 +333,19 @@ RSpec.describe API::ProjectContainerRepositories do
           end
         end
       end
+    end
+
+    include_examples 'rejected job token scopes'
+  end
 
   describe 'GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do
-    subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) }
+    let(:url) { "/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA" }
+
+    ['using API user', 'using job token'].each do |context|
+      context context do
+        include_context context
 
-    it_behaves_like 'rejected container repository access', :guest, :forbidden
+        it_behaves_like 'rejected container repository access', :guest, :forbidden unless context == 'using job token'
         it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
         context 'for reporter' do
@@ -290,11 +373,19 @@ RSpec.describe API::ProjectContainerRepositories do
           end
         end
       end
+    end
+
+    include_examples 'rejected job token scopes'
+  end
 
   describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do
+    let(:method) { :delete }
+    let(:url) { "/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA" }
     let(:service) { double('service') }
 
-    subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) }
+    ['using API user', 'using job token'].each do |context|
+      context context do
+        include_context context
 
         it_behaves_like 'rejected container repository access', :reporter, :forbidden
         it_behaves_like 'rejected container repository access', :anonymous, :not_found
@@ -335,4 +426,8 @@ RSpec.describe API::ProjectContainerRepositories do
           end
         end
       end
+    end
+
+    include_examples 'rejected job token scopes'
+  end
 end