Increase test suite around deploy tokens behavior

Also, fixes broken specs

10 files changed, 119 insertions, 193 deletions

diff --git a/spec/controllers/projects/deploy_tokens_controller_spec.rb b/spec/controllers/projects/deploy_tokens_controller_spec.rb
deleted file mode 100644
index 8de564a56af..00000000000
--- a/spec/controllers/projects/deploy_tokens_controller_spec.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-require 'spec_helper'
-
-describe Projects::DeployTokensController do
-  let(:project) { create(:project) }
-  let(:user) { create(:user) }
-  let!(:member) { project.add_master(user) }
-
-  before do
-    sign_in(user)
-  end
-
-  describe 'POST #create' do
-    let(:deploy_token_params) { attributes_for(:deploy_token) }
-
-    subject do
-      post :create,
-        namespace_id: project.namespace,
-        project_id: project,
-        deploy_token: deploy_token_params
-    end
-
-    context 'with valid params' do
-      it 'should create a new DeployToken' do
-        expect { subject }.to change(DeployToken, :count).by(1)
-      end
-
-      it 'should include a flash notice' do
-        subject
-        expect(flash[:notice]).to eq('Your new project deploy token has been created.')
-      end
-
-      it 'should redirect to project settings repository' do
-        subject
-        expect(response).to redirect_to project_settings_repository_path(project)
-      end
-    end
-
-    context 'with invalid params' do
-      let(:deploy_token_params) { attributes_for(:deploy_token, scopes: []) }
-
-      it 'should not create a new DeployToken' do
-        expect { subject }.not_to change(DeployToken, :count)
-      end
-
-      it 'should redirect to project settings repository' do
-        subject
-        expect(response).to redirect_to project_settings_repository_path(project)
-      end
-    end
-
-    context 'when user does not have enough permissions' do
-      let!(:member) { project.add_developer(user) }
-
-      it 'responds with status 404' do
-        subject
-
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
-  end
-end
diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb
index da4d3e5732d..3a4014b7768 100644
--- a/spec/controllers/projects/settings/repository_controller_spec.rb
+++ b/spec/controllers/projects/settings/repository_controller_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::Settings::RepositoryController, :clean_gitlab_redis_shared_state do
+describe Projects::Settings::RepositoryController do
   let(:project) { create(:project_empty_repo, :public) }
   let(:user) { create(:user) }
 
@@ -16,43 +16,5 @@ describe Projects::Settings::RepositoryController, :clean_gitlab_redis_shared_st
       expect(response).to have_gitlab_http_status(200)
       expect(response).to render_template(:show)
     end
-
-    context 'with no deploy token attributes present' do
-      it 'should build an empty instance of DeployToken' do
-        get :show, namespace_id: project.namespace, project_id: project
-
-        deploy_token = assigns(:deploy_token)
-        expect(deploy_token).to be_an_instance_of(DeployToken)
-        expect(deploy_token.name).to be_nil
-        expect(deploy_token.expires_at).to be_nil
-        expect(deploy_token.scopes).to eq([])
-      end
-    end
-
-    context 'with deploy token attributes present' do
-      let(:deploy_token_key) { "gitlab:deploy_token:#{project.id}:#{user.id}:attributes" }
-
-      let(:deploy_token_attributes) do
-        {
-          name: 'test-token',
-          expires_at: Date.today + 1.month
-        }
-      end
-
-      before do
-        Gitlab::Redis::SharedState.with do |redis|
-          redis.set(deploy_token_key, deploy_token_attributes.to_json)
-        end
-
-        get :show, namespace_id: project.namespace, project_id: project
-      end
-
-      it 'should build an instance of DeployToken' do
-        deploy_token = assigns(:deploy_token)
-        expect(deploy_token).to be_an_instance_of(DeployToken)
-        expect(deploy_token.name).to eq(deploy_token_attributes[:name])
-        expect(deploy_token.expires_at.to_date).to eq(deploy_token_attributes[:expires_at].to_date)
-      end
-    end
   end
 end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 7be888d812f..e3ec707076a 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -195,7 +195,7 @@ describe Gitlab::Auth do
           personal_access_token = create(:personal_access_token, scopes: ['read_registry'])
 
           expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
-          expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, [:read_project, :build_download_code, :build_read_container_image]))
+          expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, [:build_read_container_image]))
         end
       end
 
@@ -262,25 +262,38 @@ describe Gitlab::Auth do
 
       context 'when the deploy token has read_repository as scope' do
         let(:deploy_token) { create(:deploy_token, read_registry: false, projects: [project]) }
+        let(:login) { deploy_token.username }
 
-        it 'succeeds when project is present, token is valid and has read_repository as scope' do
-          abilities = %i(download_code)
-          auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities)
+        it 'succeeds when login and token are valid' do
+          auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:download_code])
 
-          expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
-          expect(gl_auth.find_for_git_client('', deploy_token.token, project: project, ip: 'ip'))
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: login)
+          expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip'))
             .to eq(auth_success)
         end
 
