Add latest changes from gitlab-org/gitlab@master

9 files changed, 110 insertions, 81 deletions

diff --git a/spec/factories/deploy_tokens.rb b/spec/factories/deploy_tokens.rb
index e86d4ab8812..657915f9976 100644
--- a/spec/factories/deploy_tokens.rb
+++ b/spec/factories/deploy_tokens.rb
@@ -7,6 +7,7 @@ FactoryBot.define do
     sequence(:name) { |n| "PDT #{n}" }
     read_repository { true }
     read_registry { true }
+    write_registry { false }
     revoked { false }
     expires_at { 5.days.from_now }
     deploy_token_type { DeployToken.deploy_token_types[:project_type] }
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index ce60a19a7b3..a0a8767637e 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -30,7 +30,7 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'optional_scopes contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.optional_scopes).to eq %i[read_user read_api read_repository write_repository read_registry sudo openid profile email]
+      expect(subject.optional_scopes).to eq %i[read_user read_api read_repository write_repository read_registry write_registry sudo openid profile email]
     end
   end
 
@@ -38,21 +38,21 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.all_available_scopes).to eq %i[api read_user read_api read_repository write_repository read_registry sudo]
+      expect(subject.all_available_scopes).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry sudo]
     end
 
     it 'contains for non-admin user all non-default scopes without ADMIN access' do
       stub_container_registry_config(enabled: true)
       user = create(:user, admin: false)
 
-      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry]
+      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry]
     end
 
     it 'contains for admin user all non-default scopes with ADMIN access' do
       stub_container_registry_config(enabled: true)
       user = create(:user, admin: true)
 
-      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry sudo]
+      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry write_registry sudo]
     end
 
     context 'registry_scopes' do
@@ -72,7 +72,7 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         end
 
         it 'contains all registry related scopes' do
-          expect(subject.registry_scopes).to eq %i[read_registry]
+          expect(subject.registry_scopes).to eq %i[read_registry write_registry]
         end
       end
     end
@@ -401,6 +401,49 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     context 'while using deploy tokens' do
       let(:auth_failure) { Gitlab::Auth::Result.new(nil, nil) }
 
+      shared_examples 'registry token scope' do
+        it 'fails when login is not valid' do
+          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.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.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.find_for_git_client(login, '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.find_for_git_client('deploy-token', deploy_token.token, project: nil, ip: 'ip'))
+            .to eq(auth_failure)
+        end
+      end
+
+      shared_examples 'deploy token with disabled registry' do
+        context 'when registry disabled' do
+          before do
+            stub_container_registry_config(enabled: false)
+          end
+
+          it 'fails when login and token are valid' do
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
+              .to eq(auth_failure)
+          end
+        end
+      end
+
       context 'when deploy token and user have the same username' do
         let(:username) { 'normal_user' }
         let(:user) { create(:user, username: username, password: 'my-secret') }
@@ -425,34 +468,33 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         context 'and belong to the same project' do
           let!(:read_registry) { create(:deploy_token, username: 'deployer', read_repository: false, projects: [project]) }
           let!(:read_repository) { create(:deploy_token, username: read_registry.username, read_registry: false, projects: [project]) }
+          let(:auth_success) { Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code]) }
 
           it 'succeeds for the right token' do
-            auth_success = Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code])
-
             expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: project, ip: 'ip'))
               .to eq(auth_success)
           end
 
           it 'fails for the wrong token' do
             expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: project, ip: 'ip'))
-              .to eq(auth_failure)
+              .not_to eq(auth_success)
           end
         end
 
         context 'and belong to different projects' do
+          let_it_be(:other_project) { create(:project) }
           let!(:read_registry) { create(:deploy_token, username: 'deployer', read_repository: false, projects: [project]) }
-          let!(:read_repository) { create(:deploy_token, username: read_registry.username, read_registry: false, projects: [project]) }
+          let!(:read_repository) { create(:deploy_token, username: read_registry.username, read_registry: false, projects: [other_project]) }
+          let(:auth_success) { Gitlab::Auth::Result.new(read_repository, other_project, :deploy_token, [:download_code]) }
 
           it 'succeeds for the right token' do
-            auth_success = Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code])
-
-            expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: project, ip: 'ip'))
+            expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: other_project, ip: 'ip'))
               .to eq(auth_success)
           end
 
           it 'fails for the wrong token' do
-            expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: project, ip: 'ip'))
-              .to eq(auth_failure)
+            expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: other_project, ip: 'ip'))
+              .not_to eq(auth_success)
           end
         end
       end
@@ -542,45 +584,32 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
               .to eq(auth_success)
           end
 
-          it 'fails when login is not valid' do
-            expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+          it_behaves_like 'registry token scope'
+        end
 
-          it 'fails when token is not valid' do
-            expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+        it_behaves_like 'deploy token with disabled registry'
+      end
 
