Add latest changes from gitlab-org/gitlab@master

36 files changed, 818 insertions, 109 deletions

diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb
index c062de468fc..21169188386 100644
--- a/spec/controllers/groups/group_links_controller_spec.rb
+++ b/spec/controllers/groups/group_links_controller_spec.rb
@@ -6,9 +6,13 @@ describe Groups::GroupLinksController do
   let(:shared_with_group) { create(:group, :private) }
   let(:shared_group) { create(:group, :private) }
   let(:user) { create(:user) }
+  let(:group_member) { create(:user) }
+  let!(:project) { create(:project, group: shared_group) }
 
   before do
     sign_in(user)
+
+    shared_with_group.add_developer(group_member)
   end
 
   describe '#create' do
@@ -40,13 +44,9 @@ describe Groups::GroupLinksController do
     end
 
     context 'when user has correct access to both groups' do
-      let(:group_member) { create(:user) }
-
       before do
         shared_with_group.add_developer(user)
         shared_group.add_owner(user)
-
-        shared_with_group.add_developer(group_member)
       end
 
       context 'when default access level is requested' do
@@ -56,6 +56,10 @@ describe Groups::GroupLinksController do
       context 'when owner access is requested' do
         let(:shared_group_access) { Gitlab::Access::OWNER }
 
+        before do
+          shared_with_group.add_owner(group_member)
+        end
+
         include_examples 'creates group group link'
 
         it 'allows admin access for group member' do
@@ -64,6 +68,10 @@ describe Groups::GroupLinksController do
         end
       end
 
+      it 'updates project permissions' do
+        expect { subject }.to change { group_member.can?(:read_project, project) }.from(false).to(true)
+      end
+
       context 'when shared with group id is not present' do
         let(:shared_with_group_id) { nil }
 
@@ -149,6 +157,7 @@ describe Groups::GroupLinksController do
     context 'when user has admin access to the shared group' do
       before do
         shared_group.add_owner(user)
+        shared_with_group.refresh_members_authorized_projects
       end
 
       it 'updates existing link' do
@@ -162,6 +171,10 @@ describe Groups::GroupLinksController do
         expect(link.group_access).to eq(Gitlab::Access::GUEST)
         expect(link.expires_at).to eq(expiry_date)
       end
+
+      it 'updates project permissions' do
+        expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
+      end
     end
 
     context 'when user does not have admin access to the shared group' do
@@ -199,11 +212,16 @@ describe Groups::GroupLinksController do
     context 'when user has admin access to the shared group' do
       before do
         shared_group.add_owner(user)
+        shared_with_group.refresh_members_authorized_projects
       end
 
       it 'deletes existing link' do
         expect { subject }.to change(GroupGroupLink, :count).by(-1)
       end
+
+      it 'updates project permissions' do
+        expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
+      end
     end
 
     context 'when user does not have admin access to the shared group' do
diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index 16947f4d3be..a675014a77b 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -260,8 +260,10 @@ describe SnippetsController do
     context 'when the snippet description contains a file' do
       include FileMoverHelpers
 
-      let(:picture_file) { "/-/system/user/#{user.id}/secret56/picture.jpg" }
-      let(:text_file) { "/-/system/user/#{user.id}/secret78/text.txt" }
+      let(:picture_secret) { SecureRandom.hex }
+      let(:text_secret) { SecureRandom.hex }
+      let(:picture_file) { "/-/system/user/#{user.id}/#{picture_secret}/picture.jpg" }
+      let(:text_file) { "/-/system/user/#{user.id}/#{text_secret}/text.txt" }
       let(:description) do
         "Description with picture: ![picture](/uploads#{picture_file}) and "\
         "text: [text.txt](/uploads#{text_file})"
@@ -284,8 +286,8 @@ describe SnippetsController do
         snippet = subject
 
         expected_description = "Description with picture: "\
-          "![picture](/uploads/-/system/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\
-          "text: [text.txt](/uploads/-/system/personal_snippet/#{snippet.id}/secret78/text.txt)"
+          "![picture](/uploads/-/system/personal_snippet/#{snippet.id}/#{picture_secret}/picture.jpg) and "\
+          "text: [text.txt](/uploads/-/system/personal_snippet/#{snippet.id}/#{text_secret}/text.txt)"
 
         expect(snippet.description).to eq(expected_description)
       end
diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb
index 0d24b02a64c..3bb70fdf376 100644
--- a/spec/features/issues/issue_detail_spec.rb
+++ b/spec/features/issues/issue_detail_spec.rb
@@ -28,7 +28,7 @@ describe 'Issue Detail', :js do
       visit project_issue_path(project, issue)
     end
 
-    it 'encodes the description to prevent xss issues' do
+    it 'encodes the description to prevent xss issues', quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/207951' do
       page.within('.issuable-details .detail-page-description') do
         image = find('img.js-lazy-loaded')
 
diff --git a/spec/features/merge_request/user_creates_merge_request_spec.rb b/spec/features/merge_request/user_creates_merge_request_spec.rb
index 67f6d8ebe32..86ee9fa5aa5 100644
--- a/spec/features/merge_request/user_creates_merge_request_spec.rb
+++ b/spec/features/merge_request/user_creates_merge_request_spec.rb
@@ -5,9 +5,9 @@ require "spec_helper"
 describe "User creates a merge request", :js do
   include ProjectForksHelper
 
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
   let(:title) { "Some feature" }
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user) }
 
   before do
     project.add_maintainer(user)
@@ -38,6 +38,26 @@ describe "User creates a merge request", :js do
     end
   end
 
+  context "XSS branch name exists" do
+    before do
+      project.repository.create_branch("<img/src='x'/onerror=alert('oops')>", "master")
+    end
+
+    it "doesn't execute the dodgy branch name" do
+      visit(project_new_merge_request_path(project))
+
+      find(".js-source-branch").click
+      click_link("<img/src='x'/onerror=alert('oops')>")
+
+      find(".js-target-branch").click
+      click_link("feature")
+
+      click_button("Compare branches")
+
+      expect { page.driver.browser.switch_to.alert }.to raise_error(Selenium::WebDriver::Error::NoSuchAlertError)
+    end
+  end
+
   context "to a forked project" do
     let(:forked_project) { fork_project(project, user, namespace: user.namespace, repository: true) }
 
diff --git a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb
index d3a0c9b790b..9d9c83331fb 100644
--- a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb
+++ b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe 'User updates wiki page' do
+  include WikiHelpers
+
   let(:user) { create(:user) }
 
   before do
