Add latest changes from gitlab-org/gitlab@master

spec/controllers/sent_notifications_controller_spec.rb

@@ -56,7 +56,7 @@ describe SentNotificationsController do
           get(:unsubscribe, params: { id: sent_notification.reply_key })
         end
-        shared_examples 'unsubscribing as anonymous' do
+        shared_examples 'unsubscribing as anonymous' do |project_visibility|
           it 'does not unsubscribe the user' do
             expect(noteable.subscribed?(user, target_project)).to be_truthy
           end
@@ -69,6 +69,18 @@ describe SentNotificationsController do
             expect(response.status).to eq(200)
             expect(response).to render_template :unsubscribe
           end
+
+          if project_visibility == :private
+            it 'does not show project name or path' do
+              expect(response.body).not_to include(noteable.project.name)
+              expect(response.body).not_to include(noteable.project.full_name)
+            end
+          else
+            it 'shows project name or path' do
+              expect(response.body).to include(noteable.project.name)
+              expect(response.body).to include(noteable.project.full_name)
+            end
+          end
         end
         context 'when project is public' do
@@ -79,7 +91,7 @@ describe SentNotificationsController do
               expect(response.body).to include(issue.title)
             end
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :public
           end
           context 'when unsubscribing from confidential issue' do
@@ -90,7 +102,7 @@ describe SentNotificationsController do
               expect(response.body).to include(confidential_issue.to_reference)
             end
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :public
           end
           context 'when unsubscribing from merge request' do
@@ -100,7 +112,12 @@ describe SentNotificationsController do
               expect(response.body).to include(merge_request.title)
             end
-            it_behaves_like 'unsubscribing as anonymous'
+            it 'shows project name or path' do
+              expect(response.body).to include(issue.project.name)
+              expect(response.body).to include(issue.project.full_name)
+            end
+
+            it_behaves_like 'unsubscribing as anonymous', :public
           end
         end
@@ -110,11 +127,11 @@ describe SentNotificationsController do
           context 'when unsubscribing from issue' do
             let(:noteable) { issue }
-            it 'shows issue title' do
+            it 'does not show issue title' do
               expect(response.body).not_to include(issue.title)
             end
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :private
           end
           context 'when unsubscribing from confidential issue' do
@@ -125,17 +142,17 @@ describe SentNotificationsController do
               expect(response.body).to include(confidential_issue.to_reference)
             end
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :private
           end
           context 'when unsubscribing from merge request' do
             let(:noteable) { merge_request }
-            it 'shows merge request title' do
+            it 'dos not show merge request title' do
               expect(response.body).not_to include(merge_request.title)
             end
-            it_behaves_like 'unsubscribing as anonymous'
+            it_behaves_like 'unsubscribing as anonymous', :private
           end
         end
       end

spec/fixtures/api/schemas/evidences/milestone.json

@@ -7,8 +7,7 @@
     "state",
     "iid",
     "created_at",
-    "due_date",
-    "issues"
+    "due_date"
   ],
   "properties": {
     "id": { "type": "integer" },
@@ -17,11 +16,7 @@
     "state": { "type": "string" },
     "iid": { "type": "integer" },
     "created_at": { "type": "date" },
-    "due_date": { "type": ["date", "null"] },
-    "issues": {
-      "type": "array",
-      "items": { "$ref": "issue.json" }
-    }
+    "due_date": { "type": ["date", "null"] }
   },
   "additionalProperties": false
 }

spec/lib/banzai/filter/relative_link_filter_spec.rb

@@ -128,6 +128,15 @@ describe Banzai::Filter::RelativeLinkFilter do
     expect { filter(act) }.not_to raise_error
   end
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences in uploads' do
+    act = link("/uploads/%FF")
+    expect { filter(act) }.not_to raise_error
+  end
+
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences in context requested path' do
+    expect { filter(link("files/test.md"), requested_path: '%FF') }.not_to raise_error
+  end
+
   it 'does not raise an exception with a garbled path' do
     act = link("open(/var/tmp/):%20/location%0Afrom:%20/test")
     expect { filter(act) }.not_to raise_error

spec/models/user_spec.rb

@@ -2638,8 +2638,8 @@ describe User, :do_not_mock_admin_mode do
           add_user(:maintainer)
         end
-        it 'loads' do
-          expect(user.ci_owned_runners).to contain_exactly(runner)
+        it 'does not load' do
+          expect(user.ci_owned_runners).to be_empty
         end
       end
@@ -2654,6 +2654,20 @@ describe User, :do_not_mock_admin_mode do
       end
     end
