Add latest changes from gitlab-org/gitlab@master

14 files changed, 166 insertions, 19 deletions

diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb
index d013bd6d427..74ffcc3aeef 100644
--- a/spec/controllers/import/bitbucket_controller_spec.rb
+++ b/spec/controllers/import/bitbucket_controller_spec.rb
@@ -136,6 +136,8 @@ describe Import::BitbucketController do
       expect(response).to have_gitlab_http_status(422)
     end
 
+    it_behaves_like 'project import rate limiter'
+
     context "when the repository owner is the Bitbucket user" do
       context "when the Bitbucket user and GitLab user's usernames match" do
         it "takes the current user's namespace" do
diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb
index f30eace7d30..a84f8368198 100644
--- a/spec/controllers/import/bitbucket_server_controller_spec.rb
+++ b/spec/controllers/import/bitbucket_server_controller_spec.rb
@@ -102,6 +102,8 @@ describe Import::BitbucketServerController do
 
       expect(response).to have_gitlab_http_status(422)
     end
+
+    it_behaves_like 'project import rate limiter'
   end
 
   describe 'POST configure' do
diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb
index f7c813576aa..9a647b8caae 100644
--- a/spec/controllers/import/fogbugz_controller_spec.rb
+++ b/spec/controllers/import/fogbugz_controller_spec.rb
@@ -75,4 +75,8 @@ describe Import::FogbugzController do
       expect(assigns(:repos)).to eq([])
     end
   end
+
+  describe 'POST create' do
+    it_behaves_like 'project import rate limiter'
+  end
 end
diff --git a/spec/controllers/import/gitea_controller_spec.rb b/spec/controllers/import/gitea_controller_spec.rb
index b7bdfcc3dc6..730e3f98c98 100644
--- a/spec/controllers/import/gitea_controller_spec.rb
+++ b/spec/controllers/import/gitea_controller_spec.rb
@@ -41,6 +41,8 @@ describe Import::GiteaController do
         assign_host_url
       end
     end
+
+    it_behaves_like 'project import rate limiter'
   end
 
   describe "GET realtime_changes" do
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb
index 5675798ac33..54fbe624cb7 100644
--- a/spec/controllers/import/github_controller_spec.rb
+++ b/spec/controllers/import/github_controller_spec.rb
@@ -71,6 +71,8 @@ describe Import::GithubController do
 
   describe "POST create" do
     it_behaves_like 'a GitHub-ish import controller: POST create'
+
+    it_behaves_like 'project import rate limiter'
   end
 
   describe "GET realtime_changes" do
diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb
index 6a3713a1212..495ea62456c 100644
--- a/spec/controllers/import/gitlab_controller_spec.rb
+++ b/spec/controllers/import/gitlab_controller_spec.rb
@@ -282,6 +282,8 @@ describe Import::GitlabController do
           expect(response).to have_gitlab_http_status(422)
         end
       end
+
+      it_behaves_like 'project import rate limiter'
     end
   end
 end
diff --git a/spec/controllers/import/gitlab_projects_controller_spec.rb b/spec/controllers/import/gitlab_projects_controller_spec.rb
index a3f6d8dcea2..285291c53fa 100644
--- a/spec/controllers/import/gitlab_projects_controller_spec.rb
+++ b/spec/controllers/import/gitlab_projects_controller_spec.rb
@@ -36,5 +36,7 @@ describe Import::GitlabProjectsController do
         expect(response).to have_gitlab_http_status(302)
       end
     end
+
+    it_behaves_like 'project import rate limiter'
   end
 end
diff --git a/spec/controllers/import/google_code_controller_spec.rb b/spec/controllers/import/google_code_controller_spec.rb
index 17be91c0bbb..3773f691ed0 100644
--- a/spec/controllers/import/google_code_controller_spec.rb
+++ b/spec/controllers/import/google_code_controller_spec.rb
@@ -58,4 +58,8 @@ describe Import::GoogleCodeController do
       expect(assigns(:incompatible_repos)).to eq([@repo])
     end
   end
+
+  describe "POST create" do
+    it_behaves_like 'project import rate limiter'
+  end
 end
