Add latest changes from gitlab-org/gitlab@13-12-stable-ee

6 files changed, 86 insertions, 34 deletions

diff --git a/spec/lib/api/entities/release_spec.rb b/spec/lib/api/entities/release_spec.rb
index 4f40830a15c..aa2c5126bb9 100644
--- a/spec/lib/api/entities/release_spec.rb
+++ b/spec/lib/api/entities/release_spec.rb
@@ -8,7 +8,8 @@ RSpec.describe API::Entities::Release do
   let(:release) { create(:release, project: project) }
   let(:evidence) { release.evidences.first }
   let(:user) { create(:user) }
-  let(:entity) { described_class.new(release, current_user: user).as_json }
+  let(:entity) { described_class.new(release, current_user: user, include_html_description: include_html_description).as_json }
+  let(:include_html_description) { false }
 
   before do
     ::Releases::CreateEvidenceService.new(release).execute
@@ -58,10 +59,8 @@ RSpec.describe API::Entities::Release do
       expect(description_html).to be_nil
     end
 
-    context 'when remove_description_html_in_release_api feature flag is disabled' do
-      before do
-        stub_feature_flags(remove_description_html_in_release_api: false)
-      end
+    context 'when include_html_description option is true' do
+      let(:include_html_description) { true }
 
       it 'renders special references if current user has access' do
         project.add_reporter(user)
@@ -77,18 +76,5 @@ RSpec.describe API::Entities::Release do
         expect(description_html).not_to include(issue_title)
       end
     end
-
-    context 'when remove_description_html_in_release_api_override feature flag is enabled' do
-      before do
-        stub_feature_flags(remove_description_html_in_release_api_override: project)
-      end
-
-      it 'renders special references if current user has access' do
-        project.add_reporter(user)
-
-        expect(description_html).to include(issue_path)
-        expect(description_html).to include(issue_title)
-      end
-    end
   end
 end
diff --git a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb
index 41a6c06f9c9..19e52d2cf4a 100644
--- a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb
+++ b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb
@@ -20,9 +20,9 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
   end
 
   describe '.default_settings_hash' do
-    it 'returns defaults for all keys' do
-      settings = described_class.default_settings_hash
+    let(:settings) { described_class.default_settings_hash }
 
+    it 'returns defaults for all keys' do
       expect(settings['enabled']).to be_truthy
       expect(settings['report_only']).to be_falsey
 
@@ -35,6 +35,17 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
 
       expect(directives.has_key?('report_uri')).to be_truthy
       expect(directives['report_uri']).to be_nil
+      expect(directives['child_src']).to eq(directives['frame_src'])
+    end
+
+    context 'when in production' do
+      before do
+        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
+      end
+
+      it 'is disabled' do
+        expect(settings['enabled']).to be_falsey
+      end
     end
 
     context 'when GITLAB_CDN_HOST is set' do
@@ -43,10 +54,9 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do
       end
 
       it 'adds GITLAB_CDN_HOST to CSP' do
-        settings = described_class.default_settings_hash
         directives = settings['directives']
 
-        expect(directives['script_src']).to eq("'strict-dynamic' 'self' 'unsafe-inline' 'unsafe-eval' https://www.recaptcha.net https://apis.google.com https://example.com")
+        expect(directives['script_src']).to eq("'strict-dynamic' 'self' 'unsafe-inline' 'unsafe-eval' https://www.google.com/recaptcha/ https://www.recaptcha.net https://apis.google.com https://example.com")
         expect(directives['style_src']).to eq("'self' 'unsafe-inline' https://example.com")
       end
     end
diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb
index 8ac1f15d67e..0ca202aa7be 100644
--- a/spec/mailers/emails/profile_spec.rb
+++ b/spec/mailers/emails/profile_spec.rb
@@ -264,7 +264,7 @@ RSpec.describe Emails::Profile do
         include_examples 'valid use case'
 
         it_behaves_like 'has the correct subject', /Your SSH key has expired/
-        it_behaves_like 'has the correct body text', /Your SSH keys with the following fingerprints has expired/
+        it_behaves_like 'has the correct body text', /Your SSH keys with the following fingerprints have expired/
       end
 
       context 'when invalid' do
diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb
index 81ddcd7cf84..dad3e34404b 100644
--- a/spec/requests/api/releases_spec.rb
+++ b/spec/requests/api/releases_spec.rb
@@ -50,6 +50,12 @@ RSpec.describe API::Releases do
         expect(json_response.second['tag_name']).to eq(release_1.tag)
       end
 
+      it 'does not include description_html' do
+        get api("/projects/#{project.id}/releases", maintainer)
+
+        expect(json_response.map { |h| h['description_html'] }).to contain_exactly(nil, nil)
+      end
+
       RSpec.shared_examples 'release sorting' do |order_by|
         subject { get api(url, access_level), params: { sort: sort, order_by: order_by } }
 
@@ -107,6 +113,15 @@ RSpec.describe API::Releases do
         expect(json_response.second['commit_path']).to eq("/#{release_1.project.full_path}/-/commit/#{release_1.commit.id}")
         expect(json_response.second['tag_path']).to eq("/#{release_1.project.full_path}/-/tags/#{release_1.tag}")
       end