+    shared_examples :group_member do
+      context 'when the user is owner' do
+        before do
+          add_user(:owner)
+        end
+
+        it 'loads' do
+          expect(user.ci_owned_runners).to contain_exactly(runner)
+        end
+      end
+
+      it_behaves_like :member
+    end
+
     context 'with groups projects runners' do
       let(:group) { create(:group) }
       let!(:project) { create(:project, group: group) }
@@ -2662,7 +2676,7 @@ describe User, :do_not_mock_admin_mode do
         group.add_user(user, access)
       end
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
     context 'with groups runners' do
@@ -2673,14 +2687,14 @@ describe User, :do_not_mock_admin_mode do
         group.add_user(user, access)
       end
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
     context 'with other projects runners' do
       let!(:project) { create(:project) }
       def add_user(access)
-        project.add_role(user, access)
+        project.add_user(user, access)
       end
       it_behaves_like :member
@@ -2698,7 +2712,7 @@ describe User, :do_not_mock_admin_mode do
         subgroup.add_user(another_user, :owner)
       end
-      it_behaves_like :member
+      it_behaves_like :group_member
     end
   end

spec/requests/api/graphql/gitlab_schema_spec.rb

@@ -8,6 +8,18 @@ describe 'GitlabSchema configurations' do
   set(:project) { create(:project) }
   shared_examples 'imposing query limits' do
+    describe 'timeouts' do
+      context 'when timeout is reached' do
+        it 'shows an error' do
+          Timecop.scale(50000000) do # ludicrously large number because the timeout has to happen before the query even begins
+            subject
+
+            expect_graphql_errors_to_include /Timeout/
+          end
+        end
+      end
+    end
+
     describe '#max_complexity' do
       context 'when complexity is too high' do
         it 'shows an error' do

spec/requests/api/releases_spec.rb

@@ -340,6 +340,40 @@ describe API::Releases do
           expect(response).to have_gitlab_http_status(:ok)
         end
+
+        context 'when release is associated to a milestone' do
+          let!(:release) do
+            create(:release, tag: 'v0.1', project: project, milestones: [milestone])
+          end
+
+          let(:milestone) { create(:milestone, project: project) }
+
+          it 'exposes milestones' do
+            get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+            expect(json_response['milestones'].first['title']).to eq(milestone.title)
+          end
+
+          context 'when project restricts visibility of issues and merge requests' do
+            let!(:project) { create(:project, :repository, :public, :issues_private, :merge_requests_private) }
+
+            it 'does not expose milestones' do
+              get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+              expect(json_response['milestones']).to be_nil
+            end
+          end
+
+          context 'when project restricts visibility of issues' do
+            let!(:project) { create(:project, :repository, :public, :issues_private) }
+
+            it 'exposes milestones' do
+              get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+              expect(json_response['milestones'].first['title']).to eq(milestone.title)
+            end
+          end
+        end
       end
     end
   end

spec/requests/api/runners_spec.rb

@@ -6,6 +6,7 @@ describe API::Runners do
   let(:admin) { create(:user, :admin) }
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
+  let(:group_maintainer) { create(:user) }
   let(:project) { create(:project, creator_id: user.id) }
   let(:project2) { create(:project, creator_id: user.id) }
@@ -20,6 +21,7 @@ describe API::Runners do
   before do
     # Set project access for users
+    create(:group_member, :maintainer, user: group_maintainer, group: group)
     create(:project_member, :maintainer, user: user, project: project)
     create(:project_member, :maintainer, user: user, project: project2)
     create(:project_member, :reporter, user: user2, project: project)
@@ -525,6 +527,20 @@ describe API::Runners do
           end.to change { Ci::Runner.project_type.count }.by(-1)
         end
+        it 'does not delete group runner with maintainer access' do
+          delete api("/runners/#{group_runner.id}", group_maintainer)
+
+          expect(response).to have_http_status(403)
+        end
+
+        it 'deletes group runner with owner access' do
+          expect do
+            delete api("/runners/#{group_runner.id}", user)
+
+            expect(response).to have_http_status(204)
+          end.to change { Ci::Runner.group_type.count }.by(-1)
+        end
+
         it_behaves_like '412 response' do
           let(:request) { api("/runners/#{project_runner.id}", user) }
         end

spec/support/shared_contexts/upload_type_check_shared_context.rb