@@ -11,8 +13,11 @@ describe 'User updates wiki page' do
   end
 
   context 'when wiki is empty' do
-    before do
+    before do |example|
       visit(project_wikis_path(project))
+
+      wait_for_svg_to_be_loaded(example)
+
       click_link "Create your first page"
     end
 
diff --git a/spec/fixtures/lib/gitlab/import_export/complex/project.json b/spec/fixtures/lib/gitlab/import_export/complex/project.json
index fae02540868..72ed8b818d8 100644
--- a/spec/fixtures/lib/gitlab/import_export/complex/project.json
+++ b/spec/fixtures/lib/gitlab/import_export/complex/project.json
@@ -6452,7 +6452,7 @@
       ]
     },
     {
-      "id": 37,
+      "id": 26,
       "project_id": 5,
       "ref": "master",
       "sha": "048721d90c449b244b7b4c53a9186b04330174ec",
@@ -6744,7 +6744,7 @@
       ]
     },
     {
-      "id": 40,
+      "id": 19,
       "project_id": 5,
       "ref": "master",
       "sha": "2ea1f3dec713d940208fb5ce4a38765ecb5d3f73",
@@ -6851,7 +6851,7 @@
       ]
     },
     {
-      "id": 42,
+      "id": 20,
       "project_id": 5,
       "ref": "master",
       "sha": "ce84140e8b878ce6e7c4d298c7202ff38170e3ac",
diff --git a/spec/frontend/error_tracking/components/error_details_spec.js b/spec/frontend/error_tracking/components/error_details_spec.js
index e43f9569ffc..2baec5f37fb 100644
--- a/spec/frontend/error_tracking/components/error_details_spec.js
+++ b/spec/frontend/error_tracking/components/error_details_spec.js
@@ -137,6 +137,28 @@ describe('ErrorDetails', () => {
       expect(wrapper.findAll(GlButton).length).toBe(3);
     });
 
+    describe('unsafe chars for culprit field', () => {
+      const findReportedText = () => wrapper.find('[data-qa-selector="reported_text"]');
+      const culprit = '<script>console.log("surprise!")</script>';
+      beforeEach(() => {
+        store.state.details.loadingStacktrace = false;
+        wrapper.setData({
+          error: {
+            culprit,
+          },
+        });
+      });
+
+      it('should not convert interpolated text to html entities', () => {
+        expect(findReportedText().findAll('script').length).toEqual(0);
+        expect(findReportedText().findAll('strong').length).toEqual(1);
+      });
+
+      it('should render text instead of converting to html entities', () => {
+        expect(findReportedText().text()).toContain(culprit);
+      });
+    });
+
     describe('Badges', () => {
       it('should show language and error level badges', () => {
         wrapper.setData({
diff --git a/spec/graphql/types/diff_refs_type_spec.rb b/spec/graphql/types/diff_refs_type_spec.rb
index 91017c827ad..85225e5809c 100644
--- a/spec/graphql/types/diff_refs_type_spec.rb
+++ b/spec/graphql/types/diff_refs_type_spec.rb
@@ -5,5 +5,9 @@ require 'spec_helper'
 describe GitlabSchema.types['DiffRefs'] do
   it { expect(described_class.graphql_name).to eq('DiffRefs') }
 
-  it { expect(described_class).to have_graphql_fields(:base_sha, :head_sha, :start_sha) }
+  it { is_expected.to have_graphql_fields(:head_sha, :base_sha, :start_sha).only }
+
+  it { expect(described_class.fields['headSha'].type).to be_non_null }
+  it { expect(described_class.fields['baseSha'].type).not_to be_non_null }
+  it { expect(described_class.fields['startSha'].type).to be_non_null }
 end
diff --git a/spec/lib/gitlab/asset_proxy_spec.rb b/spec/lib/gitlab/asset_proxy_spec.rb
new file mode 100644
index 00000000000..f5aa1819982
--- /dev/null
+++ b/spec/lib/gitlab/asset_proxy_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::AssetProxy do
+  context 'when asset proxy is disabled' do
+    before do
+      stub_asset_proxy_setting(enabled: false)
+    end
+
+    it 'returns the original URL' do
+      url = 'http://example.com/test.png'
+
+      expect(described_class.proxy_url(url)).to eq(url)
+    end
+  end
+
+  context 'when asset proxy is enabled' do
+    before do
+      stub_asset_proxy_setting(whitelist: %w(gitlab.com *.mydomain.com))
+      stub_asset_proxy_setting(
+        enabled: true,
+        url: 'https://assets.example.com',
+        secret_key: 'shared-secret',
+        domain_regexp: Banzai::Filter::AssetProxyFilter.compile_whitelist(Gitlab.config.asset_proxy.whitelist)
+      )
+    end
+
+    it 'returns a proxied URL' do
+      url = 'http://example.com/test.png'
+      proxied_url = 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67'
+
+      expect(described_class.proxy_url(url)).to eq(proxied_url)
+    end
+
+    context 'whitelisted domain' do
+      it 'returns original URL for single domain whitelist' do
+        url = 'http://gitlab.com/test.png'
+
+        expect(described_class.proxy_url(url)).to eq(url)
+      end
+
+      it 'returns original URL for wildcard subdomain whitelist' do
+        url = 'http://test.mydomain.com/test.png'
+
+        expect(described_class.proxy_url(url)).to eq(url)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/background_migration/recalculate_project_authorizations_with_min_max_user_id_spec.rb b/spec/lib/gitlab/background_migration/recalculate_project_authorizations_with_min_max_user_id_spec.rb
new file mode 100644
index 00000000000..14ba57eecbf
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/recalculate_project_authorizations_with_min_max_user_id_spec.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::RecalculateProjectAuthorizationsWithMinMaxUserId, :migration, schema: 20200204113224 do
+  let(:users_table) { table(:users) }
+  let(:min) { 1 }
+  let(:max) { 5 }
+
+  before do
+    min.upto(max) do |i|
+      users_table.create!(id: i, email: "user#{i}@example.com", projects_limit: 10)
+    end
+  end
+
+  describe '#perform' do
+    it 'initializes Users::RefreshAuthorizedProjectsService with correct users' do
+      min.upto(max) do |i|
+        user = User.find(i)
+        expect(Users::RefreshAuthorizedProjectsService).to(
+          receive(:new).with(user, any_args).and_call_original)
+      end
+
+      described_class.new.perform(min, max)
+    end
+
+    it 'executes Users::RefreshAuthorizedProjectsService' do
+      expected_call_counts = max - min + 1
+
+      service = instance_double(Users::RefreshAuthorizedProjectsService)
+      expect(Users::RefreshAuthorizedProjectsService).to(
+        receive(:new).exactly(expected_call_counts).times.and_return(service))
+      expect(service).to receive(:execute).exactly(expected_call_counts).times
+
+      described_class.new.perform(min, max)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/dependency_linker/base_linker_spec.rb b/spec/lib/gitlab/dependency_linker/base_linker_spec.rb
new file mode 100644
index 00000000000..1466ce2dfcc
--- /dev/null
+++ b/spec/lib/gitlab/dependency_linker/base_linker_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::DependencyLinker::BaseLinker do
+  let(:linker_class) do
+    Class.new(described_class) do
+      def link_dependencies
+        link_regex(%r{^(?<name>https?://[^ ]+)}, &:itself)
+      end
+    end
+  end
+
+  let(:plain_content) do
+    <<~CONTENT
+      http://\\njavascript:alert(1)
+      https://gitlab.com/gitlab-org/gitlab
+    CONTENT
+  end
+
+  let(:highlighted_content) do
+    <<~CONTENT
+      <span><span>http://</span><span>\\n</span><span>javascript:alert(1)</span></span>
+      <span><span>https://gitlab.com/gitlab-org/gitlab</span></span>
+    CONTENT
+  end
+
+  let(:linker) { linker_class.new(plain_content, highlighted_content) }
+
+  describe '#link' do
+    subject { linker.link }
+
+    it 'only converts valid links' do
+      expect(subject).to eq(
+        <<~CONTENT
+          <span><span>#{link('http://')}</span><span>#{link('\n', url: '%5Cn')}</span><span>#{link('javascript:alert(1)', url: nil)}</span></span>
+          <span><span>#{link('https://gitlab.com/gitlab-org/gitlab')}</span></span>
+        CONTENT
+      )
+    end
+  end
+
+  def link(text, url: text)
+    attrs = [
+      'rel="nofollow noreferrer noopener"',
+      'target="_blank"'
+    ]
+
+    attrs.unshift(%{href="#{url}"}) if url
+
+    %{<a #{attrs.join(' ')}>#{text}</a>}
+  end
+end
diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
index a46c6579670..7bc17b804df 100644
--- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
@@ -104,6 +104,24 @@ describe Gitlab::ImportExport::Project::TreeRestorer do
         expect(pipeline.merge_request.source_branch).to eq('feature_conflict')
       end
 
+      it 'restores pipelines based on ascending id order' do
+        expected_ordered_shas = %w[
+          2ea1f3dec713d940208fb5ce4a38765ecb5d3f73
+          ce84140e8b878ce6e7c4d298c7202ff38170e3ac
+          048721d90c449b244b7b4c53a9186b04330174ec
+          sha-notes
+          5f923865dde3436854e9ceb9cdb7815618d4e849
+          d2d430676773caa88cdaf7c55944073b2fd5561a
+          2ea1f3dec713d940208fb5ce4a38765ecb5d3f73
+        ]
+
+        project = Project.find_by_path('project')
+
+        project.ci_pipelines.order(:id).each_with_index do |pipeline, i|
+          expect(pipeline['sha']).to eq expected_ordered_shas[i]
+        end
+      end
+
       it 'preserves updated_at on issues' do
         issue = Issue.where(description: 'Aliquam enim illo et possimus.').first
 
@@ -385,7 +403,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer do
         it 'has the correct number of pipelines and statuses' do
           expect(@project.ci_pipelines.size).to eq(7)
 
-          @project.ci_pipelines.order(:id).zip([2, 2, 2, 2, 2, 0, 0])
+          @project.ci_pipelines.order(:id).zip([2, 0, 2, 2, 2, 2, 0])
             .each do |(pipeline, expected_status_size)|
             expect(pipeline.statuses.size).to eq(expected_status_size)
           end
@@ -422,7 +440,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer do
         end
 
         it 'restores external pull request for the restored pipeline' do
-          pipeline_with_external_pr = @project.ci_pipelines.order(:id).last
+          pipeline_with_external_pr = @project.ci_pipelines.where(source: 'external_pull_request_event').first
 
           expect(pipeline_with_external_pr.external_pull_request).to be_persisted
         end
diff --git a/spec/lib/gitlab/project_authorizations_spec.rb b/spec/lib/gitlab/project_authorizations_spec.rb
index 1c579128223..7b282433061 100644
--- a/spec/lib/gitlab/project_authorizations_spec.rb
+++ b/spec/lib/gitlab/project_authorizations_spec.rb
@@ -109,6 +109,20 @@ describe Gitlab::ProjectAuthorizations do
       end
     end
 
+    context 'with lower group access level than max access level for share' do
+      let(:user) { create(:user) }
+
+      it 'creates proper authorizations' do
+        group.add_reporter(user)
+
+        mapping = map_access_levels(authorizations)
+
+        expect(mapping[project_parent.id]).to be_nil
+        expect(mapping[project.id]).to eq(Gitlab::Access::REPORTER)
+        expect(mapping[project_child.id]).to eq(Gitlab::Access::REPORTER)
+      end
+    end
+
     context 'parent group user' do
       let(:user) { parent_group_user }
 
diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb
index 8d13f377677..78370f0136c 100644
--- a/spec/lib/gitlab/user_access_spec.rb
+++ b/spec/lib/gitlab/user_access_spec.rb
@@ -30,6 +30,17 @@ describe Gitlab::UserAccess do
       end
     end
 
+    describe 'push to branch in an internal project' do
+      it 'will not infinitely loop when a project is internal' do
+        project.visibility_level = Gitlab::VisibilityLevel::INTERNAL
+        project.save!
+
+        expect(project).not_to receive(:branch_allows_collaboration?)
+
+        access.can_push_to_branch?('master')
+      end
+    end
+
     describe 'push to empty project' do
       let(:empty_project) { create(:project_empty_repo) }
       let(:project_access) { described_class.new(user, project: empty_project) }
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 48fc2d826bc..d3780d22241 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -291,4 +291,18 @@ describe Gitlab::Utils do
       expect(described_class.string_to_ip_object('1:0:0:0:0:0:0:0/124')).to eq(IPAddr.new('1:0:0:0:0:0:0:0/124'))
     end
   end
+
+  describe '.parse_url' do
+    it 'returns Addressable::URI object' do
+      expect(described_class.parse_url('http://gitlab.com')).to be_instance_of(Addressable::URI)
+    end
+
+    it 'returns nil when URI cannot be parsed' do
+      expect(described_class.parse_url('://gitlab.com')).to be nil
+    end
+
+    it 'returns nil with invalid parameter' do
+      expect(described_class.parse_url(1)).to be nil
+    end
+  end
 end
diff --git a/spec/migrations/clean_grafana_url_spec.rb b/spec/migrations/clean_grafana_url_spec.rb
new file mode 100644
index 00000000000..9f060fbaf7d
--- /dev/null
+++ b/spec/migrations/clean_grafana_url_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20200214085940_clean_grafana_url.rb')
+
+describe CleanGrafanaUrl, :migration do
+  let(:application_settings_table) { table(:application_settings) }
+
+  [
+    'javascript:alert(window.opener.document.location)',
+    '  javascript:alert(window.opener.document.location)'
+  ].each do |grafana_url|
+    it "sets grafana_url back to its default value when grafana_url is '#{grafana_url}'" do
+      application_settings = application_settings_table.create!(grafana_url: grafana_url)
+
+      migrate!
+
+      expect(application_settings.reload.grafana_url).to eq('/-/grafana')
+    end
+  end
+
+  ['/-/grafana', '/some/relative/url', 'http://localhost:9000'].each do |grafana_url|
+    it "does not modify grafana_url when grafana_url is '#{grafana_url}'" do
+      application_settings = application_settings_table.create!(grafana_url: grafana_url)
+
+      migrate!
+
+      expect(application_settings.reload.grafana_url).to eq(grafana_url)
+    end
+  end
+
+  context 'when application_settings table has no rows' do
+    it 'does not fail' do
+      migrate!
+    end
+  end
+end
diff --git a/spec/migrations/schedule_recalculate_project_authorizations_second_run_spec.rb b/spec/migrations/schedule_recalculate_project_authorizations_second_run_spec.rb
new file mode 100644
index 00000000000..04726f98c89
--- /dev/null
+++ b/spec/migrations/schedule_recalculate_project_authorizations_second_run_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200204113224_schedule_recalculate_project_authorizations_second_run.rb')
+
+describe ScheduleRecalculateProjectAuthorizationsSecondRun, :migration do
+  let(:users_table) { table(:users) }
+
+  before do
+    stub_const("#{described_class}::BATCH_SIZE", 2)
+
+    1.upto(4) do |i|
+      users_table.create!(id: i, name: "user#{i}", email: "user#{i}@example.com", projects_limit: 1)
+    end
+  end
+
+  it 'schedules background migration' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+        expect(described_class::MIGRATION).to be_scheduled_migration(1, 2)
+        expect(described_class::MIGRATION).to be_scheduled_migration(3, 4)
+      end
+    end
+  end
+end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 82ece6b2b91..9dad6b1e766 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -19,6 +19,7 @@ describe ApplicationSetting do
     let(:http)  { 'http://example.com' }
     let(:https) { 'https://example.com' }
     let(:ftp)   { 'ftp://example.com' }
+    let(:javascript) { 'javascript:alert(window.opener.document.location)' }
 
     it { is_expected.to allow_value(nil).for(:home_page_url) }
     it { is_expected.to allow_value(http).for(:home_page_url) }
@@ -81,6 +82,53 @@ describe ApplicationSetting do
     it { is_expected.not_to allow_value('abc').for(:minimum_password_length) }
     it { is_expected.to allow_value(10).for(:minimum_password_length) }
 
+    context 'grafana_url validations' do
+      before do
+        subject.instance_variable_set(:@parsed_grafana_url, nil)
+      end
+
+      it { is_expected.to allow_value(http).for(:grafana_url) }
+      it { is_expected.to allow_value(https).for(:grafana_url) }
+      it { is_expected.not_to allow_value(ftp).for(:grafana_url) }
+      it { is_expected.not_to allow_value(javascript).for(:grafana_url) }
+      it { is_expected.to allow_value('/-/grafana').for(:grafana_url) }
+      it { is_expected.to allow_value('http://localhost:9000').for(:grafana_url) }
+
+      context 'when local URLs are not allowed in system hooks' do
+        before do
+          stub_application_setting(allow_local_requests_from_system_hooks: false)
+        end
+
+        it { is_expected.not_to allow_value('http://localhost:9000').for(:grafana_url) }
+      end
+
+      context 'with invalid grafana URL' do
+        it 'adds an error' do
+          subject.grafana_url = ' ' + http
+          expect(subject.save).to be false
+
+          expect(subject.errors[:grafana_url]).to eq([
+            'must be a valid relative or absolute URL. ' \
+            'Please check your Grafana URL setting in ' \
+            'Admin Area > Settings > Metrics and profiling > Metrics - Grafana'
+          ])
+        end
+      end
+
+      context 'with blocked grafana URL' do
+        it 'adds an error' do
+          subject.grafana_url = javascript
+          expect(subject.save).to be false
+
+          expect(subject.errors[:grafana_url]).to eq([
+            'is blocked: Only allowed schemes are http, https. Please check your ' \
+            'Grafana URL setting in ' \
+            'Admin Area > Settings > Metrics and profiling > Metrics - Grafana'
+          ])
+        end
+      end
+    end
+
     context 'when snowplow is enabled' do
       before do
         setting.snowplow_enabled = true
diff --git a/spec/models/badge_spec.rb b/spec/models/badge_spec.rb
index 60ae579eb03..fba8f40e99b 100644
--- a/spec/models/badge_spec.rb
+++ b/spec/models/badge_spec.rb
@@ -91,6 +91,22 @@ describe Badge do
       let(:method) { :image_url }
 
       it_behaves_like 'rendered_links'
+
+      context 'when asset proxy is enabled' do
+        let(:placeholder_url) { 'http://www.example.com/image' }
+
+        before do
+          stub_asset_proxy_setting(
+            enabled: true,
+            url: 'https://assets.example.com',
+            secret_key: 'shared-secret'
+          )
+        end
+
+        it 'returns a proxied URL' do
+          expect(badge.rendered_image_url).to start_with('https://assets.example.com')
+        end
+      end
     end
   end
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index b5ed29189fd..576ac4393ed 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -563,6 +563,18 @@ describe Group do
             expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER)
             expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER)
           end
+
+          context 'with lower group access level than max access level for share' do
+            let(:user) { create(:user) }
+
+            it 'returns correct access level' do
+              group.add_reporter(user)
+
+              expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+              expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
+              expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER)
+            end
+          end
         end
 
         context 'with user in the parent group' do
@@ -584,6 +596,33 @@ describe Group do
             expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
           end
         end
+
+        context 'unrelated project owner' do
+          let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 }
+          let!(:group) { create(:group, id: common_id) }
+          let!(:unrelated_project) { create(:project, id: common_id) }
+          let(:user) { unrelated_project.owner }
+
+          it 'returns correct access level' do
+            expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+            expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+            expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+          end
+        end
+
+        context 'user without accepted access request' do
+          let!(:user) { create(:user) }
+
+          before do
+            create(:group_member, :developer, :access_request, user: user, group: group)
+          end
+
+          it 'returns correct access level' do
+            expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+            expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+            expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS)
+          end
+        end
       end
 
       context 'when feature flag share_group_with_group is disabled' do
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index ad7dfac87af..9b5cce1aebf 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -65,10 +65,10 @@ describe GroupMember do
   end
 
   describe '#update_two_factor_requirement' do
-    let(:user) { build :user }
-    let(:group_member) { build :group_member, user: user }
-
     it 'is called after creation and deletion' do
+      user = build :user
+      group_member = build :group_member, user: user
+
       expect(user).to receive(:update_two_factor_requirement)
 
       group_member.save
@@ -79,6 +79,21 @@ describe GroupMember do
     end
   end
 
+  describe '#after_accept_invite' do
+    it 'calls #update_two_factor_requirement' do
+      email = 'foo@email.com'
+      user = build(:user, email: email)
+      group = create(:group, require_two_factor_authentication: true)
+      group_member = create(:group_member, group: group, invite_token: '1234', invite_email: email)
+
+      expect(user).to receive(:require_two_factor_authentication_from_group).and_call_original
+
+      group_member.accept_invite!(user)
+
+      expect(user.require_two_factor_authentication_from_group).to be_truthy
+    end
+  end
+
   context 'access levels' do
     context 'with parent group' do
       it_behaves_like 'inherited access level as a member of entity' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index cbd55077316..56f4c68a913 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4866,6 +4866,38 @@ describe Project do
     end
   end
 
+  context 'with cross internal project merge requests' do
+    let(:project) { create(:project, :repository, :internal) }
+    let(:forked_project) { fork_project(project, nil, repository: true) }
+    let(:user) { double(:user) }
+
+    it "does not endlessly loop for internal projects with MRs to each other", :sidekiq_inline do
+      allow(user).to receive(:can?).and_return(true, false, true)
+      allow(user).to receive(:id).and_return(1)
+
+      create(
+        :merge_request,
+        target_project: project,
+        target_branch: 'merge-test',
+        source_project: forked_project,
+        source_branch: 'merge-test',
+        allow_collaboration: true
+      )
+
+      create(
+        :merge_request,
+        target_project: forked_project,
+        target_branch: 'merge-test',
+        source_project: project,
+        source_branch: 'merge-test',
+        allow_collaboration: true
+      )
+
+      expect(user).to receive(:can?).at_most(5).times
+      project.branch_allows_collaboration?(user, "merge-test")
+    end
+  end
+
   context 'with cross project merge requests' do
     let(:user) { create(:user) }
     let(:target_project) { create(:project, :repository) }
diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb
index 3b6de5bb390..2b2bfff7be2 100644
--- a/spec/models/user_detail_spec.rb
+++ b/spec/models/user_detail_spec.rb
@@ -7,7 +7,7 @@ describe UserDetail do
 
   describe 'validations' do
     describe 'job_title' do
-      it { is_expected.to validate_presence_of(:job_title) }
+      it { is_expected.not_to validate_presence_of(:job_title) }
       it { is_expected.to validate_length_of(:job_title).is_at_most(200) }
     end
   end
diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb
index 700d1f5cbb6..c9c4f567549 100644
--- a/spec/presenters/ci/pipeline_presenter_spec.rb
+++ b/spec/presenters/ci/pipeline_presenter_spec.rb
@@ -6,6 +6,7 @@ describe Ci::PipelinePresenter do
   include Gitlab::Routing
 
   let(:user) { create(:user) }
+  let(:current_user) { user }
   let(:project) { create(:project) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
 
@@ -15,7 +16,7 @@ describe Ci::PipelinePresenter do
 
   before do
     project.add_developer(user)
-    allow(presenter).to receive(:current_user) { user }
+    allow(presenter).to receive(:current_user) { current_user }
   end
 
   it 'inherits from Gitlab::View::Presenter::Delegated' do
@@ -224,10 +225,90 @@ describe Ci::PipelinePresenter do
   describe '#all_related_merge_requests' do
     it 'memoizes the returned relation' do
       query_count = ActiveRecord::QueryRecorder.new do
-        2.times { presenter.send(:all_related_merge_requests).count }
+        3.times { presenter.send(:all_related_merge_requests).count }
       end.count
 
-      expect(query_count).to eq(1)
+      expect(query_count).to eq(2)
+    end
+
+    context 'permissions' do
+      let!(:merge_request) do
+        create(:merge_request, project: project, source_project: project)
+      end
+
+      subject(:all_related_merge_requests) do
+        presenter.send(:all_related_merge_requests)
+      end
+
+      shared_examples 'private merge requests' do
+        context 'when not logged in' do
+          let(:current_user) {}
+
+          it { is_expected.to be_empty }
+        end
+
+        context 'when logged in as a non_member' do
+          let(:current_user) { create(:user) }
+
+          it { is_expected.to be_empty }
+        end
+
+        context 'when logged in as a guest' do
+          let(:current_user) { create(:user) }
+
+          before do
+            project.add_guest(current_user)
+          end
+
+          it { is_expected.to be_empty }
+        end
+
+        context 'when logged in as a developer' do
+          it { is_expected.to contain_exactly(merge_request) }
+        end
+
+        context 'when logged in as a maintainer' do
+          let(:current_user) { create(:user) }
+
+          before do
+            project.add_maintainer(current_user)
+          end
+
+          it { is_expected.to contain_exactly(merge_request) }
+        end
+      end
+
+      context 'with a private project' do
+        it_behaves_like 'private merge requests'
+      end
+
+      context 'with a public project with private merge requests' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+          project
+            .project_feature
+            .update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+        end
+
+        it_behaves_like 'private merge requests'
+      end
+
+      context 'with a public project with public merge requests' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+          project
+            .project_feature
+            .update!(merge_requests_access_level: ProjectFeature::ENABLED)
+        end
+
+        context 'when not logged in' do
+          let(:current_user) {}
+
+          it { is_expected.to contain_exactly(merge_request) }
+        end
+      end
     end
   end
 
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index 0309618e243..bcc1c6bc4d4 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -116,6 +116,18 @@ describe API::Triggers do
         end
       end
     end
+
+    context 'when is triggered by a pipeline hook' do
+      it 'does not create a new pipeline' do
+        expect do
+          post api("/projects/#{project.id}/ref/master/trigger/pipeline?token=#{trigger_token}"),
+            params: { ref: 'refs/heads/other-branch' },
+            headers: { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' }
+        end.not_to change(Ci::Pipeline, :count)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
   end
 
   describe 'GET /projects/:id/triggers' do
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 5003dfcc951..84f4a7a4e7a 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -769,6 +769,15 @@ describe Auth::ContainerRegistryAuthenticationService do
     context 'when deploy token has read_registry as a scope' do
       let(:current_user) { create(:deploy_token, projects: [project]) }
 
+      shared_examples 'able to login' do
+        context 'registry provides read_container_image authentication_abilities' do
+          let(:current_params) { {} }
+          let(:authentication_abilities) { [:read_container_image] }
+
+          it_behaves_like 'an authenticated'
+        end
+      end
+
       context 'for public project' do
         let(:project) { create(:project, :public) }
 
@@ -783,6 +792,8 @@ describe Auth::ContainerRegistryAuthenticationService do
 
           it_behaves_like 'an inaccessible'
         end
+
+        it_behaves_like 'able to login'
       end
 
       context 'for internal project' do
@@ -799,6 +810,8 @@ describe Auth::ContainerRegistryAuthenticationService do
 
           it_behaves_like 'an inaccessible'
         end
+
+        it_behaves_like 'able to login'
       end
 
       context 'for private project' do
@@ -815,18 +828,38 @@ describe Auth::ContainerRegistryAuthenticationService do
 
           it_behaves_like 'an inaccessible'
         end
+
+        it_behaves_like 'able to login'
       end
     end
 
     context 'when deploy token does not have read_registry scope' do
       let(:current_user) { create(:deploy_token, projects: [project], read_registry: false) }
 
+      shared_examples 'unable to login' do
+        context 'registry provides no container authentication_abilities' do
+          let(:current_params) { {} }
+          let(:authentication_abilities) { [] }
+
+          it_behaves_like 'a forbidden'
+        end
+
+        context 'registry provides inapplicable container authentication_abilities' do
+          let(:current_params) { {} }
+          let(:authentication_abilities) { [:download_code] }
+
+          it_behaves_like 'a forbidden'
+        end
+      end
+
       context 'for public project' do
         let(:project) { create(:project, :public) }
 
         context 'when pulling' do
           it_behaves_like 'a pullable'
         end
+
+        it_behaves_like 'unable to login'
       end
 
       context 'for internal project' do
@@ -835,6 +868,8 @@ describe Auth::ContainerRegistryAuthenticationService do
         context 'when pulling' do
           it_behaves_like 'an inaccessible'
         end
+
+        it_behaves_like 'unable to login'
       end
 
       context 'for private project' do
@@ -843,6 +878,15 @@ describe Auth::ContainerRegistryAuthenticationService do
         context 'when pulling' do
           it_behaves_like 'an inaccessible'
         end
+
+        context 'when logging in' do
+          let(:current_params) { {} }
+          let(:authentication_abilities) { [] }
+
+          it_behaves_like 'a forbidden'
+        end
+
+        it_behaves_like 'unable to login'
       end
     end
 
diff --git a/spec/services/groups/group_links/destroy_service_spec.rb b/spec/services/groups/group_links/destroy_service_spec.rb
index 6f49b6eda94..284bcd0df2e 100644
--- a/spec/services/groups/group_links/destroy_service_spec.rb
+++ b/spec/services/groups/group_links/destroy_service_spec.rb
@@ -40,24 +40,11 @@ describe Groups::GroupLinks::DestroyService, '#execute' do
     end
 
     it 'updates project authorization once per group' do
-      expect(GroupGroupLink).to receive(:delete)
+      expect(GroupGroupLink).to receive(:delete).and_call_original
       expect(group).to receive(:refresh_members_authorized_projects).once
       expect(another_group).to receive(:refresh_members_authorized_projects).once
 
       subject.execute(links)
     end
-
-    it 'rolls back changes when error happens' do
-      group.add_developer(user)
-
-      expect(group).to receive(:refresh_members_authorized_projects).once.and_call_original
-      expect(another_group).to(
-        receive(:refresh_members_authorized_projects).and_raise('boom'))
-
-      expect { subject.execute(links) }.to raise_error('boom')
-
-      expect(GroupGroupLink.count).to eq(links.length)
-      expect(Ability.allowed?(user, :read_project, project)).to be_truthy
-    end
   end
 end
diff --git a/spec/services/groups/group_links/update_service_spec.rb b/spec/services/groups/group_links/update_service_spec.rb
new file mode 100644
index 00000000000..446364c9799
--- /dev/null
+++ b/spec/services/groups/group_links/update_service_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::GroupLinks::UpdateService, '#execute' do
+  let(:user) { create(:user) }
+
+  let_it_be(:group) { create(:group, :private) }
+  let_it_be(:shared_group) { create(:group, :private) }
+  let_it_be(:project) { create(:project, group: shared_group) }
+  let(:group_member) { create(:user) }
+  let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
+
+  let(:expiry_date) { 1.month.from_now.to_date }
+  let(:group_link_params) do
+    { group_access: Gitlab::Access::GUEST,
+      expires_at: expiry_date }
+  end
+
+  subject { described_class.new(link).execute(group_link_params) }
+
+  before do
+    group.add_developer(group_member)
+  end
+
+  it 'updates existing link' do
+    expect(link.group_access).to eq(Gitlab::Access::DEVELOPER)
+    expect(link.expires_at).to be_nil
+
+    subject
+
+    link.reload
+
+    expect(link.group_access).to eq(Gitlab::Access::GUEST)
+    expect(link.expires_at).to eq(expiry_date)
+  end
+
+  it 'updates project permissions' do
+    expect { subject }.to change { group_member.can?(:create_release, project) }.from(true).to(false)
+  end
+
+  it 'executes UserProjectAccessChangedService' do
+    expect_next_instance_of(UserProjectAccessChangedService) do |service|
+      expect(service).to receive(:execute)
+    end
+
+    subject
+  end
+
+  context 'with only param not requiring authorization refresh' do
+    let(:group_link_params) { { expires_at: Date.tomorrow } }
+
+    it 'does not execute UserProjectAccessChangedService' do
+      expect(UserProjectAccessChangedService).not_to receive(:new)
+
+      subject
+    end
+  end
+end
diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
index 21a139cdf3c..496d1fe67f2 100644
--- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
+++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
@@ -134,6 +134,21 @@ describe Projects::LfsPointers::LfsDownloadService do
       end
     end
 
+    context 'when an lfs object with the same oid already exists' do
+      let!(:existing_lfs_object) { create(:lfs_object, oid: oid) }
+
+      before do
+        stub_full_request(download_link).to_return(body: lfs_content)
+      end
+
+      it_behaves_like 'no lfs object is created'
+
+      it 'does not update the file attached to the existing LfsObject' do
+        expect { subject.execute }
+          .not_to change { existing_lfs_object.reload.file.file.file }
+      end
+    end
+
     context 'when credentials present' do
       let(:download_link_with_credentials) { "http://user:password@gitlab.com/#{oid}" }
       let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) }
@@ -211,17 +226,5 @@ describe Projects::LfsPointers::LfsDownloadService do
         subject.execute
       end
     end
-
-    context 'when an lfs object with the same oid already exists' do
-      before do
-        create(:lfs_object, oid: oid)
-      end
-
-      it 'does not download the file' do
-        expect(subject).not_to receive(:download_lfs_file!)
-
-        subject.execute
-      end
-    end
   end
 end
diff --git a/spec/services/projects/lfs_pointers/lfs_object_download_list_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_object_download_list_service_spec.rb
index 9dac29765a2..e94d8a85987 100644
--- a/spec/services/projects/lfs_pointers/lfs_object_download_list_service_spec.rb
+++ b/spec/services/projects/lfs_pointers/lfs_object_download_list_service_spec.rb
@@ -24,7 +24,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do
   describe '#execute' do
     context 'when no lfs pointer is linked' do
       before do
-        allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return([])
         allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links)
         expect(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:new).with(project, remote_uri: URI.parse(default_endpoint)).and_call_original
       end
@@ -35,12 +34,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do
         subject.execute
       end
 
-      it 'links existent lfs objects to the project' do
-        expect_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute)
-
-        subject.execute
-      end
-
       it 'retrieves the download links of non existent objects' do
         expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(all_oids)
 
@@ -48,32 +41,6 @@ describe Projects::LfsPointers::LfsObjectDownloadListService do
       end
     end
 
-    context 'when some lfs objects are linked' do
-      before do
-        allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(existing_lfs_objects.keys)
-        allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).and_return(oid_download_links)
-      end
-
-      it 'retrieves the download links of non existent objects' do
-        expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with(oids)
-
-        subject.execute
-      end
-    end
-
-    context 'when all lfs objects are linked' do
-      before do
-        allow_any_instance_of(Projects::LfsPointers::LfsLinkService).to receive(:execute).and_return(all_oids.keys)
-        allow_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute)
-      end
-
-      it 'retrieves no download links' do
-        expect_any_instance_of(Projects::LfsPointers::LfsDownloadLinkListService).to receive(:execute).with({}).and_call_original
-
-        expect(subject.execute).to be_empty
-      end
-    end
-
     context 'when lfsconfig file exists' do
       before do
         allow(project.repository).to receive(:lfsconfig_for).and_return("[lfs]\n\turl = #{lfs_endpoint}\n")
diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb
index 63ebbcb93f9..3a306f80b3c 100644
--- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb
+++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb
@@ -7,6 +7,7 @@ RSpec.shared_context 'GroupPolicy context' do
   let_it_be(:maintainer) { create(:user) }
   let_it_be(:owner) { create(:user) }
   let_it_be(:admin) { create(:admin) }
+  let_it_be(:non_group_member) { create(:user) }
   let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only) }
 
   let(:guest_permissions) do
diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
index 73087befad2..662c64647d6 100644
--- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
@@ -69,13 +69,39 @@ RSpec.shared_examples 'handle uploads' do
   end
 
   describe "GET #show" do
+    let(:filename) { "rails_sample.jpg" }
+
+    let(:upload_service) do
+      UploadService.new(model, jpg, uploader_class).execute
+    end
+
     let(:show_upload) do
-      get :show, params: params.merge(secret: secret, filename: "rails_sample.jpg")
+      get :show, params: params.merge(secret: secret, filename: filename)
     end
 
     before do
       allow(FileUploader).to receive(:generate_secret).and_return(secret)
-      UploadService.new(model, jpg, uploader_class).execute
+      upload_service
+    end
+
+    context 'when the secret is invalid' do
+      let(:secret) { "../../../../../../../../" }
+      let(:filename) { "Gemfile.lock" }
+      let(:upload_service) { nil }
+
+      it 'responds with status 404' do
+        show_upload
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      it 'is a working exploit without the validation' do
+        allow_any_instance_of(FileUploader).to receive(:secret) { secret }
+
+        show_upload
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
     end
 
     context 'when accessing a specific upload via different model' do
diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb
index 9f053f03b0e..474515b537c 100644
--- a/spec/uploaders/file_mover_spec.rb
+++ b/spec/uploaders/file_mover_spec.rb
@@ -7,7 +7,7 @@ describe FileMover do
 
   let(:user) { create(:user) }
   let(:filename) { 'banana_sample.gif' }