+        it 'fails when login is not valid' do
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'random_login')
+          expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+
+        it 'fails when token is not valid' do
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+          expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+
         it 'fails if token is nil' do
-          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-          expect(gl_auth.find_for_git_client('', nil, project: project, ip: 'ip'))
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+          expect(gl_auth.find_for_git_client(login, nil, project: project, ip: 'ip'))
             .to eq(auth_failure)
         end
 
         it 'fails if token is not related to project' do
-          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-          expect(gl_auth.find_for_git_client('', 'abcdef', project: project, ip: 'ip'))
+          another_deploy_token = create(:deploy_token)
+          expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+          expect(gl_auth.find_for_git_client(login, another_deploy_token.token, project: project, ip: 'ip'))
             .to eq(auth_failure)
         end
 
@@ -296,30 +309,42 @@ describe Gitlab::Auth do
 
       context 'when the deploy token has read_registry as a scope' do
         let(:deploy_token) { create(:deploy_token, read_repository: false, projects: [project]) }
+        let(:login) { deploy_token.username }
 
         context 'when registry enabled' do
           before do
             stub_container_registry_config(enabled: true)
           end
 
-          it 'succeeds if deploy token does have read_registry as scope' do
-            abilities = %i(read_project build_download_code build_read_container_image)
-            auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, abilities)
+          it 'succeeds when login and token are valid' do
+            auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:build_read_container_image])
 
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
-            expect(gl_auth.find_for_git_client('', deploy_token.token, project: nil, ip: 'ip'))
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: login)
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
               .to eq(auth_success)
           end
 
+          it 'fails when login is not valid' do
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'random_login')
+            expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip'))
+              .to eq(auth_failure)
+          end
+
+          it 'fails when token is not valid' do
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+            expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip'))
+              .to eq(auth_failure)
+          end
+
           it 'fails if token is nil' do
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-            expect(gl_auth.find_for_git_client('', nil, project: nil, ip: 'ip'))
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+            expect(gl_auth.find_for_git_client(login, nil, project: nil, ip: 'ip'))
               .to eq(auth_failure)
           end
 
           it 'fails if token is not related to project' do
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-            expect(gl_auth.find_for_git_client('', 'abcdef', project: nil, ip: 'ip'))
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+            expect(gl_auth.find_for_git_client(login, 'abcdef', project: nil, ip: 'ip'))
               .to eq(auth_failure)
           end
 
@@ -338,30 +363,9 @@ describe Gitlab::Auth do
             stub_container_registry_config(enabled: false)
           end
 