+# frozen_string_literal: true
+
+# Construct an `uploader` variable that is configured to `check_upload_type`
+# with `mime_types` and `extensions`.
+shared_context 'uploader with type check' do
+  let(:uploader_class) do
+    Class.new(GitlabUploader) do
+      include UploadTypeCheck::Concern
+      storage :file
+    end
+  end
+
+  let(:mime_types) { nil }
+  let(:extensions) { nil }
+  let(:uploader) do
+    uploader_class.class_exec(mime_types, extensions) do |mime_types, extensions|
+      check_upload_type mime_types: mime_types, extensions: extensions
+    end
+    uploader_class.new(build_stubbed(:user))
+  end
+end
+
+shared_context 'stubbed MimeMagic mime type detection' do
+  let(:mime_type) { '' }
+  let(:magic_mime) { mime_type }
+  let(:ext_mime) { mime_type }
+  before do
+    magic_mime_obj = MimeMagic.new(magic_mime)
+    ext_mime_obj = MimeMagic.new(ext_mime)
+    allow(MimeMagic).to receive(:by_magic).with(anything).and_return(magic_mime_obj)
+    allow(MimeMagic).to receive(:by_path).with(anything).and_return(ext_mime_obj)
+  end
+end

spec/support/shared_examples/uploaders/upload_type_shared_examples.rb

+# frozen_string_literal: true
+
+def check_content_matches_extension!(file = double(read: nil, path: ''))
+  magic_file = UploadTypeCheck::MagicFile.new(file)
+  uploader.check_content_matches_extension!(magic_file)
+end
+
+shared_examples 'upload passes content type check' do
+  it 'does not raise error' do
+    expect { check_content_matches_extension! }.not_to raise_error
+  end
+end
+
+shared_examples 'upload fails content type check' do
+  it 'raises error' do
+    expect { check_content_matches_extension! }.to raise_error(CarrierWave::IntegrityError)
+  end
+end
+
+def upload_type_checked_filenames(filenames)
+  Array(filenames).each do |filename|
+    # Feed the uploader "some" content.
+    path = File.join('spec', 'fixtures', 'dk.png')
+    file = File.new(path, 'r')
+    # Rename the file with what we want.
+    allow(file).to receive(:path).and_return(filename)
+
+    # Force the content type to match the extension type.
+    mime_type = MimeMagic.by_path(filename)
+    allow(MimeMagic).to receive(:by_magic).and_return(mime_type)
+
+    uploaded_file = Rack::Test::UploadedFile.new(file, original_filename: filename)
+    uploader.cache!(uploaded_file)
+  end
+end
+
+def upload_type_checked_fixtures(upload_fixtures)
+  upload_fixtures = Array(upload_fixtures)
+  upload_fixtures.each do |upload_fixture|
+    path = File.join('spec', 'fixtures', upload_fixture)
+    uploader.cache!(fixture_file_upload(path))
+  end
+end
+
+shared_examples 'type checked uploads' do |upload_fixtures = nil, filenames: nil|
+  it 'check type' do
+    upload_fixtures = Array(upload_fixtures)
+    filenames = Array(filenames)
+
+    times = upload_fixtures.length + filenames.length
+    expect(uploader).to receive(:check_content_matches_extension!).exactly(times).times
+
+    upload_type_checked_fixtures(upload_fixtures) unless upload_fixtures.empty?
+    upload_type_checked_filenames(filenames) unless filenames.empty?
+  end
+end
+
+shared_examples 'skipped type checked uploads' do |upload_fixtures = nil, filenames: nil|
+  it 'skip type check' do
+    expect(uploader).not_to receive(:check_content_matches_extension!)
+
+    upload_type_checked_fixtures(upload_fixtures) if upload_fixtures
+    upload_type_checked_filenames(filenames) if filenames
+  end
+end

spec/uploaders/avatar_uploader_spec.rb

@@ -46,4 +46,16 @@ describe AvatarUploader do
       expect(uploader.absolute_path).to eq(absolute_path)
     end
   end
+
+  context 'upload type check' do
+    AvatarUploader::SAFE_IMAGE_EXT.each do |ext|
+      context "#{ext} extension" do
+        it_behaves_like 'type checked uploads', filenames: "image.#{ext}"
+      end
+    end
+
+    context 'skip image/svg+xml integrity check' do
+      it_behaves_like 'skipped type checked uploads', filenames: 'image.svg'
+    end
+  end
 end