-  let(:secret) { 'secret55' }
+  let(:secret) { SecureRandom.hex }
   let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", secret, filename) }
 
   let(:temp_description) do
@@ -47,8 +47,8 @@ describe FileMover do
           subject
 
           expect(snippet.reload.description)
-            .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
-                   "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ")
+            .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) ")
         end
 
         it 'updates existing upload record' do
@@ -75,8 +75,8 @@ describe FileMover do
           subject
 
           expect(snippet.reload.description)
-            .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
-                   "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ")
+            .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) ")
         end
 
         it 'does not change the upload record' do
@@ -101,8 +101,8 @@ describe FileMover do
           subject
 
           expect(snippet.reload.description)
-            .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
-                   "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ")
+            .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) ")
         end
 
         it 'creates new target upload record an delete the old upload' do
@@ -121,8 +121,8 @@ describe FileMover do
           subject
 
           expect(snippet.reload.description)
-            .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
-                   "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ")
+            .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) ")
         end
 
         it 'does not change the upload record' do
@@ -138,12 +138,8 @@ describe FileMover do
       let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", '..', 'another_subdir_of_temp') }
 
       it 'does not trigger move if path is outside designated directory' do
-        stub_file_mover("uploads/-/system/user/#{user.id}/another_subdir_of_temp")
         expect(FileUtils).not_to receive(:move)
