diff options
author | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-02-21 22:17:23 +0100 |
---|---|---|
committer | Pawel Chojnacki <pawel@chojnacki.ws> | 2017-03-06 15:41:25 +0100 |
commit | 2ff139ddee49dca7109b9f547e54c6e472c7195b (patch) | |
tree | 101b2d8553f311dc4eab9ca54c374e68ec48b3a0 | |
parent | 0ef8a643489ad1a3da4431155326f0a6e206d870 (diff) | |
download | gitlab-ce-2ff139ddee49dca7109b9f547e54c6e472c7195b.tar.gz |
Make Warden set_user hook validate user ip uniquness
+ rename shared context
-rw-r--r-- | app/controllers/application_controller.rb | 2 | ||||
-rw-r--r-- | config/initializers/warden.rb | 5 | ||||
-rw-r--r-- | spec/controllers/sessions_controller_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/auth/unique_ips_limiter_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/doorkeeper_access_spec.rb | 22 | ||||
-rw-r--r-- | spec/support/unique_ip_check_shared_examples.rb | 14 |
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 |