spec/uploaders/favicon_uploader_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe FaviconUploader do
+  let_it_be(:model) { build_stubbed(:user) }
+  let_it_be(:uploader) { described_class.new(model, :favicon) }
+
+  context 'upload type check' do
+    FaviconUploader::EXTENSION_WHITELIST.each do |ext|
+      context "#{ext} extension" do
+        it_behaves_like 'type checked uploads', filenames: "image.#{ext}"
+      end
+    end
+  end
+
+  context 'upload non-whitelisted file extensions' do
+    it 'will deny upload' do
+      path = File.join('spec', 'fixtures', 'banana_sample.gif')
+      fixture_file = fixture_file_upload(path)
+      expect { uploader.cache!(fixture_file) }.to raise_exception(CarrierWave::IntegrityError)
+    end
+  end
+end

spec/uploaders/upload_type_check_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe UploadTypeCheck do
+  include_context 'uploader with type check'
+
+  def upload_fixture(filename)
+    fixture_file_upload(File.join('spec', 'fixtures', filename))
+  end
+
+  describe '#check_content_matches_extension! callback using file upload' do
+    context 'when extension matches contents' do
+      it 'not raise error on upload' do
+        expect { uploader.cache!(upload_fixture('banana_sample.gif')) }.not_to raise_error
+      end
+    end
+
+    context 'when extension does not match contents' do
+      it 'raise error' do
+        expect { uploader.cache!(upload_fixture('not_a_png.png')) }.to raise_error(CarrierWave::IntegrityError)
+      end
+    end
+  end
+
+  describe '#check_content_matches_extension! callback using stubs' do
+    include_context 'stubbed MimeMagic mime type detection'
+
+    context 'when no extension and with ambiguous/text content' do
+      let(:magic_mime) { '' }
+      let(:ext_mime) { '' }
+
+      it_behaves_like 'upload passes content type check'
+    end
+
+    context 'when no extension and with non-text content' do
+      let(:magic_mime) { 'image/gif' }
+      let(:ext_mime) { '' }
+
+      it_behaves_like 'upload fails content type check'
+    end
+
+    # Most text files will exhibit this behaviour.
+    context 'when ambiguous content with text extension' do
+      let(:magic_mime) { '' }
+      let(:ext_mime) { 'text/plain' }
+
+      it_behaves_like 'upload passes content type check'
+    end
+
+    context 'when text content with text extension' do
+      let(:magic_mime) { 'text/plain' }
+      let(:ext_mime) { 'text/plain' }
+
+      it_behaves_like 'upload passes content type check'
+    end
+
+    context 'when ambiguous content with non-text extension' do
+      let(:magic_mime) { '' }
+      let(:ext_mime) { 'application/zip' }
+
+      it_behaves_like 'upload fails content type check'
+    end
+
+    # These are the types when uploading a .dmg
+    context 'when content and extension do not match' do
+      let(:magic_mime) { 'application/x-bzip' }
+      let(:ext_mime) { 'application/x-apple-diskimage' }
+
+      it_behaves_like 'upload fails content type check'
+    end
+  end
+
+  describe '#check_content_matches_extension! mime_type filtering' do
+    context 'without mime types' do
+      let(:mime_types) { nil }
+
+      it_behaves_like 'type checked uploads', %w[doc_sample.txt rails_sample.jpg]
+    end
+
+    context 'with mime types string' do
+      let(:mime_types) { 'text/plain' }
+
+      it_behaves_like 'type checked uploads', %w[doc_sample.txt]
+      it_behaves_like 'skipped type checked uploads', %w[dk.png]
+    end
+
+    context 'with mime types regex' do
+      let(:mime_types) { [/image\/(gif|png)/] }
+
+      it_behaves_like 'type checked uploads', %w[banana_sample.gif dk.png]
+      it_behaves_like 'skipped type checked uploads', %w[doc_sample.txt]
+    end
+
+    context 'with mime types array' do
+      let(:mime_types) { ['text/plain', /image\/png/] }
+
+      it_behaves_like 'type checked uploads', %w[doc_sample.txt dk.png]
+      it_behaves_like 'skipped type checked uploads', %w[audio_sample.wav]
+    end
+  end
+
+  describe '#check_content_matches_extension! extensions filtering' do
+    context 'without extensions' do
+      let(:extensions) { nil }
+
+      it_behaves_like 'type checked uploads', %w[doc_sample.txt dk.png]
+    end
+
+    context 'with extensions string' do
+      let(:extensions) { 'txt' }
+
+      it_behaves_like 'type checked uploads', %w[doc_sample.txt]
+      it_behaves_like 'skipped type checked uploads', %w[rails_sample.jpg]
+    end
+
+    context 'with extensions array of strings' do
+      let(:extensions) { %w[txt png] }
+
+      it_behaves_like 'type checked uploads', %w[doc_sample.txt dk.png]
+      it_behaves_like 'skipped type checked uploads', %w[audio_sample.wav]
+    end
+  end
+end