Add latest changes from gitlab-org/gitlab@master

7 files changed, 114 insertions, 121 deletions

diff --git a/spec/lib/gitlab/application_rate_limiter_spec.rb b/spec/lib/gitlab/application_rate_limiter_spec.rb
index 0ce7ae8d3aa..c74bcf8d678 100644
--- a/spec/lib/gitlab/application_rate_limiter_spec.rb
+++ b/spec/lib/gitlab/application_rate_limiter_spec.rb
@@ -8,7 +8,7 @@ RSpec.describe Gitlab::ApplicationRateLimiter do
 
   subject { described_class }
 
-  describe '.throttled?' do
+  describe '.throttled?', :clean_gitlab_redis_rate_limiting do
     let(:rate_limits) do
       {
         test_action: {
@@ -56,136 +56,51 @@ RSpec.describe Gitlab::ApplicationRateLimiter do
       end
     end
 
-    context 'when rate_limiter_safe_increment is disabled' do
-      let(:redis) { double('redis') }
-      let(:key) { rate_limits.keys[0] }
+    shared_examples 'throttles based on key and scope' do
+      let(:start_time) { Time.current.beginning_of_hour }
 
-      before do
-        allow(Gitlab::Redis::RateLimiting).to receive(:with).and_yield(redis)
-
-        stub_feature_flags(rate_limiter_safe_increment: false)
-      end
-
-      shared_examples 'action rate limiter' do
-        it 'increases the throttle count and sets the expiration time' do
-          expect(redis).to receive(:incr).with(cache_key).and_return(1)
-          expect(redis).to receive(:expire).with(cache_key, 120)
-
-          expect(subject.throttled?(key, scope: scope)).to be_falsy
+      it 'returns true when threshold is exceeded' do
+        travel_to(start_time) do
+          expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
         end
 
-        it 'returns true if the key is throttled' do
-          expect(redis).to receive(:incr).with(cache_key).and_return(2)
-          expect(redis).not_to receive(:expire)
+        travel_to(start_time + 1.minute) do
+          expect(subject.throttled?(:test_action, scope: scope)).to eq(true)
 
-          expect(subject.throttled?(key, scope: scope)).to be_truthy
-        end
-
-        context 'when throttling is disabled' do
-          it 'returns false and does not set expiration time' do
-            expect(redis).not_to receive(:incr)
-            expect(redis).not_to receive(:expire)
-
-            expect(subject.throttled?(key, scope: scope, threshold: 0)).to be_falsy
-          end
+          # Assert that it does not affect other actions or scope
+          expect(subject.throttled?(:another_action, scope: scope)).to eq(false)
+          expect(subject.throttled?(:test_action, scope: [user])).to eq(false)
         end
       end
 
-      context 'when the key is an array of only ActiveRecord models' do
-        let(:scope) { [user, project] }
+      it 'returns false when interval has elapsed' do
+        travel_to(start_time) do
+          expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
 
-        let(:cache_key) do
-          "application_rate_limiter:test_action:user:#{user.id}:project:#{project.id}"
+          # another_action has a threshold of 3 so we simulate 2 requests
+          expect(subject.throttled?(:another_action, scope: scope)).to eq(false)
+          expect(subject.throttled?(:another_action, scope: scope)).to eq(false)
         end
 
-        it_behaves_like 'action rate limiter'
-
-        context 'when a scope attribute is nil' do
-          let(:scope) { [user, nil] }
-
-          let(:cache_key) do
-            "application_rate_limiter:test_action:user:#{user.id}"
-          end
-
-          it_behaves_like 'action rate limiter'
-        end
-      end
-
-      context 'when the key is a combination of ActiveRecord models and strings' do
-        let(:project) { create(:project, :public, :repository) }
-        let(:commit) { project.repository.commit }
-        let(:path) { 'app/controllers/groups_controller.rb' }
-        let(:scope) { [project, commit, path] }
-
-        let(:cache_key) do
-          "application_rate_limiter:test_action:project:#{project.id}:commit:#{commit.sha}:#{path}"
-        end
-
-        it_behaves_like 'action rate limiter'
-
-        context 'when a scope attribute is nil' do
-          let(:scope) { [project, commit, nil] }
-
-          let(:cache_key) do
-            "application_rate_limiter:test_action:project:#{project.id}:commit:#{commit.sha}"
-          end
+        travel_to(start_time + 2.minutes) do
+          expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
 
-          it_behaves_like 'action rate limiter'
+          # Assert that another_action has its own interval that hasn't elapsed
+          expect(subject.throttled?(:another_action, scope: scope)).to eq(true)
         end
       end
     end
 
-    context 'when rate_limiter_safe_increment is enabled', :clean_gitlab_redis_rate_limiting do
-      before do
-        stub_feature_flags(rate_limiter_safe_increment: true)
-      end
-
-      shared_examples 'throttles based on key and scope' do
-        let(:start_time) { Time.current.beginning_of_hour }
-
-        it 'returns true when threshold is exceeded' do
-          travel_to(start_time) do
-            expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
-          end
-
-          travel_to(start_time + 1.minute) do
-            expect(subject.throttled?(:test_action, scope: scope)).to eq(true)
-
-            # Assert that it does not affect other actions or scope
-            expect(subject.throttled?(:another_action, scope: scope)).to eq(false)
-            expect(subject.throttled?(:test_action, scope: [user])).to eq(false)
-          end
-        end
-
-        it 'returns false when interval has elapsed' do
-          travel_to(start_time) do
-            expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
-
-            # another_action has a threshold of 3 so we simulate 2 requests
-            expect(subject.throttled?(:another_action, scope: scope)).to eq(false)
-            expect(subject.throttled?(:another_action, scope: scope)).to eq(false)
-          end
-
-          travel_to(start_time + 2.minutes) do
-            expect(subject.throttled?(:test_action, scope: scope)).to eq(false)
-
-            # Assert that another_action has its own interval that hasn't elapsed
-            expect(subject.throttled?(:another_action, scope: scope)).to eq(true)
-          end
-        end
-      end
-
-      context 'when using ActiveRecord models as scope' do
-        let(:scope) { [user, project] }
+    context 'when using ActiveRecord models as scope' do
+      let(:scope) { [user, project] }
 
-        it_behaves_like 'throttles based on key and scope'
-      end
+      it_behaves_like 'throttles based on key and scope'
+    end
 
-      context 'when using ActiveRecord models and strings as scope' do
-        let(:scope) { [project, 'app/controllers/groups_controller.rb'] }
+    context 'when using ActiveRecord models and strings as scope' do
+      let(:scope) { [project, 'app/controllers/groups_controller.rb'] }
 
-        it_behaves_like 'throttles based on key and scope'
-      end
+      it_behaves_like 'throttles based on key and scope'
     end
   end
 
diff --git a/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb b/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb
index e746451b1b9..f9628849dbf 100644
--- a/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb
+++ b/spec/lib/gitlab/background_migration/populate_personal_snippet_statistics_spec.rb
@@ -111,11 +111,11 @@ RSpec.describe Gitlab::BackgroundMigration::PopulatePersonalSnippetStatistics do
       if with_repo
         allow(snippet).to receive(:disk_path).and_return(disk_path(snippet))
 
+        raw_repository(snippet).create_repository
+
         TestEnv.copy_repo(snippet,
                           bare_repo: TestEnv.factory_repo_path_bare,
                           refs: TestEnv::BRANCH_SHA)
-
-        raw_repository(snippet).create_repository
       end
     end
   end
diff --git a/spec/lib/gitlab/background_migration/populate_project_snippet_statistics_spec.rb b/spec/lib/gitlab/background_migration/populate_project_snippet_statistics_spec.rb
index 897f5e81372..7884e0d97c0 100644
--- a/spec/lib/gitlab/background_migration/populate_project_snippet_statistics_spec.rb
+++ b/spec/lib/gitlab/background_migration/populate_project_snippet_statistics_spec.rb
@@ -183,11 +183,11 @@ RSpec.describe Gitlab::BackgroundMigration::PopulateProjectSnippetStatistics do
       if with_repo
         allow(snippet).to receive(:disk_path).and_return(disk_path(snippet))
 
+        raw_repository(snippet).create_repository
+
         TestEnv.copy_repo(snippet,
                           bare_repo: TestEnv.factory_repo_path_bare,
                           refs: TestEnv::BRANCH_SHA)
-
-        raw_repository(snippet).create_repository
       end
     end
   end
diff --git a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb
index cdcc862c376..9d49db1f018 100644
--- a/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb
+++ b/spec/lib/gitlab/database/count/reltuples_count_strategy_spec.rb
@@ -38,7 +38,8 @@ RSpec.describe Gitlab::Database::Count::ReltuplesCountStrategy do
       it 'returns nil counts for inherited tables' do
         models.each { |model| expect(model).not_to receive(:count) }
 
-        expect(subject).to eq({ Namespace => 3 })
+        # 3 Namespaces as parents for each Project and 3 ProjectNamespaces(for each Project)
+        expect(subject).to eq({ Namespace => 6 })
       end
     end
 
diff --git a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb
index c2028f8c238..2f261aebf02 100644
--- a/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb
+++ b/spec/lib/gitlab/database/count/tablesample_count_strategy_spec.rb
@@ -47,7 +47,8 @@ RSpec.describe Gitlab::Database::Count::TablesampleCountStrategy do
         result = subject
         expect(result[Project]).to eq(3)
         expect(result[Group]).to eq(1)
-        expect(result[Namespace]).to eq(4)
+        # 1-Group, 3 namespaces for each project and 3 project namespaces for each project
+        expect(result[Namespace]).to eq(7)
       end
     end
 
diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb
index 55102554508..20d5972bd88 100644
--- a/spec/lib/gitlab/gpg/commit_spec.rb
+++ b/spec/lib/gitlab/gpg/commit_spec.rb
@@ -136,7 +136,7 @@ RSpec.describe Gitlab::Gpg::Commit do
           it 'returns a valid signature' do
             verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true)
             allow(GPGME::Crypto).to receive(:new).and_return(crypto)
-            allow(crypto).to receive(:verify).and_return(verified_signature)
+            allow(crypto).to receive(:verify).and_yield(verified_signature)
 
             signature = described_class.new(commit).signature
 
@@ -178,7 +178,7 @@ RSpec.describe Gitlab::Gpg::Commit do
             keyid = GpgHelpers::User1.fingerprint.last(16)
             verified_signature = double('verified-signature', fingerprint: keyid, valid?: true)
             allow(GPGME::Crypto).to receive(:new).and_return(crypto)
-            allow(crypto).to receive(:verify).and_return(verified_signature)
+            allow(crypto).to receive(:verify).and_yield(verified_signature)
 
             signature = described_class.new(commit).signature
 
@@ -194,6 +194,71 @@ RSpec.describe Gitlab::Gpg::Commit do
           end
         end
 
+        context 'commit with multiple signatures' do
+          let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first }
+
+          let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) }
+
+          let!(:gpg_key) do
+            create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+          end
+
+          let!(:crypto) { instance_double(GPGME::Crypto) }
+
+          before do
+            fake_signature = [
+              GpgHelpers::User1.signed_commit_signature,
+              GpgHelpers::User1.signed_commit_base_data
+            ]
+
+            allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+              .with(Gitlab::Git::Repository, commit_sha)
+              .and_return(fake_signature)
+          end
+
+          it 'returns an invalid signatures error' do
+            verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true)
+            allow(GPGME::Crypto).to receive(:new).and_return(crypto)
+            allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(verified_signature)
+
+            signature = described_class.new(commit).signature
+
+            expect(signature).to have_attributes(
+              commit_sha: commit_sha,
+              project: project,
+              gpg_key: gpg_key,
+              gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+              gpg_key_user_name: GpgHelpers::User1.names.first,
+              gpg_key_user_email: GpgHelpers::User1.emails.first,
+              verification_status: 'multiple_signatures'
+            )
+          end
+
+          context 'when feature flag is disabled' do
+            before do
+              stub_feature_flags(multiple_gpg_signatures: false)
+            end
+
+            it 'returns an valid signature' do
+              verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true)
+              allow(GPGME::Crypto).to receive(:new).and_return(crypto)
+              allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(verified_signature)
+
+              signature = described_class.new(commit).signature
+
+              expect(signature).to have_attributes(
+                commit_sha: commit_sha,
+                project: project,
+                gpg_key: gpg_key,
+                gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+                gpg_key_user_name: GpgHelpers::User1.names.first,
+                gpg_key_user_email: GpgHelpers::User1.emails.first,
+                verification_status: 'verified'
+              )
+            end
+          end
+        end
+
         context 'commit signed with a subkey' do
           let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User3.emails.first }
 
diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index 2f38ed58727..f0ba0f0459d 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -425,6 +425,9 @@ RSpec.describe Gitlab::PathRegex do
     it { is_expected.not_to match('gitlab.org/') }
     it { is_expected.not_to match('/gitlab.org') }
     it { is_expected.not_to match('gitlab git') }
+    it { is_expected.not_to match('gitlab?') }
+    it { is_expected.to match('gitlab.org-') }
+    it { is_expected.to match('gitlab.org_') }
   end
 
   describe '.project_path_format_regex' do
@@ -437,6 +440,14 @@ RSpec.describe Gitlab::PathRegex do
     it { is_expected.not_to match('?gitlab') }
     it { is_expected.not_to match('git lab') }
     it { is_expected.not_to match('gitlab.git') }
+    it { is_expected.not_to match('gitlab?') }
+    it { is_expected.not_to match('gitlab git') }
+    it { is_expected.to match('gitlab.org') }
+    it { is_expected.to match('gitlab.org-') }
+    it { is_expected.to match('gitlab.org_') }
+    it { is_expected.to match('gitlab.org.') }
+    it { is_expected.not_to match('gitlab.org/') }
+    it { is_expected.not_to match('/gitlab.org') }
   end
 
   context 'repository routes' do