Merge branch 'deploy-tokens-container-registry-specs' into 'master'

Verify that deploy token has valid access when pulling container registry image

Closes #45148

See merge request gitlab-org/gitlab-ce!18260

app/models/deploy_token.rb

@@ -34,7 +34,7 @@ class DeployToken < ActiveRecord::Base
   end
 
   def has_access_to?(requested_project)
-    project == requested_project
+    active? && project == requested_project
   end
 
   # This is temporal. Currently we limit DeployToken

app/services/auth/container_registry_authentication_service.rb

@@ -149,7 +149,8 @@ module Auth
     def deploy_token_can_pull?(requested_project)
       has_authentication_ability?(:read_container_image) &&
         current_user.is_a?(DeployToken) &&
-        current_user.has_access_to?(requested_project)
+        current_user.has_access_to?(requested_project) &&
+        current_user.read_registry?
     end
 
     ##

changelogs/unreleased/deploy-tokens-container-registry-specs.yml

+---
+title: Verify that deploy token has valid access when pulling container registry image
+merge_request: 18260
+author:
+type: fixed

spec/models/deploy_token_spec.rb

@@ -78,19 +78,30 @@ describe DeployToken do
   describe '#has_access_to?' do
     let(:project) { create(:project) }
 
-    subject(:deploy_token) { create(:deploy_token, projects: [project]) }
+    subject { deploy_token.has_access_to?(project) }
 
-    context 'when the deploy token has access to the project' do
-      it 'should return true' do
-        expect(deploy_token.has_access_to?(project)).to be_truthy
-      end
+    context 'when deploy token is active and related to project' do
+      let(:deploy_token) { create(:deploy_token, projects: [project]) }
+
+      it { is_expected.to be_truthy }
     end
 
-    context 'when the deploy token does not have access to the project' do
-      it 'should return false' do
-        another_project = create(:project)
-        expect(deploy_token.has_access_to?(another_project)).to be_falsy
-      end
+    context 'when deploy token is active but not related to project' do
+      let(:deploy_token) { create(:deploy_token) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when deploy token is revoked and related to project' do
+      let(:deploy_token) { create(:deploy_token, :revoked, projects: [project]) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when deploy token is revoked and not related to the project' do
+      let(:deploy_token) { create(:deploy_token, :revoked) }
+
+      it { is_expected.to be_falsy }
     end
   end
 

spec/services/auth/container_registry_authentication_service_spec.rb

@@ -585,4 +585,140 @@ describe Auth::ContainerRegistryAuthenticationService do
       it_behaves_like 'not a container repository factory'
     end
   end
+
+  context 'for deploy tokens' do
+    let(:current_params) do
+      { scope: "repository:#{project.full_path}:pull" }
+    end
+
+    context 'when deploy token has read_registry as a scope' do
+      let(:current_user) { create(:deploy_token, projects: [project]) }
+
+      context 'for public project' do
+        let(:project) { create(:project, :public) }
+
+        context 'when pulling' do
+          it_behaves_like 'a pullable'
+        end
+
+        context 'when pushing' do
+          let(:current_params) do
+            { scope: "repository:#{project.full_path}:push" }
+          end
+
+          it_behaves_like 'an inaccessible'
+        end
+      end
+
+      context 'for internal project' do
+        let(:project) { create(:project, :internal) }
+
+        context 'when pulling' do
+          it_behaves_like 'a pullable'
+        end
+
+        context 'when pushing' do
+          let(:current_params) do
+            { scope: "repository:#{project.full_path}:push" }
+          end
+
+          it_behaves_like 'an inaccessible'
+        end
+      end
+
+      context 'for private project' do
+        let(:project) { create(:project, :private) }
+
+        context 'when pulling' do
+          it_behaves_like 'a pullable'
+        end
+
+        context 'when pushing' do
+          let(:current_params) do
+            { scope: "repository:#{project.full_path}:push" }
+          end
+
+          it_behaves_like 'an inaccessible'
+        end
+      end
+    end
+
+    context 'when deploy token does not have read_registry scope' do
+      let(:current_user) { create(:deploy_token, projects: [project], read_registry: false) }
+
+      context 'for public project' do
+        let(:project) { create(:project, :public) }
+
+        context 'when pulling' do
+          it_behaves_like 'a pullable'
+        end
+      end
+
+      context 'for internal project' do
+        let(:project) { create(:project, :internal) }
+
+        context 'when pulling' do
+          it_behaves_like 'an inaccessible'
+        end
+      end
+
+      context 'for private project' do
+        let(:project) { create(:project, :internal) }
+
+        context 'when pulling' do
+          it_behaves_like 'an inaccessible'
+        end
+      end
+    end
+
+    context 'when deploy token is not related to the project' do
+      let(:current_user) { create(:deploy_token, read_registry: false) }
+
+      context 'for public project' do
+        let(:project) { create(:project, :public) }
+
+        context 'when pulling' do
+          it_behaves_like 'a pullable'
+        end
+      end
+
+      context 'for internal project' do
+        let(:project) { create(:project, :internal) }
+
+        context 'when pulling' do
+          it_behaves_like 'an inaccessible'
+        end
+      end
+
+      context 'for private project' do
+        let(:project) { create(:project, :internal) }
+
+        context 'when pulling' do
+          it_behaves_like 'an inaccessible'
+        end
+      end
+    end
+
+    context 'when deploy token has been revoked' do
+      let(:current_user) { create(:deploy_token, :revoked, projects: [project]) }
+
+      context 'for public project' do
+        let(:project) { create(:project, :public) }
+
+        it_behaves_like 'a pullable'
+      end
+
+      context 'for internal project' do
+        let(:project) { create(:project, :internal) }
+
+        it_behaves_like 'an inaccessible'
+      end
+
+      context 'for private project' do
+        let(:project) { create(:project, :internal) }
+
+        it_behaves_like 'an inaccessible'
+      end
+    end
+  end
 end