From e2addae80ad3773e9a089f026575ee2f9cd4b848 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 12 Apr 2018 13:23:48 +0200 Subject: [PATCH] Fixed issue #1337 - "Signin detected from a new device" Spam. --- .../application_controller/handles_devices.rb | 1 + app/controllers/form_controller.rb | 1 + app/models/user_device.rb | 26 +++- .../user_device_controller_test.rb | 141 +++++++++++++++--- test/unit/user_device_test.rb | 14 ++ 5 files changed, 165 insertions(+), 18 deletions(-) diff --git a/app/controllers/application_controller/handles_devices.rb b/app/controllers/application_controller/handles_devices.rb index 718dae82e..abef7ebff 100644 --- a/app/controllers/application_controller/handles_devices.rb +++ b/app/controllers/application_controller/handles_devices.rb @@ -47,6 +47,7 @@ module ApplicationController::HandlesDevices raise Exceptions::UnprocessableEntity, 'Need fingerprint param!' end if params[:fingerprint] + UserDevice.fingerprint_validation(params[:fingerprint]) session[:user_device_fingerprint] = params[:fingerprint] end end diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index bea472d77..d0a535ddf 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -4,6 +4,7 @@ class FormController < ApplicationController skip_before_action :verify_csrf_token before_action :cors_preflight_check_execute after_action :set_access_control_headers_execute + skip_before_action :user_device_check def configuration return if !enabled? diff --git a/app/models/user_device.rb b/app/models/user_device.rb index 05ac72928..1ef721e48 100644 --- a/app/models/user_device.rb +++ b/app/models/user_device.rb @@ -5,6 +5,9 @@ class UserDevice < ApplicationModel store :location_details validates :name, presence: true + before_create :fingerprint_validation + before_update :fingerprint_validation + =begin store new device for user if device not already known @@ -34,7 +37,8 @@ store new device for user if device not already known # find device by fingerprint device_exists_by_fingerprint = false - if fingerprint + if fingerprint.present? + UserDevice.fingerprint_validation(fingerprint) user_devices = UserDevice.where( user_id: user_id, fingerprint: fingerprint, @@ -221,4 +225,24 @@ delete device devices of user def self.remove(user_id) UserDevice.where(user_id: user_id).destroy_all end + +=begin + +check fingerprint string + + UserDevice.fingerprint_validation(fingerprint) + +=end + + def self.fingerprint_validation(fingerprint) + return true if fingerprint.blank? + raise Exceptions::UnprocessableEntity, "fingerprint is #{fingerprint.length} chars but can only be 160 chars!" if fingerprint.length > 160 + true + end + + private + + def fingerprint_validation + UserDevice.fingerprint_validation(fingerprint) + end end diff --git a/test/integration/user_device_controller_test.rb b/test/integration/user_device_controller_test.rb index d3697b407..99407dba1 100644 --- a/test/integration/user_device_controller_test.rb +++ b/test/integration/user_device_controller_test.rb @@ -41,6 +41,9 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest ENV['TEST_REMOTE_IP'] = '5.9.62.170' # de ENV['HTTP_USER_AGENT'] = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0' + ENV['SWITCHED_FROM_USER_ID'] = nil + + UserDevice.destroy_all end test '01 - index with nobody' do @@ -173,6 +176,15 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest test '04 - login index with admin with fingerprint - II' do + UserDevice.create!( + user_id: @admin.id, + name: 'test 1', + location: 'some location', + user_agent: 'some user agent', + ip: '127.0.0.1', + fingerprint: 'fingerprintI', + ) + params = { fingerprint: 'my_finger_print_II', username: 'user-device-admin', password: 'adminpw' } post '/api/v1/signin', params: params.to_json, headers: @headers assert_response(201) @@ -180,13 +192,13 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(3, UserDevice.where(user_id: @admin.id).count) + assert_equal(2, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) assert_equal(result.class, Hash) assert_not(result['error']) assert(result['config']) - assert('my_finger_print_II', controller.session[:user_device_fingerprint]) + assert('my_finger_print_III', controller.session[:user_device_fingerprint]) get '/api/v1/users', params: params.to_json, headers: @headers assert_response(200) @@ -195,7 +207,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(3, UserDevice.where(user_id: @admin.id).count) + assert_equal(2, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) @@ -210,7 +222,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(3, UserDevice.where(user_id: @admin.id).count) + assert_equal(2, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) @@ -223,7 +235,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(4, UserDevice.where(user_id: @admin.id).count) + assert_equal(3, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(1, email_notification_count('user_device_new_location', @admin.email)) @@ -234,6 +246,16 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest test '05 - login index with admin with fingerprint - II' do + UserDevice.add( + ENV['HTTP_USER_AGENT'], + ENV['TEST_REMOTE_IP'], + @admin.id, + 'my_finger_print_II', + 'session', # session|basic_auth|token_auth|sso + ) + + assert_equal(1, UserDevice.where(user_id: @admin.id).count) + params = { fingerprint: 'my_finger_print_II', username: 'user-device-admin', password: 'adminpw' } post '/api/v1/signin', params: params.to_json, headers: @headers assert_response(201) @@ -241,7 +263,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(4, UserDevice.where(user_id: @admin.id).count) + assert_equal(1, UserDevice.where(user_id: @admin.id).count) assert_equal(0, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) assert_equal(result.class, Hash) @@ -252,9 +274,19 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest test '06 - login index with admin with basic auth' do - ENV['HTTP_USER_AGENT'] = 'curl 1.2.3' + ENV['HTTP_USER_AGENT'] = 'curl 1.0.0' + UserDevice.add( + ENV['HTTP_USER_AGENT'], + '127.0.0.1', + @admin.id, + '', + 'basic_auth', # session|basic_auth|token_auth|sso + ) + assert_equal(1, UserDevice.where(user_id: @admin.id).count) + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-admin', 'adminpw') + ENV['HTTP_USER_AGENT'] = 'curl 1.2.3' params = {} get '/api/v1/users', params: params.to_json, headers: @headers.merge('Authorization' => credentials) assert_response(200) @@ -262,7 +294,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(5, UserDevice.where(user_id: @admin.id).count) + assert_equal(2, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) assert_equal(result.class, Array) @@ -276,7 +308,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(5, UserDevice.where(user_id: @admin.id).count) + assert_equal(2, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) assert_equal(result.class, Array) @@ -294,7 +326,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(5, UserDevice.where(user_id: @admin.id).count) + assert_equal(2, UserDevice.where(user_id: @admin.id).count) assert_equal(1, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) assert_equal(result.class, Array) @@ -307,6 +339,17 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest test '07 - login index with admin with basic auth' do ENV['HTTP_USER_AGENT'] = 'curl 1.2.3' + + UserDevice.add( + ENV['HTTP_USER_AGENT'], + ENV['TEST_REMOTE_IP'], + @admin.id, + '', + 'basic_auth', # session|basic_auth|token_auth|sso + ) + + assert_equal(1, UserDevice.where(user_id: @admin.id).count) + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-admin', 'adminpw') params = {} @@ -316,7 +359,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(5, UserDevice.where(user_id: @admin.id).count) + assert_equal(1, UserDevice.where(user_id: @admin.id).count) assert_equal(0, email_notification_count('user_device_new', @admin.email)) assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) assert_equal(result.class, Array) @@ -348,11 +391,20 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest test '09 - login index with agent with basic auth' do + ENV['HTTP_USER_AGENT'] = 'curl 1.2.3' + + UserDevice.add( + ENV['HTTP_USER_AGENT'], + ENV['TEST_REMOTE_IP'], + @agent.id, + '', + 'basic_auth', # session|basic_auth|token_auth|sso + ) + assert_equal(1, UserDevice.where(user_id: @agent.id).count) assert_equal(0, email_notification_count('user_device_new', @agent.email)) assert_equal(0, email_notification_count('user_device_new_location', @agent.email)) - ENV['HTTP_USER_AGENT'] = 'curl 1.2.3' credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-agent', 'agentpw') params = {} @@ -371,7 +423,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest test '10 - login with switched_from_user_id' do - assert_equal(1, UserDevice.where(user_id: @agent.id).count) + assert_equal(0, UserDevice.where(user_id: @agent.id).count) assert_equal(0, email_notification_count('user_device_new', @agent.email)) assert_equal(0, email_notification_count('user_device_new_location', @agent.email)) @@ -384,7 +436,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(1, UserDevice.where(user_id: @agent.id).count) + assert_equal(0, UserDevice.where(user_id: @agent.id).count) assert_equal(0, email_notification_count('user_device_new', @agent.email)) assert_equal(0, email_notification_count('user_device_new_location', @agent.email)) assert_equal(result.class, Hash) @@ -394,7 +446,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(1, UserDevice.where(user_id: @agent.id).count) + assert_equal(0, UserDevice.where(user_id: @agent.id).count) assert_equal(0, email_notification_count('user_device_new', @agent.email)) assert_equal(0, email_notification_count('user_device_new_location', @agent.email)) @@ -408,7 +460,7 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest Scheduler.worker(true) - assert_equal(1, UserDevice.where(user_id: @agent.id).count) + assert_equal(0, UserDevice.where(user_id: @agent.id).count) assert_equal(0, email_notification_count('user_device_new', @agent.email)) assert_equal(0, email_notification_count('user_device_new_location', @agent.email)) ENV['USER_DEVICE_UPDATED_AT'] = nil @@ -424,9 +476,64 @@ class UserDeviceControllerTest < ActionDispatch::IntegrationTest # ip reset ENV['TEST_REMOTE_IP'] = '5.9.62.170' # de - assert_equal(1, UserDevice.where(user_id: @agent.id).count) + assert_equal(0, UserDevice.where(user_id: @agent.id).count) assert_equal(0, email_notification_count('user_device_new', @agent.email)) assert_equal(0, email_notification_count('user_device_new_location', @agent.email)) end + + test '11 - login with invalid fingerprint' do + + assert_equal(0, UserDevice.where(user_id: @admin.id).count) + assert_equal(0, email_notification_count('user_device_new', @admin.email)) + assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) + + params = { fingerprint: 'to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890to_long_1234567890', username: 'user-device-admin', password: 'adminpw' } + post '/api/v1/signin', params: params.to_json, headers: @headers + assert_response(422) + result = JSON.parse(@response.body) + + assert_equal(result.class, Hash) + assert_equal('fingerprint is 198 chars but can only be 160 chars!', result['error']) + assert_not(result['config']) + assert_not(controller.session[:user_device_fingerprint]) + + Scheduler.worker(true) + + assert_equal(0, UserDevice.where(user_id: @admin.id).count) + assert_equal(0, email_notification_count('user_device_new', @admin.email)) + assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) + + end + + test '12 - login form controller - check no user device logging' do + + Setting.set('form_ticket_create', true) + + assert_equal(0, UserDevice.where(user_id: @admin.id).count) + assert_equal(0, email_notification_count('user_device_new', @admin.email)) + assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) + + credentials = ActionController::HttpAuthentication::Basic.encode_credentials('user-device-admin', 'adminpw') + + params = { + fingerprint: 'long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890long_1234567890' + } + post '/api/v1/form_config', params: params.to_json, headers: @headers.merge('Authorization' => credentials) + assert_response(200) + + result = JSON.parse(@response.body) + assert_equal(result.class, Hash) + assert_not(result['error']) + assert(result['endpoint']) + assert_not(controller.session[:user_device_fingerprint]) + + Scheduler.worker(true) + + assert_equal(0, UserDevice.where(user_id: @admin.id).count) + assert_equal(0, email_notification_count('user_device_new', @admin.email)) + assert_equal(0, email_notification_count('user_device_new_location', @admin.email)) + + end + end diff --git a/test/unit/user_device_test.rb b/test/unit/user_device_test.rb index 4d1ff1e52..8c6561208 100644 --- a/test/unit/user_device_test.rb +++ b/test/unit/user_device_test.rb @@ -315,4 +315,18 @@ class UserDeviceTest < ActiveSupport::TestCase end + test 'invalid fingerprint size' do + + assert_raises(Exceptions::UnprocessableEntity) do + UserDevice.add( + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36', + '91.115.248.231', + @agent.id, + 'fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234fingerprint1234', + 'session', + ) + end + + end + end