-          it 'fails if deploy token have read_registry as scope' do
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-            expect(gl_auth.find_for_git_client('', deploy_token.token, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
-
-          it 'fails if token is nil' do
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-            expect(gl_auth.find_for_git_client('', nil, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
-
-          it 'fails if token is not related to project' do
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-            expect(gl_auth.find_for_git_client('', 'abcdef', project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
-
-          it 'fails if token has been revoked' do
-            deploy_token.revoke!
-
-            expect(deploy_token.revoked?).to be_truthy
-            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'deploy-token')
-            expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: nil, ip: 'ip'))
+          it 'fails when login and token are valid' do
+            expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
               .to eq(auth_failure)
           end
         end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 000e9e86813..6c625596605 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -147,21 +147,29 @@ describe Gitlab::GitAccess do
         end
 
         context 'when actor is DeployToken' do
-          let(:project_deploy_token) { create(:project_deploy_token, project: project) }
-          let(:actor) { project_deploy_token.deploy_token }
+          let(:actor) { create(:deploy_token, projects: [project]) }
 
           context 'when DeployToken is active and belongs to project' do
             it 'allows pull access' do
               expect { pull_access_check }.not_to raise_error
             end
+
+            it 'blocks the push' do
+              expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
+            end
           end
 
           context 'when DeployToken does not belong to project' do
-            let(:actor) { create(:deploy_token) }
+            let(:another_project) { create(:project) }
+            let(:actor) { create(:deploy_token, projects: [another_project]) }
 
             it 'blocks pull access' do
               expect { pull_access_check }.to raise_not_found
             end
+
+            it 'blocks the push' do
+              expect { push_access_check }.to raise_not_found
+            end
           end
         end
       end
@@ -613,6 +621,41 @@ describe Gitlab::GitAccess do
       end
     end
 
+    describe 'deploy token permissions' do
+      let(:deploy_token) { create(:deploy_token) }
+      let(:actor) { deploy_token }
+
+      context 'pull code' do
+        context 'when project is authorized' do
+          before do
+            deploy_token.projects << project
+          end
+
+          it { expect { pull_access_check }.not_to raise_error }
+        end
+
+        context 'when unauthorized' do
+          context 'from public project' do
+            let(:project) { create(:project, :public, :repository) }
+
+            it { expect { pull_access_check }.not_to raise_error }
+          end
+
+          context 'from internal project' do
+            let(:project) { create(:project, :internal, :repository) }
+
+            it { expect { pull_access_check }.to raise_not_found }
+          end
+
+          context 'from private project' do
+            let(:project) { create(:project, :private, :repository) }
+
+            it { expect { pull_access_check }.to raise_not_found }
+          end
+        end
+      end
+    end
+
     describe 'build authentication_abilities permissions' do
       let(:authentication_abilities) { build_authentication_abilities }
 
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index d38e665436f..897a5984782 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -145,6 +145,9 @@ pipeline_schedule:
 - pipelines
 pipeline_schedule_variables:
 - pipeline_schedule
+deploy_tokens:
+- project_deploy_tokens
+- projects
 deploy_keys:
 - user
 - deploy_keys_projects
@@ -281,6 +284,7 @@ project:
 - project_badges
 - source_of_merge_requests
 - internal_ids
+- project_deploy_tokens
 - deploy_tokens
 award_emoji:
 - awardable
diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb
index 395c97f13a5..1adc049ca58 100644
--- a/spec/models/deploy_token_spec.rb
+++ b/spec/models/deploy_token_spec.rb
@@ -70,8 +70,27 @@ describe DeployToken do
   end
 
   describe '#username' do
-    it 'returns Ghost username' do
+    it 'returns a harcoded username' do
       expect(deploy_token.username).to eq("gitlab+deploy-token-#{deploy_token.id}")
     end
   end
+
+  describe '#has_access_to?' do
+    let(:project) { create(:project) }
+
+    subject(:deploy_token) { create(:deploy_token, projects: [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
+    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
+    end
+  end
 end
diff --git a/spec/policies/deploy_token_policy_spec.rb b/spec/policies/deploy_token_policy_spec.rb
index f6d8d19aac9..eea287d895e 100644
--- a/spec/policies/deploy_token_policy_spec.rb
+++ b/spec/policies/deploy_token_policy_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe DeployTokenPolicy do
   let(:current_user) { create(:user) }
   let(:project) { create(:project) }
-  let(:deploy_token) { create(:deploy_token, project: project) }
+  let(:deploy_token) { create(:deploy_token, projects: [project]) }
 
   subject { described_class.new(current_user, deploy_token) }
 
diff --git a/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb b/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb
index f52bd46074d..a416acffe94 100644
--- a/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb
+++ b/spec/presenters/projects/settings/deploy_tokens_presenter_spec.rb
@@ -13,20 +13,4 @@ describe Projects::Settings::DeployTokensPresenter do
       expect(presenter.length).to eq(3)
     end
   end
-
-  describe '#temporal_token' do
-    context 'when a deploy token has been created recently' do
-      it 'returns the token of the deploy' do
-        deploy_token = ::DeployTokens::CreateService.new(project, user, attributes_for(:deploy_token)).execute
-
-        expect(presenter.temporal_token).to eq(deploy_token.token)
-      end
-    end
-
-    context 'when a deploy token has not been created recently' do
-      it 'does returns nil' do
-        expect(presenter.temporal_token).to be_nil
-      end
-    end
-  end
 end
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 0949ec24c50..290eeae828e 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -558,7 +558,6 @@ describe Auth::ContainerRegistryAuthenticationService do
       let(:project) { create(:project, :public) }
 
       context 'when pulling and pushing' do
-        let(:current_user) { create(:deploy_token, projects: [project]) }
         let(:current_params) do
           { scope: "repository:#{project.full_path}:pull,push" }
         end
diff --git a/spec/services/deploy_tokens/create_service_spec.rb b/spec/services/deploy_tokens/create_service_spec.rb
index 4830f17faa8..2a308a9bf8c 100644
--- a/spec/services/deploy_tokens/create_service_spec.rb
+++ b/spec/services/deploy_tokens/create_service_spec.rb
@@ -20,20 +20,6 @@ describe DeployTokens::CreateService, :clean_gitlab_redis_shared_state do
       it 'returns a DeployToken' do
         expect(subject).to be_an_instance_of DeployToken
       end
-
-      it 'should store the token on redis' do
-        redis_key = DeployToken.redis_shared_state_key(user.id)
-        subject
-
-        expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).not_to be_nil
-      end
-
-      it 'should not store deploy token attributes on redis' do
-        redis_key = DeployToken.redis_shared_state_key(user.id) + ":attributes"
-        subject
-
-        expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).to be_nil
-      end
     end
 
     context 'when the deploy token is invalid' do
@@ -46,20 +32,6 @@ describe DeployTokens::CreateService, :clean_gitlab_redis_shared_state do
       it 'should not create a new ProjectDeployToken' do
         expect { subject }.not_to change { ProjectDeployToken.count }
       end
-
-      it 'should not store the token on redis' do
-        redis_key = DeployToken.redis_shared_state_key(user.id)
-        subject
-
-        expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).to be_nil
-      end
-
-      it 'should store deploy token attributes on redis' do
-        redis_key = DeployToken.redis_shared_state_key(user.id) + ":attributes"
-        subject
-
-        expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).not_to be_nil
-      end
     end
   end
 end