-          it 'fails if token is nil' do
-            expect(gl_auth.find_for_git_client(login, nil, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+      context 'when the deploy token has write_registry as a scope' do
+        let_it_be(:deploy_token) { create(:deploy_token, write_registry: true, read_repository: false, read_registry: false, projects: [project]) }
+        let_it_be(:login) { deploy_token.username }
 
-          it 'fails if token is not related to project' do
-            expect(gl_auth.find_for_git_client(login, 'abcdef', project: nil, ip: 'ip'))
-              .to eq(auth_failure)
+        context 'when registry enabled' do
+          before do
+            stub_container_registry_config(enabled: true)
           end
 
-          it 'fails if token has been revoked' do
-            deploy_token.revoke!
-
-            expect(deploy_token.revoked?).to be_truthy
-            expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
-        end
+          it 'succeeds when login and a project token are valid' do
+            auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:create_container_image])
 
-        context 'when registry disabled' do
-          before do
-            stub_container_registry_config(enabled: false)
+            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip'))
+              .to eq(auth_success)
           end
 
-          it 'fails when login and token are valid' do
-            expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip'))
-              .to eq(auth_failure)
-          end
+          it_behaves_like 'registry token scope'
         end
+
+        it_behaves_like 'deploy token with disabled registry'
       end
     end
   end
diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb
index 568699cf3f6..a2d4c046d46 100644
--- a/spec/models/deploy_token_spec.rb
+++ b/spec/models/deploy_token_spec.rb
@@ -62,7 +62,7 @@ describe DeployToken do
 
     context 'with no scopes' do
       it 'is invalid' do
-        deploy_token = build(:deploy_token, read_repository: false, read_registry: false)
+        deploy_token = build(:deploy_token, read_repository: false, read_registry: false, write_registry: false)
 
         expect(deploy_token).not_to be_valid
         expect(deploy_token.errors[:base].first).to eq("Scopes can't be blank")
@@ -79,7 +79,7 @@ describe DeployToken do
 
     context 'with only one scope' do
       it 'returns scopes assigned to DeployToken' do
