Skip to content

Allow DeployKey to `:download_code`

Background

Following up from gitlab-com/gl-infra/delivery#1848 (comment 613791553)

DeployKey is not a PolicyActor, therefore when authenticating, it might be raising a NoMethodError because it did not respond to from_ci_job_token?. When the error can be happening is explained at: gitlab-com/gl-infra/delivery#1848 (comment 613707783)

Failing test to show the error

Here's a failing test provided by @stanhu with a bit of modification:

diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 27df461af1c..5036a353e54 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -795,6 +795,29 @@ def set_access_level(access_level)
     end
   end
 
+  context 'deploy key access' do
+    context 'private project' do
+      let(:project) { private_project }
+      let(:deploy_key) { create(:deploy_key, user: owner) }
+
+      subject { described_class.new(deploy_key, project) }
+
+      context 'when the deploy key is enabled in the project' do
+        let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) }
+
+        it 'allows download access' do
+          expect(subject).to be_allowed(:download_code)
+        end
+      end
+
+      context 'when the deploy key is not enabled in the project' do
+        it 'does not allow access when a user has read access to the repo' do
+          expect(subject).not_to be_allowed(:download_code)
+        end
+      end
+    end
+  end
+
   context 'deploy token access' do
     let!(:project_deploy_token) do
       create(:project_deploy_token, project: project, deploy_token: deploy_token)

Potential solution

If we include PolicyActor for Key or DeployKey then it'll no longer raise the error, but it still cannot pass the tests because it's not allowed to :download_code. We'll also need to edit ProjectPolicy in order to pass the tests.

There's a number of user.is_a?(DeployToken) in ProjectPolicy therefore a straightforward fix might be adding that kind of check elsewhere to allow :download_code, but I think we might need to have a better overall design over this. For example: #216952

Edited by Lin Jen-Shin