diff --git a/spec/controllers/import/phabricator_controller_spec.rb b/spec/controllers/import/phabricator_controller_spec.rb
index a127e3cda3a..62a719cfb5b 100644
--- a/spec/controllers/import/phabricator_controller_spec.rb
+++ b/spec/controllers/import/phabricator_controller_spec.rb
@@ -88,5 +88,7 @@ describe Import::PhabricatorController do
         expect { post_create }.not_to change { current_user.namespace.projects.reload.size }
       end
     end
+
+    it_behaves_like 'project import rate limiter'
   end
 end
diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb
index 65652468d93..7c61d034ac9 100644
--- a/spec/initializers/lograge_spec.rb
+++ b/spec/initializers/lograge_spec.rb
@@ -94,6 +94,11 @@ describe 'lograge', type: :request do
     let(:logger) do
       Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
     end
+    let(:log_data) { JSON.parse(log_output.string) }
+
+    before do
+      Lograge.logger = logger
+    end
 
     describe 'with an exception' do
       let(:exception) { RuntimeError.new('bad request') }
@@ -102,18 +107,29 @@ describe 'lograge', type: :request do
       before do
         allow(exception).to receive(:backtrace).and_return(backtrace)
         event.payload[:exception_object] = exception
-        Lograge.logger = logger
       end
 
       it 'adds exception data to log' do
         subscriber.process_action(event)
 
-        log_data = JSON.parse(log_output.string)
-
         expect(log_data['exception.class']).to eq('RuntimeError')
         expect(log_data['exception.message']).to eq('bad request')
         expect(log_data['exception.backtrace']).to eq(Gitlab::BacktraceCleaner.clean_backtrace(backtrace))
       end
     end
+
+    describe 'with etag_route' do
+      let(:etag_route) { 'etag route' }
+
+      before do
+        event.payload[:etag_route] = etag_route
+      end
+
+      it 'adds etag_route to log' do
+        subscriber.process_action(event)
+
+        expect(log_data['etag_route']).to eq(etag_route)
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/etag_caching/middleware_spec.rb b/spec/lib/gitlab/etag_caching/middleware_spec.rb
index 24df67b3058..5e9df555241 100644
--- a/spec/lib/gitlab/etag_caching/middleware_spec.rb
+++ b/spec/lib/gitlab/etag_caching/middleware_spec.rb
@@ -8,6 +8,7 @@ describe Gitlab::EtagCaching::Middleware do
   let(:app_status_code) { 200 }
   let(:if_none_match) { nil }
   let(:enabled_path) { '/gitlab-org/gitlab-foss/noteable/issue/1/notes' }
+  let(:endpoint) { 'issue_notes' }
 
   context 'when ETag caching is not enabled for current route' do
     let(:path) { '/gitlab-org/gitlab-foss/tree/master/noteable/issue/1/notes' }
@@ -50,9 +51,9 @@ describe Gitlab::EtagCaching::Middleware do
 
       it 'tracks "etag_caching_key_not_found" event' do
         expect(Gitlab::Metrics).to receive(:add_event)
-          .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
+          .with(:etag_caching_middleware_used, endpoint: endpoint)
         expect(Gitlab::Metrics).to receive(:add_event)
-          .with(:etag_caching_key_not_found, endpoint: 'issue_notes')
+          .with(:etag_caching_key_not_found, endpoint: endpoint)
 
         middleware.call(build_request(path, if_none_match))
       end
@@ -74,6 +75,37 @@ describe Gitlab::EtagCaching::Middleware do
     end
   end
 