-        deploy_token = create(:deploy_token, read_registry: false)
+        deploy_token = create(:deploy_token, read_registry: false, write_registry: false)
         expect(deploy_token.scopes).to eq([:read_repository])
       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 84f4a7a4e7a..8273269c2fb 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -766,8 +766,8 @@ describe Auth::ContainerRegistryAuthenticationService do
       { scopes: ["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 'when deploy token has read and write registry as scopes' do
+      let(:current_user) { create(:deploy_token, write_registry: true, projects: [project]) }
 
       shared_examples 'able to login' do
         context 'registry provides read_container_image authentication_abilities' do
@@ -790,7 +790,7 @@ describe Auth::ContainerRegistryAuthenticationService do
             { scopes: ["repository:#{project.full_path}:push"] }
           end
 
-          it_behaves_like 'an inaccessible'
+          it_behaves_like 'a pushable'
         end
 
         it_behaves_like 'able to login'
@@ -808,7 +808,7 @@ describe Auth::ContainerRegistryAuthenticationService do
             { scopes: ["repository:#{project.full_path}:push"] }
           end
 
-          it_behaves_like 'an inaccessible'
+          it_behaves_like 'a pushable'
         end
 
         it_behaves_like 'able to login'
@@ -826,7 +826,7 @@ describe Auth::ContainerRegistryAuthenticationService do
             { scopes: ["repository:#{project.full_path}:push"] }
           end
 
-          it_behaves_like 'an inaccessible'
+          it_behaves_like 'a pushable'
         end
 
         it_behaves_like 'able to login'
diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb
index 443e3dfddf1..c8354f6ba4e 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/spec/services/projects/fork_service_spec.rb
@@ -311,6 +311,8 @@ describe Projects::ForkService do
       fork_before_move = fork_project(project)
 
       # Stub everything required to move a project to a Gitaly shard that does not exist
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
       stub_storage_settings('test_second_storage' => { 'path' => TestEnv::SECOND_STORAGE_PATH })
       allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_repository)
         .and_return(true)
diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb
index 23ce6f9165d..05555fa76f7 100644
--- a/spec/services/projects/update_repository_storage_service_spec.rb
+++ b/spec/services/projects/update_repository_storage_service_spec.rb
@@ -20,6 +20,8 @@ describe Projects::UpdateRepositoryStorageService do
       let(:project_repository_double) { double(:repository) }
 
       before do
+        allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+        allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
         allow(Gitlab::Git::Repository).to receive(:new).and_call_original
         allow(Gitlab::Git::Repository).to receive(:new)
           .with('test_second_storage', project.repository.raw.relative_path, project.repository.gl_repository, project.repository.full_path)
@@ -49,17 +51,20 @@ describe Projects::UpdateRepositoryStorageService do
         end
       end
 
-      context 'when the project is already on the target storage' do
+      context 'when the filesystems are the same' do
         it 'bails out and does nothing' do
           result = subject.execute(project.repository_storage)
 
           expect(result[:status]).to eq(:error)
-          expect(result[:message]).to match(/repository and source have the same storage/)
+          expect(result[:message]).to match(/SameFilesystemError/)
         end
       end
 
       context 'when the move fails' do
         it 'unmarks the repository as read-only without updating the repository storage' do
+          allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+          allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
+
           expect(project_repository_double).to receive(:create_repository)
             .and_return(true)
           expect(project_repository_double).to receive(:replicate)
@@ -77,6 +82,9 @@ describe Projects::UpdateRepositoryStorageService do
 
       context 'when the checksum does not match' do
         it 'unmarks the repository as read-only without updating the repository storage' do
+          allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+          allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
+
           expect(project_repository_double).to receive(:create_repository)
             .and_return(true)
           expect(project_repository_double).to receive(:replicate)
@@ -97,6 +105,9 @@ describe Projects::UpdateRepositoryStorageService do
         let!(:pool) { create(:pool_repository, :ready, source_project: project) }
 
         it 'leaves the pool' do
+          allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+          allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
+
           expect(project_repository_double).to receive(:create_repository)
             .and_return(true)
           expect(project_repository_double).to receive(:replicate)
diff --git a/spec/support/services/deploy_token_shared_examples.rb b/spec/support/services/deploy_token_shared_examples.rb
index 9d681970739..adc5ea0fcdc 100644
--- a/spec/support/services/deploy_token_shared_examples.rb
+++ b/spec/support/services/deploy_token_shared_examples.rb
@@ -46,7 +46,7 @@ RSpec.shared_examples 'a deploy token creation service' do
     end
 
     context 'when the deploy token is invalid' do
-      let(:deploy_token_params) { attributes_for(:deploy_token, read_repository: false, read_registry: false) }
+      let(:deploy_token_params) { attributes_for(:deploy_token, read_repository: false, read_registry: false, write_registry: false) }
 
       it 'does not create a new DeployToken' do
         expect { subject }.not_to change { DeployToken.count }
diff --git a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb
index b22379b8b68..d6166ac8188 100644
--- a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb
+++ b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb
@@ -22,6 +22,9 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
   context 'when the move succeeds', :clean_gitlab_redis_shared_state do
     before do
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
+
       allow(project_repository_double).to receive(:create_repository)
         .and_return(true)
       allow(project_repository_double).to receive(:replicate)
@@ -83,17 +86,19 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
     end
   end
 
-  context 'when the project is already on the target storage' do
+  context 'when the filesystems are the same' do
     it 'bails out and does nothing' do
       result = subject.execute(project.repository_storage)
 
       expect(result[:status]).to eq(:error)
-      expect(result[:message]).to match(/repository and source have the same storage/)
+      expect(result[:message]).to match(/SameFilesystemError/)
     end
   end
 
   context "when the move of the #{repository_type} repository fails" do
     it 'unmarks the repository as read-only without updating the repository storage' do
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
       allow(project_repository_double).to receive(:create_repository)
         .and_return(true)
       allow(project_repository_double).to receive(:replicate)
@@ -119,6 +124,8 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
   context "when the checksum of the #{repository_type} repository does not match" do
     it 'unmarks the repository as read-only without updating the repository storage' do
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid)
       allow(project_repository_double).to receive(:create_repository)
         .and_return(true)
       allow(project_repository_double).to receive(:replicate)
diff --git a/spec/workers/project_update_repository_storage_worker_spec.rb b/spec/workers/project_update_repository_storage_worker_spec.rb
index ed99b8135c2..57a4c2128b3 100644
--- a/spec/workers/project_update_repository_storage_worker_spec.rb
+++ b/spec/workers/project_update_repository_storage_worker_spec.rb
@@ -9,33 +9,12 @@ describe ProjectUpdateRepositoryStorageWorker do
   subject { described_class.new }
 
   describe "#perform" do
-    context 'when source and target repositories are on different filesystems' do
-      before do
-        allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
-        allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('new_storage').and_return(SecureRandom.uuid)
+    it "calls the update repository storage service" do
+      expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance|
+        expect(instance).to receive(:execute).with('new_storage')
       end
 
-      it "calls the update repository storage service" do
-        expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance|
-          expect(instance).to receive(:execute).with('new_storage')
-        end
-
-        subject.perform(project.id, 'new_storage')
-      end
-    end
-
-    context 'when source and target repositories are on the same filesystems' do
-      let(:filesystem_id) { SecureRandom.uuid }
-
-      before do
-        allow(Gitlab::GitalyClient).to receive(:filesystem_id).and_return(filesystem_id)
-      end
-
-      it 'raises an error' do
-        expect_any_instance_of(::Projects::UpdateRepositoryStorageService).not_to receive(:new)
-
-        expect { subject.perform(project.id, 'new_storage') }.to raise_error(ProjectUpdateRepositoryStorageWorker::SameFilesystemError)
-      end
+      subject.perform(project.id, 'new_storage')
     end
   end
 end