Automatic merge of gitlab-org/gitlab-ce master

changelogs/unreleased/25942-remove-fake-repository-path-response.yml

+---
+title: Remove fake repository_path response
+merge_request: 25942
+author: Fabio Papa
+type: other

lib/api/helpers.rb

@@ -9,6 +9,7 @@ module API
     include Helpers::Pagination
 
     SUDO_HEADER = "HTTP_SUDO".freeze
+    GITLAB_SHARED_SECRET_HEADER = "Gitlab-Shared-Secret".freeze
     SUDO_PARAM = :sudo
     API_USER_ENV = 'gitlab.api.user'.freeze
 
@@ -225,10 +226,12 @@ module API
     end
 
     def authenticate_by_gitlab_shell_token!
-      input = params['secret_token'].try(:chomp)
-      unless Devise.secure_compare(secret_token, input)
-        unauthorized!
-      end
+      input = params['secret_token']
+      input ||= Base64.decode64(headers[GITLAB_SHARED_SECRET_HEADER]) if headers.key?(GITLAB_SHARED_SECRET_HEADER)
+
+      input&.chomp!
+
+      unauthorized! unless Devise.secure_compare(secret_token, input)
     end
 
     def authenticated_with_full_private_access!

lib/api/internal.rb

@@ -81,11 +81,6 @@ module API
             gl_id: Gitlab::GlId.gl_id(user),
             gl_username: user&.username,
             git_config_options: [],
-
-            # This repository_path is a bogus value but gitlab-shell still requires
-            # its presence. https://gitlab.com/gitlab-org/gitlab-shell/issues/135
-            repository_path: '/',
-
             gitaly: gitaly_payload(params[:action])
           }
 

spec/migrations/add_foreign_keys_to_todos_spec.rb

@@ -36,7 +36,7 @@ describe AddForeignKeysToTodos, :migration do
   end
 
   context 'add foreign key on note_id' do