+  shared_examples 'sends a process_action.action_controller notification' do |status_code|
+    let(:expected_items) do
+      {
+        etag_route: endpoint,
+        params:     {},
+        format:     :html,
+        method:     'GET',
+        path:       enabled_path,
+        status:     status_code
+      }
+    end
+
+    it 'sends the expected payload' do
+      payload = payload_for('process_action.action_controller') do
+        middleware.call(build_request(path, if_none_match))
+      end
+
+      expect(payload).to include(expected_items)
+
+      expect(payload[:headers].env['HTTP_IF_NONE_MATCH']).to eq('W/"123"')
+    end
+
+    it 'log subscriber processes action' do
+      expect_any_instance_of(ActionController::LogSubscriber).to receive(:process_action)
+        .with(instance_of(ActiveSupport::Notifications::Event))
+        .and_call_original
+
+      middleware.call(build_request(path, if_none_match))
+    end
+  end
+
   context 'when If-None-Match header matches ETag in store' do
     let(:path) { enabled_path }
     let(:if_none_match) { 'W/"123"' }
@@ -94,6 +126,8 @@ describe Gitlab::EtagCaching::Middleware do
       expect(status).to eq 304
     end
 
+    it_behaves_like 'sends a process_action.action_controller notification', 304
+
     it 'returns empty body' do
       _, _, body = middleware.call(build_request(path, if_none_match))
 
@@ -102,9 +136,9 @@ describe Gitlab::EtagCaching::Middleware do
 
     it 'tracks "etag_caching_cache_hit" event' do
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
+        .with(:etag_caching_middleware_used, endpoint: endpoint)
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_cache_hit, endpoint: 'issue_notes')
+        .with(:etag_caching_cache_hit, endpoint: endpoint)
 
       middleware.call(build_request(path, if_none_match))
     end
@@ -120,6 +154,8 @@ describe Gitlab::EtagCaching::Middleware do
 
         expect(status).to eq 429
       end
+
+      it_behaves_like 'sends a process_action.action_controller notification', 429
     end
   end
 
@@ -141,9 +177,9 @@ describe Gitlab::EtagCaching::Middleware do
       mock_app_response
 
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
+        .with(:etag_caching_middleware_used, endpoint: endpoint)
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_resource_changed, endpoint: 'issue_notes')
+        .with(:etag_caching_resource_changed, endpoint: endpoint)
 
       middleware.call(build_request(path, if_none_match))
     end
@@ -159,9 +195,9 @@ describe Gitlab::EtagCaching::Middleware do
 
     it 'tracks "etag_caching_header_missing" event' do
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
+        .with(:etag_caching_middleware_used, endpoint: endpoint)
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_header_missing, endpoint: 'issue_notes')
+        .with(:etag_caching_header_missing, endpoint: endpoint)
 
       middleware.call(build_request(path, if_none_match))
     end
@@ -197,6 +233,21 @@ describe Gitlab::EtagCaching::Middleware do
   end
 
   def build_request(path, if_none_match)
-    { 'PATH_INFO' => path, 'HTTP_IF_NONE_MATCH' => if_none_match }
+    { 'PATH_INFO' => path,
+      'HTTP_IF_NONE_MATCH' => if_none_match,
+      'rack.input' => '',
+      'REQUEST_METHOD' => 'GET' }
+  end
+
+  def payload_for(event)
+    payload = nil
+    subscription = ActiveSupport::Notifications.subscribe event do |_, _, _, _, extra_payload|
+      payload = extra_payload
+    end
+
+    yield
+
+    ActiveSupport::Notifications.unsubscribe(subscription)
+    payload
   end
 end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 4438d3aab82..84e1f95828a 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -77,6 +77,14 @@ describe API::Users do
         expect(json_response.first.keys).not_to include 'highest_role'
       end
 
+      it "does not return the current or last sign-in ip addresses" do
+        get api("/users"), params: { username: user.username }
+
+        expect(response).to match_response_schema('public_api/v4/user/basics')
+        expect(json_response.first.keys).not_to include 'current_sign_in_ip'
+        expect(json_response.first.keys).not_to include 'last_sign_in_ip'
+      end
+
       context "when public level is restricted" do
         before do
           stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
@@ -314,6 +322,14 @@ describe API::Users do
       expect(json_response.keys).not_to include 'highest_role'
     end
 
+    it "does not return the user's sign in IPs" do
+      get api("/users/#{user.id}", user)
+
+      expect(response).to match_response_schema('public_api/v4/user/basic')
+      expect(json_response.keys).not_to include 'current_sign_in_ip'
+      expect(json_response.keys).not_to include 'last_sign_in_ip'
+    end
+
     context 'when authenticated as admin' do
       it 'includes the `is_admin` field' do
         get api("/users/#{user.id}", admin)
