Make Warden set_user hook validate user ip uniquness

 + rename shared context

6 files changed, 28 insertions, 23 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0d3ee4f4c1f..1c66c530cd2 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -41,7 +41,7 @@ class ApplicationController < ActionController::Base
   end
 
   rescue_from Gitlab::Auth::TooManyIps do |e|
-    head :forbidden, retry_after: UniqueIpsLimiter.config.unique_ips_limit_time_window
+    head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window
   end
 
   def redirect_back_or_default(default: root_path, options: {})
diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb
new file mode 100644
index 00000000000..3d83fb92d56
--- /dev/null
+++ b/config/initializers/warden.rb
@@ -0,0 +1,5 @@
+Rails.application.configure do |config|
+  Warden::Manager.after_set_user do |user, auth, opts|
+    Gitlab::Auth::UniqueIpsLimiter.limit_user!(user)
+  end
+end
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index fe11bdf4a78..a06c29dd91a 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -30,11 +30,11 @@ describe SessionsController do
           expect(SecurityEvent.last.details[:with]).to eq('standard')
         end
 
-        include_examples 'user login operation with unique ip limit' do
-          def operation
+        include_examples 'user login request with unique ip limit', 302 do
+          def request
             post(:create, user: { login: user.username, password: user.password })
-
             expect(subject.current_user).to eq user
+            subject.sign_out user
           end
         end
       end
diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
index 3214786d172..94dcddcc30c 100644
--- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
+++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Auth::UniqueIpsLimiter, :redis, lib: true do
-  include_context 'enable unique ips sign in limit'
+  include_context 'unique ips sign in limit'
   let(:user) { create(:user) }
 
   describe '#count_unique_ips' do
diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb
index 634b2261c5e..2974875510a 100644
--- a/spec/requests/api/doorkeeper_access_spec.rb
+++ b/spec/requests/api/doorkeeper_access_spec.rb
@@ -4,12 +4,12 @@ describe API::API, api: true do
   include ApiHelpers
 
   let!(:user) { create(:user) }
-  let!(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) }
-  let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: 'api' }
+  let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) }
+  let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: "api" }
 
-  describe 'when unauthenticated' do
-    it 'returns authentication success' do
-      get api('/user'), access_token: token.token
+  describe "unauthenticated" do
+    it "returns authentication success" do
+      get api("/user"), access_token: token.token
       expect(response).to have_http_status(200)
     end
 
@@ -20,16 +20,16 @@ describe API::API, api: true do
     end
   end
 
-  describe 'when token invalid' do
-    it 'returns authentication error' do
-      get api('/user'), access_token: '123a'
+  describe "when token invalid" do
+    it "returns authentication error" do
+      get api("/user"), access_token: "123a"
       expect(response).to have_http_status(401)
     end
   end
 
-  describe 'authorization by private token' do
-    it 'returns authentication success' do
-      get api('/user', user)
+  describe "authorization by private token" do
+    it "returns authentication success" do
+      get api("/user", user)
       expect(response).to have_http_status(200)
     end
 
diff --git a/spec/support/unique_ip_check_shared_examples.rb b/spec/support/unique_ip_check_shared_examples.rb
index 024fb132778..772e6722fc1 100644
--- a/spec/support/unique_ip_check_shared_examples.rb
+++ b/spec/support/unique_ip_check_shared_examples.rb
@@ -1,4 +1,4 @@
-shared_context 'enable unique ips sign in limit' do
+shared_context 'unique ips sign in limit' do
   include StubENV
   before(:each) do
     Gitlab::Redis.with(&:flushall)
@@ -19,7 +19,7 @@ shared_context 'enable unique ips sign in limit' do
 end
 
 shared_examples 'user login operation with unique ip limit' do
-  include_context 'enable unique ips sign in limit' do
+  include_context 'unique ips sign in limit' do
     before { current_application_settings.update!(unique_ips_limit_per_user: 1) }
 
     it 'allows user authenticating from the same ip' do
@@ -38,23 +38,23 @@ shared_examples 'user login operation with unique ip limit' do
   end
 end
 
-shared_examples 'user login request with unique ip limit' do
-  include_context 'enable unique ips sign in limit' do
+shared_examples 'user login request with unique ip limit' do |success_status = 200|
+  include_context 'unique ips sign in limit' do
     before { current_application_settings.update!(unique_ips_limit_per_user: 1) }
 
     it 'allows user authenticating from the same ip' do
       change_ip('ip')
       request
-      expect(response).to have_http_status(200)
+      expect(response).to have_http_status(success_status)
 
       request
-      expect(response).to have_http_status(200)
+      expect(response).to have_http_status(success_status)
     end
 
     it 'blocks user authenticating from two distinct ips' do
       change_ip('ip')
       request
-      expect(response).to have_http_status(200)
+      expect(response).to have_http_status(success_status)
 
       change_ip('ip2')
       request