+
+      context 'when include_html_description option is true' do
+        it 'includes description_html field' do
+          get api("/projects/#{project.id}/releases", maintainer), params: { include_html_description: true }
+
+          expect(json_response.map { |h| h['description_html'] })
+            .to contain_exactly(instance_of(String), instance_of(String))
+        end
+      end
     end
 
     it 'returns an upcoming_release status for a future release' do
@@ -328,6 +343,12 @@ RSpec.describe API::Releases do
           .to match_array(release.sources.map(&:url))
       end
 
+      it 'does not include description_html' do
+        get api("/projects/#{project.id}/releases/v0.1", maintainer)
+
+        expect(json_response['description_html']).to eq(nil)
+      end
+
       context 'with evidence' do
         let!(:evidence) { create(:evidence, release: release) }
 
@@ -403,6 +424,14 @@ RSpec.describe API::Releases do
         end
       end
 
+      context 'when include_html_description option is true' do
+        it 'includes description_html field' do
+          get api("/projects/#{project.id}/releases/v0.1", maintainer), params: { include_html_description: true }
+
+          expect(json_response['description_html']).to be_instance_of(String)
+        end
+      end
+
       context 'when user is a guest' do
         it 'responds 403 Forbidden' do
           get api("/projects/#{project.id}/releases/v0.1", guest)
diff --git a/spec/services/spam/akismet_service_spec.rb b/spec/services/spam/akismet_service_spec.rb
index f75b0216b78..1cd049da592 100644
--- a/spec/services/spam/akismet_service_spec.rb
+++ b/spec/services/spam/akismet_service_spec.rb
@@ -4,12 +4,15 @@ require 'spec_helper'
 
 RSpec.describe Spam::AkismetService do
   let(:fake_akismet_client) { double(:akismet_client) }
+  let(:ip) { '1.2.3.4' }
+  let(:user_agent) { 'some user_agent' }
+  let(:referer) { 'some referer' }
 
   let_it_be(:text) { "Would you like to buy some tinned meat product?" }
   let_it_be(:spam_owner) { create(:user) }
 
   subject do
-    options = { ip_address: '1.2.3.4', user_agent: 'some user_agent', referrer: 'some referrer' }
+    options = { ip_address: ip, user_agent: user_agent, referer: referer }
     described_class.new(spam_owner.name, spam_owner.email, text, options)
   end
 
@@ -56,6 +59,21 @@ RSpec.describe Spam::AkismetService do
     it_behaves_like 'no activity if Akismet is not enabled', :spam?, :check
 
     context 'if Akismet is enabled' do
+      it 'correctly transforms options for the akismet client' do
+        expected_check_params = {
+          type: 'comment',
+          text: text,
+          created_at: anything,
+          author: spam_owner.name,
+          author_email: spam_owner.email,
+          # NOTE: The akismet_client needs the option to be named `:referrer`, not `:referer`
+          referrer: referer
+        }
+
+        expect(fake_akismet_client).to receive(:check).with(ip, user_agent, expected_check_params)
+        subject.spam?
+      end
+
       context 'the text is spam' do
         before do
           allow(fake_akismet_client).to receive(:check).and_return([true, false])
@@ -86,19 +104,31 @@ RSpec.describe Spam::AkismetService do
         end
       end
 
-      context 'if Akismet is not available' do
+      describe 'error handling' do
         before do
-          allow(fake_akismet_client).to receive(:check).and_raise(StandardError.new("oh noes!"))
+          allow(fake_akismet_client).to receive(:check).and_raise(error)
         end
 
-        specify do
-          expect(subject.spam?).to be_falsey
+        context 'StandardError other than ArgumentError is raised' do
+          let(:error) { Akismet::Error.new("Lovely spam! Wonderful spam!") }
+
+          specify do
+            expect(subject.spam?).to be_falsey
+          end
+
+          it 'logs an error' do
+            expect(Gitlab::AppLogger).to receive(:error).with(/Error during Akismet.*flagging as not spam.*Lovely spam/)
+
+            subject.spam?
+          end
         end
 
-        it 'logs an error' do
-          expect(Gitlab::AppLogger).to receive(:error).with(/skipping check/)
+        context 'ArgumentError is raised in dev' do
+          let(:error) { ArgumentError }
 
-          subject.spam?
+          it 'raises original error' do
+            expect { subject.spam? }.to raise_error(error)
+          end
         end
       end
     end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index c59daa6c919..bd9ba53c04c 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -286,9 +286,6 @@ RSpec.configure do |config|
       # As we're ready to change `master` usages to `main`, let's enable it
       stub_feature_flags(main_branch_over_master: false)
 
-      # Selectively disable by actor https://docs.gitlab.com/ee/development/feature_flags/#selectively-disable-by-actor
-      stub_feature_flags(remove_description_html_in_release_api_override: false)
-
       # Disable issue respositioning to avoid heavy load on database when importing big projects.
       # This is only turned on when app is handling heavy project imports.
       # Can be removed when we find a better way to deal with the problem.