-    let(:note) { create(:note) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
+    let(:note) { table(:notes).create! }
     let!(:todo_with_note) { create_todo(note_id: note.id) }
     let!(:todo_with_invalid_note) { create_todo(note_id: 4711) }
     let!(:todo_without_note) { create_todo(note_id: nil) }

spec/migrations/calculate_conv_dev_index_percentages_spec.rb

@@ -3,12 +3,30 @@
 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20170803090603_calculate_conv_dev_index_percentages.rb')
 
-describe CalculateConvDevIndexPercentages, :delete do
+describe CalculateConvDevIndexPercentages, :migration do
   let(:migration) { described_class.new }
   let!(:conv_dev_index) do
-    create(:conversational_development_index_metric, # rubocop:disable RSpec/FactoriesInMigrationSpecs
+    table(:conversational_development_index_metrics).create!(
+      leader_issues: 9.256,
       leader_notes: 0,
+      leader_milestones: 16.2456,
+      leader_boards: 5.2123,
+      leader_merge_requests: 1.2,
+      leader_ci_pipelines: 12.1234,
+      leader_environments: 3.3333,
+      leader_deployments: 1.200,
+      leader_projects_prometheus_active: 0.111,
+      leader_service_desk_issues: 15.891,
+      instance_issues: 1.234,
+      instance_notes: 28.123,
       instance_milestones: 0,
+      instance_boards: 3.254,
+      instance_merge_requests: 0.6,
+      instance_ci_pipelines: 2.344,
+      instance_environments: 2.2222,
+      instance_deployments: 0.771,
+      instance_projects_prometheus_active: 0.109,
+      instance_service_desk_issues: 13.345,
       percentage_issues: 0,
       percentage_notes: 0,
       percentage_milestones: 0,

spec/migrations/cleanup_nonexisting_namespace_pending_delete_projects_spec.rb

 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20170816102555_cleanup_nonexisting_namespace_pending_delete_projects.rb')
 
-describe CleanupNonexistingNamespacePendingDeleteProjects do
-  before do
-    # Stub after_save callbacks that will fail when Project has invalid namespace
-    allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil)
-    allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil)
-  end
+describe CleanupNonexistingNamespacePendingDeleteProjects, :migration do
+  let(:projects) { table(:projects) }
+  let(:namespaces) { table(:namespaces) }
 
   describe '#up' do
-    set(:some_project) { create(:project) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
+    let!(:some_project) { projects.create! }
+    let(:namespace) { namespaces.create!(name: 'test', path: 'test') }
 
     it 'only cleans up when namespace does not exist' do
-      create(:project, pending_delete: true) # rubocop:disable RSpec/FactoriesInMigrationSpecs
-      project = build(:project, pending_delete: true, namespace: nil, namespace_id: Namespace.maximum(:id).to_i.succ) # rubocop:disable RSpec/FactoriesInMigrationSpecs
-      project.save(validate: false)
+      projects.create!(pending_delete: true, namespace_id: namespace.id)
+      project = projects.create!(pending_delete: true, namespace_id: 0)
 
       expect(NamespacelessProjectDestroyWorker).to receive(:bulk_perform_async).with([[project.id]])
 
@@ -22,7 +19,7 @@ describe CleanupNonexistingNamespacePendingDeleteProjects do
     end
 
     it 'does nothing when no pending delete projects without namespace found' do
-      create(:project, pending_delete: true, namespace: create(:namespace)) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+      projects.create!(pending_delete: true, namespace_id: namespace.id)
 
       expect(NamespacelessProjectDestroyWorker).not_to receive(:bulk_perform_async)
 

spec/migrations/issues_moved_to_id_foreign_key_spec.rb

 require 'spec_helper'
 require Rails.root.join('db', 'migrate', '20171106151218_issues_moved_to_id_foreign_key.rb')
 
-# The schema version has to be far enough in advance to have the
-# only_mirror_protected_branches column in the projects table to create a
-# project via FactoryBot.
-describe IssuesMovedToIdForeignKey, :migration, schema: 20171114150259 do
-  let!(:issue_first) { create(:issue, moved_to_id: issue_second.id) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
-  let!(:issue_second) { create(:issue, moved_to_id: issue_third.id) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
-  let!(:issue_third) { create(:issue) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
+describe IssuesMovedToIdForeignKey, :migration do
+  let(:issues) { table(:issues) }
+
+  let!(:issue_third) { issues.create! }
+  let!(:issue_second) { issues.create!(moved_to_id: issue_third.id) }
+  let!(:issue_first) { issues.create!(moved_to_id: issue_second.id) }
 
   subject { described_class.new }
 
   it 'removes the orphaned moved_to_id' do
     subject.down
 
-    issue_third.update(moved_to_id: 100000)
+    issue_third.update!(moved_to_id: 0)
 
     subject.up
 

spec/migrations/move_personal_snippets_files_spec.rb

 require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20170612071012_move_personal_snippets_files.rb')
 
-describe MovePersonalSnippetsFiles do
+describe MovePersonalSnippetsFiles, :migration do
   let(:migration) { described_class.new }
   let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_snippet_files_test") }
   let(:uploads_dir) { File.join(test_dir, 'uploads') }
   let(:new_uploads_dir) { File.join(uploads_dir, '-', 'system') }
 
+  let(:notes) { table(:notes) }
+  let(:snippets) { table(:snippets) }
+  let(:uploads) { table(:uploads) }
+
+  let(:user) { table(:users).create!(email: 'user@example.com', projects_limit: 10) }
+  let(:project) { table(:projects).create!(name: 'gitlab', namespace_id: 1) }
+
   before do
     allow(CarrierWave).to receive(:root).and_return(test_dir)
     allow(migration).to receive(:base_directory).and_return(test_dir)
@@ -16,14 +23,14 @@ describe MovePersonalSnippetsFiles do
 
   describe "#up" do
     let(:snippet) do
-      snippet = create(:personal_snippet) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+      snippet = snippets.create!(author_id: user.id)
       create_upload('picture.jpg', snippet)
       snippet.update(description: markdown_linking_file('picture.jpg', snippet))
       snippet
     end
 
     let(:snippet_with_missing_file) do
-      snippet = create(:snippet) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+      snippet = snippets.create!(author_id: user.id, project_id: project.id)
       create_upload('picture.jpg', snippet, create_file: false)
       snippet.update(description: markdown_linking_file('picture.jpg', snippet))
       snippet
@@ -62,7 +69,10 @@ describe MovePersonalSnippetsFiles do
         secret = "secret#{snippet.id}"
         file_location = "/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg"
         markdown = markdown_linking_file('picture.jpg', snippet)
-        note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown}") # rubocop:disable RSpec/FactoriesInMigrationSpecs
+        note = notes.create!(noteable_id: snippet.id,
+                             noteable_type: Snippet,
+                             note: "with #{markdown}",
+                             author_id: user.id)
 
         migration.up
 
@@ -73,14 +83,14 @@ describe MovePersonalSnippetsFiles do
 
   describe "#down" do
     let(:snippet) do
-      snippet = create(:personal_snippet) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+      snippet = snippets.create!(author_id: user.id)
       create_upload('picture.jpg', snippet, in_new_path: true)
       snippet.update(description: markdown_linking_file('picture.jpg', snippet, in_new_path: true))
       snippet
     end
 
     let(:snippet_with_missing_file) do
-      snippet = create(:personal_snippet) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+      snippet = snippets.create!(author_id: user.id)
       create_upload('picture.jpg', snippet, create_file: false, in_new_path: true)
       snippet.update(description: markdown_linking_file('picture.jpg', snippet, in_new_path: true))
       snippet
@@ -119,7 +129,10 @@ describe MovePersonalSnippetsFiles do
         markdown = markdown_linking_file('picture.jpg', snippet, in_new_path: true)
         secret = "secret#{snippet.id}"
         file_location = "/uploads/personal_snippet/#{snippet.id}/#{secret}/picture.jpg"
-        note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown}") # rubocop:disable RSpec/FactoriesInMigrationSpecs
+        note = notes.create!(noteable_id: snippet.id,
+                             noteable_type: Snippet,
+                             note: "with #{markdown}",
+                             author_id: user.id)
 
         migration.down
 
@@ -135,7 +148,7 @@ describe MovePersonalSnippetsFiles do
 
       secret = '123456789'
       filename = 'hello.jpg'
-      snippet = create(:personal_snippet) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+      snippet = snippets.create!(author_id: user.id)
 
       path_before = "/uploads/personal_snippet/#{snippet.id}/#{secret}/#{filename}"
       path_after = "/uploads/system/personal_snippet/#{snippet.id}/#{secret}/#{filename}"
@@ -161,7 +174,11 @@ describe MovePersonalSnippetsFiles do
       FileUtils.touch(absolute_path)
     end
 
-    create(:upload, model: snippet, path: "#{secret}/#{filename}", uploader: PersonalFileUploader) # rubocop:disable RSpec/FactoriesInMigrationSpecs
+    uploads.create!(model_id: snippet.id,
+                    model_type: snippet.class,
+                    path: "#{secret}/#{filename}",
+                    uploader: PersonalFileUploader,
+                    size: 100.kilobytes)
   end
 
   def markdown_linking_file(filename, snippet, in_new_path: false)

spec/requests/api/internal_spec.rb

@@ -26,6 +26,21 @@ describe API::Internal do
 
       expect(json_response['redis']).to be(false)
     end
+
+    context 'authenticating' do
+      it 'authenticates using a header' do
+        get api("/internal/check"),
+            headers: { API::Helpers::GITLAB_SHARED_SECRET_HEADER => Base64.encode64(secret_token) }
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      it 'returns 401 when no credentials provided' do
+        get(api("/internal/check"))
+
+        expect(response).to have_gitlab_http_status(401)
+      end
+    end
   end
 
   describe 'GET /internal/broadcast_message' do
@@ -332,7 +347,6 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_truthy
-          expect(json_response["repository_path"]).to eq('/')
           expect(json_response["gl_project_path"]).to eq(project.wiki.full_path)
           expect(json_response["gl_repository"]).to eq("wiki-#{project.id}")
           expect(user.reload.last_activity_on).to be_nil
@@ -345,7 +359,6 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_truthy
-          expect(json_response["repository_path"]).to eq('/')
           expect(json_response["gl_project_path"]).to eq(project.wiki.full_path)
           expect(json_response["gl_repository"]).to eq("wiki-#{project.id}")
           expect(user.reload.last_activity_on).to eql(Date.today)
@@ -358,7 +371,6 @@ describe API::Internal do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response["status"]).to be_truthy
-          expect(json_response["repository_path"]).to eq('/')
           expect(json_response["gl_repository"]).to eq("project-#{project.id}")
           expect(json_response["gl_project_path"]).to eq(project.full_path)
           expect(json_response["gitaly"]).not_to be_nil
@@ -378,7 +390,6 @@ describe API::Internal do
 
             expect(response).to have_gitlab_http_status(200)
             expect(json_response["status"]).to be_truthy
-            expect(json_response["repository_path"]).to eq('/')
             expect(json_response["gl_repository"]).to eq("project-#{project.id}")
             expect(json_response["gl_project_path"]).to eq(project.full_path)
             expect(json_response["gitaly"]).not_to be_nil