@@ -328,12 +344,34 @@ describe API::Users do
         expect(response).to match_response_schema('public_api/v4/user/admin')
         expect(json_response.keys).to include 'created_at'
       end
+
       it 'includes the `highest_role` field' do
         get api("/users/#{user.id}", admin)
 
         expect(response).to match_response_schema('public_api/v4/user/admin')
         expect(json_response['highest_role']).to be(0)
       end
+
+      context 'when user has not logged in' do
+        it 'does not include the sign in IPs' do
+          get api("/users/#{user.id}", admin)
+
+          expect(response).to match_response_schema('public_api/v4/user/admin')
+          expect(json_response).to include('current_sign_in_ip' => nil, 'last_sign_in_ip' => nil)
+        end
+      end
+
+      context 'when user has logged in' do
+        let_it_be(:signed_in_user) { create(:user, :with_sign_ins) }
+
+        it 'includes the sign in IPs' do
+          get api("/users/#{signed_in_user.id}", admin)
+
+          expect(response).to match_response_schema('public_api/v4/user/admin')
+          expect(json_response['current_sign_in_ip']).to eq('127.0.0.1')
+          expect(json_response['last_sign_in_ip']).to eq('127.0.0.1')
+        end
+      end
     end
 
     context 'for an anonymous user' do
diff --git a/spec/support/controllers/project_import_rate_limiter_shared_examples.rb b/spec/support/controllers/project_import_rate_limiter_shared_examples.rb
new file mode 100644
index 00000000000..336f801f923
--- /dev/null
+++ b/spec/support/controllers/project_import_rate_limiter_shared_examples.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+shared_examples 'project import rate limiter' do
+  let(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+  end
+
+  context 'when limit exceeds' do
+    before do
+      allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true)
+    end
+
+    it 'notifies and redirects user' do
+      post :create, params: {}
+
+      expect(flash[:alert]).to eq('This endpoint has been requested too many times. Try again later.')
+      expect(response).to have_gitlab_http_status(302)
+    end
+  end
+end
diff --git a/spec/support/helpers/smime_helper.rb b/spec/support/helpers/smime_helper.rb
index 3ad19cd3da0..96da3d81708 100644
--- a/spec/support/helpers/smime_helper.rb
+++ b/spec/support/helpers/smime_helper.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 module SmimeHelper
-  include OpenSSL
-
   INFINITE_EXPIRY = 1000.years
   SHORT_EXPIRY = 30.minutes
 
@@ -20,12 +18,12 @@ module SmimeHelper
     public_key = key.public_key
 
     subject = if certificate_authority
-                X509::Name.parse("/CN=EU")
+                OpenSSL::X509::Name.parse("/CN=EU")
               else
-                X509::Name.parse("/CN=#{email_address}")
+                OpenSSL::X509::Name.parse("/CN=#{email_address}")
               end
 
-    cert = X509::Certificate.new
+    cert = OpenSSL::X509::Certificate.new
     cert.subject = subject
 
     cert.issuer = signed_by&.fetch(:cert, nil)&.subject || subject
@@ -36,7 +34,7 @@ module SmimeHelper
     cert.serial = 0x0
     cert.version = 2
 
-    extension_factory = X509::ExtensionFactory.new
+    extension_factory = OpenSSL::X509::ExtensionFactory.new
     if certificate_authority
       extension_factory.subject_certificate = cert
       extension_factory.issuer_certificate = cert
@@ -50,7 +48,7 @@ module SmimeHelper
       cert.add_extension(extension_factory.create_extension('extendedKeyUsage', 'clientAuth,emailProtection', false))
     end
 
-    cert.sign(signed_by&.fetch(:key, nil) || key, Digest::SHA256.new)
+    cert.sign(signed_by&.fetch(:key, nil) || key, OpenSSL::Digest::SHA256.new)
 
     { key: key, cert: cert }
   end