-
-        subject
-
-        expect(snippet.reload.description).to eq(temp_description)
+        expect { subject }.to raise_error(FileUploader::InvalidSecret)
       end
     end
 
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index beb7aa3cf2c..efdbea85d4a 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -6,7 +6,8 @@ describe FileUploader do
   let(:group) { create(:group, name: 'awesome') }
   let(:project) { create(:project, :legacy_storage, namespace: group, name: 'project') }
   let(:uploader) { described_class.new(project, :avatar) }
-  let(:upload) { double(model: project, path: 'secret/foo.jpg') }
+  let(:upload) { double(model: project, path: "#{secret}/foo.jpg") }
+  let(:secret) { "55dc16aa0edd05693fd98b5051e83321" } # this would be nicer as SecureRandom.hex, but the shared_examples breaks
 
   subject { uploader }
 
@@ -14,7 +15,7 @@ describe FileUploader do
     include_examples 'builds correct paths',
                      store_dir: %r{awesome/project/\h+},
                      upload_path: %r{\h+/<filename>},
-                     absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg}
+                     absolute_path: %r{#{described_class.root}/awesome/project/55dc16aa0edd05693fd98b5051e83321/foo.jpg}
   end
 
   context 'legacy storage' do
@@ -51,11 +52,11 @@ describe FileUploader do
   end
 
   describe 'initialize' do
-    let(:uploader) { described_class.new(double, secret: 'secret') }
+    let(:uploader) { described_class.new(double, secret: secret) }
 
     it 'accepts a secret parameter' do
       expect(described_class).not_to receive(:generate_secret)
-      expect(uploader.secret).to eq('secret')
+      expect(uploader.secret).to eq(secret)
     end
   end
 
@@ -154,8 +155,39 @@ describe FileUploader do
 
   describe '#secret' do
     it 'generates a secret if none is provided' do
-      expect(described_class).to receive(:generate_secret).and_return('secret')
-      expect(uploader.secret).to eq('secret')
+      expect(described_class).to receive(:generate_secret).and_return(secret)
+      expect(uploader.secret).to eq(secret)
+      expect(uploader.secret.size).to eq(32)
+    end
+
+    context "validation" do
+      before do
+        uploader.instance_variable_set(:@secret, secret)
+      end
+
+      context "32-byte hexadecimal" do
+        let(:secret) { SecureRandom.hex }
+
+        it "returns the secret" do
+          expect(uploader.secret).to eq(secret)
+        end
+      end
+
+      context "10-byte hexadecimal" do
+        let(:secret) { SecureRandom.hex(10) }
+
+        it "returns the secret" do
+          expect(uploader.secret).to eq(secret)
+        end
+      end
+
+      context "invalid secret supplied" do
+        let(:secret) { "%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2Fgrafana%2Fconf%2F" }
+
+        it "raises an exception" do
+          expect { uploader.secret }.to raise_error(described_class::InvalidSecret)
+        end
+      end
     end
   end
 
diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb
index ec652af222d..c211ec3607c 100644
--- a/spec/uploaders/personal_file_uploader_spec.rb
+++ b/spec/uploaders/personal_file_uploader_spec.rb
@@ -6,12 +6,13 @@ describe PersonalFileUploader do
   let(:model) { create(:personal_snippet) }
   let(:uploader) { described_class.new(model) }
   let(:upload) { create(:upload, :personal_snippet_upload) }
+  let(:secret) { SecureRandom.hex }
 
   subject { uploader }
 
   shared_examples '#base_dir' do
     before do
-      subject.instance_variable_set(:@secret, 'secret')
+      subject.instance_variable_set(:@secret, secret)
     end
 
     it 'is prefixed with uploads/-/system' do
@@ -23,12 +24,12 @@ describe PersonalFileUploader do
 
   shared_examples '#to_h' do
     before do
-      subject.instance_variable_set(:@secret, 'secret')
+      subject.instance_variable_set(:@secret, secret)
     end
 
     it 'is correct' do
       allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name'))
-      expected_url = "/uploads/-/system/personal_snippet/#{model.id}/secret/file_name"
+      expected_url = "/uploads/-/system/personal_snippet/#{model.id}/#{secret}/file_name"
 
       expect(uploader.to_h).to eq(
         alt: 'file_name',
diff --git a/spec/validators/addressable_url_validator_spec.rb b/spec/validators/addressable_url_validator_spec.rb
index e8a44f7a12a..46b1bebb074 100644
--- a/spec/validators/addressable_url_validator_spec.rb
+++ b/spec/validators/addressable_url_validator_spec.rb
@@ -5,6 +5,9 @@ require 'spec_helper'
 describe AddressableUrlValidator do
   let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
 
+  let(:validator) { described_class.new(validator_options.reverse_merge(attributes: [:link_url])) }
+  let(:validator_options) { {} }
+
   subject { validator.validate(badge) }
 
   include_examples 'url validator examples', described_class::DEFAULT_OPTIONS[:schemes]
@@ -114,6 +117,19 @@ describe AddressableUrlValidator do
     end
   end
 
+  context 'when blocked_message is set' do
+    let(:message) { 'is not allowed due to: %{exception_message}' }
+    let(:validator_options) { { blocked_message: message } }
+
+    it 'blocks url with provided error message' do
+      badge.link_url = 'javascript:alert(window.opener.document.location)'
+
+      subject
+
+      expect(badge.errors.first[1]).to eq 'is not allowed due to: Only allowed schemes are http, https'
+    end
+  end
+
   context 'when allow_nil is set to true' do
     let(:validator) { described_class.new(attributes: [:link_url], allow_nil: true) }