From 26ec7e3f12a3d88294b8631c57ed2c1c95b60a5e Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 16:55:40 +0300 Subject: [PATCH 01/15] Combine tests --- .../integration/epp/login/credentials_test.rb | 64 ------- .../epp/login/password_change_test.rb | 32 ---- .../epp/login/session_limit_test.rb | 61 ------- test/integration/epp/login_test.rb | 167 ++++++++++++++++++ 4 files changed, 167 insertions(+), 157 deletions(-) delete mode 100644 test/integration/epp/login/credentials_test.rb delete mode 100644 test/integration/epp/login/password_change_test.rb delete mode 100644 test/integration/epp/login/session_limit_test.rb create mode 100644 test/integration/epp/login_test.rb diff --git a/test/integration/epp/login/credentials_test.rb b/test/integration/epp/login/credentials_test.rb deleted file mode 100644 index 0f7dac97c..000000000 --- a/test/integration/epp/login/credentials_test.rb +++ /dev/null @@ -1,64 +0,0 @@ -require 'test_helper' - -class EppLoginCredentialsTest < EppTestCase - def test_correct_credentials - request_xml = <<-XML - - - - - test_bestnames - testtest - - 1.0 - en - - - https://epp.tld.ee/schema/domain-eis-1.0.xsd - https://epp.tld.ee/schema/contact-ee-1.1.xsd - urn:ietf:params:xml:ns:host-1.0 - - - - - XML - - post epp_login_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=new_session_id' } - assert EppSession.find_by(session_id: 'new_session_id') - assert_equal users(:api_bestnames), EppSession.find_by(session_id: 'new_session_id').user - assert_epp_response :completed_successfully - end - - def test_already_logged_in - assert true # Handled by EPP proxy - end - - def test_wrong_credentials - request_xml = <<-XML - - - - - non-existent - valid-but-wrong - - 1.0 - en - - - https://epp.tld.ee/schema/domain-eis-1.0.xsd - https://epp.tld.ee/schema/contact-ee-1.1.xsd - urn:ietf:params:xml:ns:host-1.0 - - - - - XML - - post epp_login_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=any_random_string' } - - assert_epp_response :authentication_error_server_closing_connection - end -end diff --git a/test/integration/epp/login/password_change_test.rb b/test/integration/epp/login/password_change_test.rb deleted file mode 100644 index 3b1834406..000000000 --- a/test/integration/epp/login/password_change_test.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'test_helper' - -class EppLoginPasswordChangeTest < EppTestCase - def test_password_change - request_xml = <<-XML - - - - - test_bestnames - testtest - new-password - - 1.0 - en - - - https://epp.tld.ee/schema/domain-eis-1.0.xsd - https://epp.tld.ee/schema/contact-ee-1.1.xsd - urn:ietf:params:xml:ns:host-1.0 - - - - - XML - - post epp_login_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=new_session_id' } - assert_equal 'new-password', users(:api_bestnames).plain_text_password - assert_epp_response :completed_successfully - end -end \ No newline at end of file diff --git a/test/integration/epp/login/session_limit_test.rb b/test/integration/epp/login/session_limit_test.rb deleted file mode 100644 index f1d83fe7b..000000000 --- a/test/integration/epp/login/session_limit_test.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'test_helper' - -class EppLoginSessionLimitTest < EppTestCase - setup do - travel_to Time.zone.parse('2010-07-05') - EppSession.delete_all - end - - def test_not_reached - (EppSession.limit_per_registrar - 1).times do - EppSession.create!(session_id: SecureRandom.hex, - user: users(:api_bestnames), - updated_at: Time.zone.parse('2010-07-05')) - end - - assert_difference 'EppSession.count' do - post epp_login_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=new_session_id' } - end - assert_epp_response :completed_successfully - end - - def test_reached - EppSession.limit_per_registrar.times do - EppSession.create!(session_id: SecureRandom.hex, - user: users(:api_bestnames), - updated_at: Time.zone.parse('2010-07-05')) - end - - assert_no_difference 'EppSession.count' do - post epp_login_path, params: { frame: request_xml }, - headers: { 'HTTP_COOKIE' => 'session=new_session_id' } - end - assert_epp_response :authentication_error_server_closing_connection - end - - private - - def request_xml - <<-XML - - - - - test_bestnames - testtest - - 1.0 - en - - - https://epp.tld.ee/schema/domain-eis-1.0.xsd - https://epp.tld.ee/schema/contact-ee-1.1.xsd - urn:ietf:params:xml:ns:host-1.0 - - - - - XML - end -end diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb new file mode 100644 index 000000000..d3ef22ec1 --- /dev/null +++ b/test/integration/epp/login_test.rb @@ -0,0 +1,167 @@ +require 'test_helper' + +class EppLoginTest < EppTestCase + def test_correct_credentials + request_xml = <<-XML + + + + + test_bestnames + testtest + + 1.0 + en + + + https://epp.tld.ee/schema/domain-eis-1.0.xsd + https://epp.tld.ee/schema/contact-ee-1.1.xsd + urn:ietf:params:xml:ns:host-1.0 + urn:ietf:params:xml:ns:keyrelay-1.0 + + + + + XML + + post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + assert EppSession.find_by(session_id: 'new_session_id') + assert_equal users(:api_bestnames), EppSession.find_by(session_id: 'new_session_id').user + assert_epp_response :completed_successfully + end + + def test_already_logged_in + assert true # Handled by mod_epp + end + + def test_wrong_credentials + request_xml = <<-XML + + + + + non-existent + valid-but-wrong + + 1.0 + en + + + https://epp.tld.ee/schema/domain-eis-1.0.xsd + https://epp.tld.ee/schema/contact-ee-1.1.xsd + urn:ietf:params:xml:ns:host-1.0 + urn:ietf:params:xml:ns:keyrelay-1.0 + + + + + XML + + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=any_random_string' + + assert_epp_response :authentication_error_server_closing_connection + end + + def test_password_change + request_xml = <<-XML + + + + + test_bestnames + testtest + new-password + + 1.0 + en + + + https://epp.tld.ee/schema/domain-eis-1.0.xsd + https://epp.tld.ee/schema/contact-ee-1.1.xsd + urn:ietf:params:xml:ns:host-1.0 + urn:ietf:params:xml:ns:keyrelay-1.0 + + + + + XML + + post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + assert_equal 'new-password', users(:api_bestnames).plain_text_password + assert_epp_response :completed_successfully + end + + def test_not_reached + travel_to Time.zone.parse('2010-07-05') + EppSession.delete_all + request_xml = <<-XML + + + + + test_bestnames + testtest + + 1.0 + en + + + https://epp.tld.ee/schema/domain-eis-1.0.xsd + https://epp.tld.ee/schema/contact-ee-1.1.xsd + urn:ietf:params:xml:ns:host-1.0 + urn:ietf:params:xml:ns:keyrelay-1.0 + + + + + XML + + (EppSession.limit_per_registrar - 1).times do + EppSession.create!(session_id: SecureRandom.hex, + user: users(:api_bestnames), + updated_at: Time.zone.parse('2010-07-05')) + end + + assert_difference 'EppSession.count' do + post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + end + assert_epp_response :completed_successfully + end + + def test_reached + travel_to Time.zone.parse('2010-07-05') + EppSession.delete_all + request_xml = <<-XML + + + + + test_bestnames + testtest + + 1.0 + en + + + https://epp.tld.ee/schema/domain-eis-1.0.xsd + https://epp.tld.ee/schema/contact-ee-1.1.xsd + urn:ietf:params:xml:ns:host-1.0 + urn:ietf:params:xml:ns:keyrelay-1.0 + + + + + XML + + EppSession.limit_per_registrar.times do + EppSession.create!(session_id: SecureRandom.hex, + user: users(:api_bestnames), + updated_at: Time.zone.parse('2010-07-05')) + end + + assert_no_difference 'EppSession.count' do + post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + end + assert_epp_response :authentication_error_server_closing_connection + end +end From 087300ff9d440e9c18759288184ee8337490223b Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 17:01:29 +0300 Subject: [PATCH 02/15] Reformat --- app/controllers/epp/sessions_controller.rb | 6 +++--- test/integration/epp/login_test.rb | 11 +++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/controllers/epp/sessions_controller.rb b/app/controllers/epp/sessions_controller.rb index b1c7fbbfb..e84659c86 100644 --- a/app/controllers/epp/sessions_controller.rb +++ b/app/controllers/epp/sessions_controller.rb @@ -90,8 +90,8 @@ module Epp if params[:parsed_frame].css('newPW').first unless @api_user.update(plain_text_password: params[:parsed_frame].css('newPW').first.text) handle_errors(@api_user) and return + end end - end epp_session = EppSession.new epp_session.session_id = epp_session_id @@ -100,8 +100,8 @@ module Epp render_epp_response('login_success') else handle_errors + end end - end def ip_white? webclient_request = ENV['webclient_ips'].split(',').map(&:strip).include?(request.ip) @@ -125,7 +125,7 @@ module Epp @api_user = current_user # cache current_user for logging epp_session.destroy render_epp_response('logout') - end + end ### HELPER METHODS ### diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index d3ef22ec1..2c69dcf49 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -23,11 +23,11 @@ class EppLoginTest < EppTestCase XML + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' - post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + assert_epp_response :completed_successfully assert EppSession.find_by(session_id: 'new_session_id') assert_equal users(:api_bestnames), EppSession.find_by(session_id: 'new_session_id').user - assert_epp_response :completed_successfully end def test_already_logged_in @@ -56,7 +56,6 @@ class EppLoginTest < EppTestCase XML - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=any_random_string' assert_epp_response :authentication_error_server_closing_connection @@ -85,8 +84,8 @@ class EppLoginTest < EppTestCase XML + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' - post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } assert_equal 'new-password', users(:api_bestnames).plain_text_password assert_epp_response :completed_successfully end @@ -123,7 +122,7 @@ class EppLoginTest < EppTestCase end assert_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' end assert_epp_response :completed_successfully end @@ -160,7 +159,7 @@ class EppLoginTest < EppTestCase end assert_no_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, { 'HTTP_COOKIE' => 'session=new_session_id' } + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' end assert_epp_response :authentication_error_server_closing_connection end From 7ba5b3b2ae74992aa1a4112848c37911d40d27b5 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 17:10:08 +0300 Subject: [PATCH 03/15] Refactor EPP login password change --- app/controllers/epp/sessions_controller.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/epp/sessions_controller.rb b/app/controllers/epp/sessions_controller.rb index e84659c86..6c3509786 100644 --- a/app/controllers/epp/sessions_controller.rb +++ b/app/controllers/epp/sessions_controller.rb @@ -87,10 +87,11 @@ module Epp end if success - if params[:parsed_frame].css('newPW').first - unless @api_user.update(plain_text_password: params[:parsed_frame].css('newPW').first.text) - handle_errors(@api_user) and return - end + new_password = params[:parsed_frame].at_css('newPW')&.text + + if new_password.present? + @api_user.plain_text_password = new_password + @api_user.save! end epp_session = EppSession.new From 3a5779782a111442b8cf6610e02a0f842106cc5d Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 17:53:32 +0300 Subject: [PATCH 04/15] Prohibit authenticated EPP user from logging in again Fixes #1313 --- app/controllers/epp/sessions_controller.rb | 14 +++++++++- test/integration/epp/login_test.rb | 30 ++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/app/controllers/epp/sessions_controller.rb b/app/controllers/epp/sessions_controller.rb index 6c3509786..df706b55d 100644 --- a/app/controllers/epp/sessions_controller.rb +++ b/app/controllers/epp/sessions_controller.rb @@ -88,12 +88,24 @@ module Epp if success new_password = params[:parsed_frame].at_css('newPW')&.text + password_change = new_password.present? - if new_password.present? + if password_change @api_user.plain_text_password = new_password @api_user.save! end + already_authenticated = EppSession.exists?(session_id: epp_session_id) + + if already_authenticated + epp_errors << { + msg: 'Command use error; Already authenticated', + code: 2002, + } + handle_errors + return + end + epp_session = EppSession.new epp_session.session_id = epp_session_id epp_session.user = @api_user diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index 2c69dcf49..c44ac1eee 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -30,8 +30,34 @@ class EppLoginTest < EppTestCase assert_equal users(:api_bestnames), EppSession.find_by(session_id: 'new_session_id').user end - def test_already_logged_in - assert true # Handled by mod_epp + def test_user_cannot_login_again + session = epp_sessions(:api_bestnames) + user = session.user + + request_xml = <<-XML + + + + + #{user.username} + #{user.plain_text_password} + + 1.0 + en + + + https://epp.tld.ee/schema/domain-eis-1.0.xsd + https://epp.tld.ee/schema/contact-ee-1.1.xsd + urn:ietf:params:xml:ns:host-1.0 + urn:ietf:params:xml:ns:keyrelay-1.0 + + + + + XML + post '/epp/session/login', { frame: request_xml }, HTTP_COOKIE: "session=#{session.session_id}" + + assert_epp_response :use_error end def test_wrong_credentials From 370b37cff6f479a49e3d8e21d3931817b6db4775 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 18:22:30 +0300 Subject: [PATCH 05/15] Improve readability --- test/integration/epp/login_test.rb | 65 +++++++++++++++++++----------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index c44ac1eee..83e41cc27 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -1,14 +1,17 @@ require 'test_helper' class EppLoginTest < EppTestCase - def test_correct_credentials + def test_logging_in_with_correct_credentials_creates_new_session + user = users(:api_bestnames) + new_session_id = 'new-session-id' + request_xml = <<-XML - test_bestnames - testtest + #{user.username} + #{user.plain_text_password} 1.0 en @@ -23,11 +26,13 @@ class EppLoginTest < EppTestCase XML - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' - + assert_difference 'EppSession.count' do + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => "session=#{new_session_id}" + end assert_epp_response :completed_successfully - assert EppSession.find_by(session_id: 'new_session_id') - assert_equal users(:api_bestnames), EppSession.find_by(session_id: 'new_session_id').user + session = EppSession.last + assert_equal new_session_id, session.session_id + assert_equal user, session.user end def test_user_cannot_login_again @@ -55,19 +60,25 @@ class EppLoginTest < EppTestCase XML - post '/epp/session/login', { frame: request_xml }, HTTP_COOKIE: "session=#{session.session_id}" + assert_no_difference 'EppSession.count' do + post '/epp/session/login', { frame: request_xml }, HTTP_COOKIE: "session=#{session.session_id}" + end assert_epp_response :use_error end - def test_wrong_credentials + def test_user_cannot_login_with_wrong_credentials + user = users(:api_bestnames) + wrong_password = 'a' * ApiUser.min_password_length + assert_not_equal wrong_password, user.plain_text_password + request_xml = <<-XML - non-existent - valid-but-wrong + #{user.username} + #{wrong_password} 1.0 en @@ -82,20 +93,26 @@ class EppLoginTest < EppTestCase XML - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=any_random_string' + assert_no_difference 'EppSession.count' do + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' + end assert_epp_response :authentication_error_server_closing_connection end def test_password_change + user = users(:api_bestnames) + new_password = 'a' * ApiUser.min_password_length + assert_not_equal new_password, user.plain_text_password + request_xml = <<-XML - test_bestnames - testtest - new-password + #{user.username} + #{user.plain_text_password} + #{new_password} 1.0 en @@ -110,10 +127,11 @@ class EppLoginTest < EppTestCase XML - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' + user.reload - assert_equal 'new-password', users(:api_bestnames).plain_text_password assert_epp_response :completed_successfully + assert_equal new_password, user.plain_text_password end def test_not_reached @@ -148,12 +166,13 @@ class EppLoginTest < EppTestCase end assert_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=non-existent' end assert_epp_response :completed_successfully end - def test_reached + def test_user_cannot_login_when_session_limit_reached + user = users(:api_bestnames) travel_to Time.zone.parse('2010-07-05') EppSession.delete_all request_xml = <<-XML @@ -161,8 +180,8 @@ class EppLoginTest < EppTestCase - test_bestnames - testtest + #{user.username} + #{user.plain_text_password} 1.0 en @@ -180,12 +199,12 @@ class EppLoginTest < EppTestCase EppSession.limit_per_registrar.times do EppSession.create!(session_id: SecureRandom.hex, - user: users(:api_bestnames), + user: user, updated_at: Time.zone.parse('2010-07-05')) end assert_no_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new_session_id' + post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' end assert_epp_response :authentication_error_server_closing_connection end From 744d6a2b537b4e3f70d429f886f1fd382d44f644 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 18:23:18 +0300 Subject: [PATCH 06/15] Remove unnecessary test --- test/integration/epp/login_test.rb | 37 ------------------------------ 1 file changed, 37 deletions(-) diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index 83e41cc27..fe8ef2cf8 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -134,43 +134,6 @@ class EppLoginTest < EppTestCase assert_equal new_password, user.plain_text_password end - def test_not_reached - travel_to Time.zone.parse('2010-07-05') - EppSession.delete_all - request_xml = <<-XML - - - - - test_bestnames - testtest - - 1.0 - en - - - https://epp.tld.ee/schema/domain-eis-1.0.xsd - https://epp.tld.ee/schema/contact-ee-1.1.xsd - urn:ietf:params:xml:ns:host-1.0 - urn:ietf:params:xml:ns:keyrelay-1.0 - - - - - XML - - (EppSession.limit_per_registrar - 1).times do - EppSession.create!(session_id: SecureRandom.hex, - user: users(:api_bestnames), - updated_at: Time.zone.parse('2010-07-05')) - end - - assert_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=non-existent' - end - assert_epp_response :completed_successfully - end - def test_user_cannot_login_when_session_limit_reached user = users(:api_bestnames) travel_to Time.zone.parse('2010-07-05') From daeb00ebe7719689703cee1ad5d469e01cef4ffe Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Fri, 13 Sep 2019 21:06:23 +0300 Subject: [PATCH 07/15] Change EPP response code according to its specification Fixes #587 --- app/controllers/epp/sessions_controller.rb | 4 ++-- app/models/epp/response/result/code.rb | 2 ++ test/integration/epp/login_test.rb | 2 +- test/models/epp/response/result/code_test.rb | 2 ++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/epp/sessions_controller.rb b/app/controllers/epp/sessions_controller.rb index df706b55d..04603dbe7 100644 --- a/app/controllers/epp/sessions_controller.rb +++ b/app/controllers/epp/sessions_controller.rb @@ -79,8 +79,8 @@ module Epp if success && EppSession.limit_reached?(@api_user.registrar) epp_errors << { - msg: 'Authentication error; server closing connection (connection limit reached)', - code: '2501' + msg: 'Session limit exceeded; server closing connection (connection limit reached)', + code: '2502', } success = false diff --git a/app/models/epp/response/result/code.rb b/app/models/epp/response/result/code.rb index 1be4a3f7c..10edf0a35 100644 --- a/app/models/epp/response/result/code.rb +++ b/app/models/epp/response/result/code.rb @@ -30,6 +30,7 @@ module Epp data_management_policy_violation: 2308, command_failed: 2400, authentication_error_server_closing_connection: 2501, + session_limit_exceeded_server_closing_connection: 2502, }.freeze private_constant :KEY_TO_VALUE @@ -59,6 +60,7 @@ module Epp 2308 => 'Data management policy violation', 2400 => 'Command failed', 2501 => 'Authentication error; server closing connection', + 2502 => 'Session limit exceeded; server closing connection', }.freeze private_constant :DEFAULT_DESCRIPTIONS diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index fe8ef2cf8..7442728a0 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -169,6 +169,6 @@ class EppLoginTest < EppTestCase assert_no_difference 'EppSession.count' do post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' end - assert_epp_response :authentication_error_server_closing_connection + assert_epp_response :session_limit_exceeded_server_closing_connection end end diff --git a/test/models/epp/response/result/code_test.rb b/test/models/epp/response/result/code_test.rb index f16013180..184a18438 100644 --- a/test/models/epp/response/result/code_test.rb +++ b/test/models/epp/response/result/code_test.rb @@ -51,6 +51,7 @@ class EppResponseResultCodeTest < ActiveSupport::TestCase data_management_policy_violation: 2308, command_failed: 2400, authentication_error_server_closing_connection: 2501, + session_limit_exceeded_server_closing_connection: 2502, } assert_equal codes, Epp::Response::Result::Code.codes end @@ -82,6 +83,7 @@ class EppResponseResultCodeTest < ActiveSupport::TestCase 2308 => 'Data management policy violation', 2400 => 'Command failed', 2501 => 'Authentication error; server closing connection', + 2502 => 'Session limit exceeded; server closing connection', } assert_equal descriptions, Epp::Response::Result::Code.default_descriptions end From 59fbcde4ca6356e334ec5b36823d4dd84060dfdd Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 17 Sep 2019 17:17:51 +0300 Subject: [PATCH 08/15] Remove hardcoded value --- app/models/epp_session.rb | 7 +++---- config/application.yml.sample | 2 ++ test/models/epp_session_test.rb | 4 ---- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb index f1b641aa0..1336be39e 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -6,11 +6,10 @@ class EppSession < ApplicationRecord class_attribute :timeout self.timeout = (ENV['epp_session_timeout_seconds'] || 300).to_i.seconds - alias_attribute :last_access, :updated_at + class_attribute :limit_per_registrar + self.limit_per_registrar = (ENV['epp_session_limit_per_registrar'] || 4).to_i - def self.limit_per_registrar - 4 - end + alias_attribute :last_access, :updated_at def self.limit_reached?(registrar) count = where(user_id: registrar.api_users.ids).where('updated_at >= ?', Time.zone.now - 1.second).count diff --git a/config/application.yml.sample b/config/application.yml.sample index ab64ed35e..65e83b603 100644 --- a/config/application.yml.sample +++ b/config/application.yml.sample @@ -170,6 +170,8 @@ tara_rant_redirect_uri: 'redirect_uri' default_email_validation_type: 'regex' +epp_session_limit_per_registrar: '4' + # Since the keys for staging are absent from the repo, we need to supply them separate for testing. test: payments_seb_bank_certificate: 'test/fixtures/files/seb_bank_cert.pem' diff --git a/test/models/epp_session_test.rb b/test/models/epp_session_test.rb index 8ed63f6ab..398753bd1 100644 --- a/test/models/epp_session_test.rb +++ b/test/models/epp_session_test.rb @@ -49,10 +49,6 @@ class EppSessionTest < ActiveSupport::TestCase end end - def test_limit_per_registrar - assert_equal 4, EppSession.limit_per_registrar - end - def test_limit_is_per_registrar travel_to Time.zone.parse('2010-07-05') EppSession.delete_all From 2d1e11ac6d7d93ba31f119640c7fd0810203c79c Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 17 Sep 2019 17:39:58 +0300 Subject: [PATCH 09/15] Improve readability --- test/integration/epp/login_test.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index 7442728a0..bd49f591d 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -134,10 +134,16 @@ class EppLoginTest < EppTestCase assert_equal new_password, user.plain_text_password end - def test_user_cannot_login_when_session_limit_reached + def test_user_cannot_login_when_session_limit_is_exceeded user = users(:api_bestnames) travel_to Time.zone.parse('2010-07-05') - EppSession.delete_all + eliminate_effect_of_existing_epp_sessions + EppSession.limit_per_registrar.times do + EppSession.create!(session_id: SecureRandom.hex, + user: user, + updated_at: Time.zone.parse('2010-07-05')) + end + request_xml = <<-XML @@ -160,15 +166,15 @@ class EppLoginTest < EppTestCase XML - EppSession.limit_per_registrar.times do - EppSession.create!(session_id: SecureRandom.hex, - user: user, - updated_at: Time.zone.parse('2010-07-05')) - end - assert_no_difference 'EppSession.count' do post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' end assert_epp_response :session_limit_exceeded_server_closing_connection end + + private + + def eliminate_effect_of_existing_epp_sessions + EppSession.delete_all + end end From 1e997ad5b1ff39f05eb1ab3f61e5868851fd9bd1 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 17 Sep 2019 17:50:02 +0300 Subject: [PATCH 10/15] Simplify test --- test/integration/epp/login_test.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index bd49f591d..ada2ff0c3 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -1,6 +1,14 @@ require 'test_helper' class EppLoginTest < EppTestCase + setup do + @original_session_limit_per_registrar = EppSession.limit_per_registrar + end + + teardown do + EppSession.limit_per_registrar = @original_session_limit_per_registrar + end + def test_logging_in_with_correct_credentials_creates_new_session user = users(:api_bestnames) new_session_id = 'new-session-id' @@ -138,11 +146,10 @@ class EppLoginTest < EppTestCase user = users(:api_bestnames) travel_to Time.zone.parse('2010-07-05') eliminate_effect_of_existing_epp_sessions - EppSession.limit_per_registrar.times do - EppSession.create!(session_id: SecureRandom.hex, - user: user, - updated_at: Time.zone.parse('2010-07-05')) - end + EppSession.limit_per_registrar = 1 + EppSession.create!(session_id: 'any', + user: user, + updated_at: Time.zone.parse('2010-07-05')) request_xml = <<-XML From 9eb7d3527b8bbc9175cbd53e9889eb6d99ffe611 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 17 Sep 2019 17:55:24 +0300 Subject: [PATCH 11/15] Do not take time into account when checking EPP session limit --- app/models/epp_session.rb | 2 +- test/integration/epp/login_test.rb | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb index 1336be39e..657f76973 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -12,7 +12,7 @@ class EppSession < ApplicationRecord alias_attribute :last_access, :updated_at def self.limit_reached?(registrar) - count = where(user_id: registrar.api_users.ids).where('updated_at >= ?', Time.zone.now - 1.second).count + count = where(user_id: registrar.api_users.ids).count count >= limit_per_registrar end diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index ada2ff0c3..fe62f9afe 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -144,12 +144,9 @@ class EppLoginTest < EppTestCase def test_user_cannot_login_when_session_limit_is_exceeded user = users(:api_bestnames) - travel_to Time.zone.parse('2010-07-05') eliminate_effect_of_existing_epp_sessions EppSession.limit_per_registrar = 1 - EppSession.create!(session_id: 'any', - user: user, - updated_at: Time.zone.parse('2010-07-05')) + EppSession.create!(session_id: 'any', user: user) request_xml = <<-XML From cbb3c0d5ef706c63102cd9e1e311d24aab162621 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Tue, 17 Sep 2019 18:03:49 +0300 Subject: [PATCH 12/15] Improve readability --- app/models/epp_session.rb | 6 +++--- config/application.yml.sample | 2 +- test/integration/epp/login_test.rb | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb index 657f76973..7567bf3d2 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -6,14 +6,14 @@ class EppSession < ApplicationRecord class_attribute :timeout self.timeout = (ENV['epp_session_timeout_seconds'] || 300).to_i.seconds - class_attribute :limit_per_registrar - self.limit_per_registrar = (ENV['epp_session_limit_per_registrar'] || 4).to_i + class_attribute :sessions_per_registrar + self.sessions_per_registrar = (ENV['epp_session_limit_per_registrar'] || 4).to_i alias_attribute :last_access, :updated_at def self.limit_reached?(registrar) count = where(user_id: registrar.api_users.ids).count - count >= limit_per_registrar + count >= sessions_per_registrar end def self.expired diff --git a/config/application.yml.sample b/config/application.yml.sample index 65e83b603..e979e772a 100644 --- a/config/application.yml.sample +++ b/config/application.yml.sample @@ -170,7 +170,7 @@ tara_rant_redirect_uri: 'redirect_uri' default_email_validation_type: 'regex' -epp_session_limit_per_registrar: '4' +epp_sessions_per_registrar: '4' # Since the keys for staging are absent from the repo, we need to supply them separate for testing. test: diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index fe62f9afe..d82ba8891 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -2,11 +2,11 @@ require 'test_helper' class EppLoginTest < EppTestCase setup do - @original_session_limit_per_registrar = EppSession.limit_per_registrar + @original_sessions_per_registrar_setting = EppSession.sessions_per_registrar end teardown do - EppSession.limit_per_registrar = @original_session_limit_per_registrar + EppSession.sessions_per_registrar = @original_sessions_per_registrar_setting end def test_logging_in_with_correct_credentials_creates_new_session @@ -142,10 +142,10 @@ class EppLoginTest < EppTestCase assert_equal new_password, user.plain_text_password end - def test_user_cannot_login_when_session_limit_is_exceeded + def test_user_cannot_login_when_max_allowed_sessions_per_registrar_is_exceeded user = users(:api_bestnames) eliminate_effect_of_existing_epp_sessions - EppSession.limit_per_registrar = 1 + EppSession.sessions_per_registrar = 1 EppSession.create!(session_id: 'any', user: user) request_xml = <<-XML From 26c8fe6a3e8e68a75ba3076e1d329d50f5ea0d4a Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 4 Sep 2020 15:06:40 +0500 Subject: [PATCH 13/15] Fix tests --- test/integration/epp/login_test.rb | 15 ++++++++++----- test/models/epp_session_test.rb | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/test/integration/epp/login_test.rb b/test/integration/epp/login_test.rb index d82ba8891..dd7a750c9 100644 --- a/test/integration/epp/login_test.rb +++ b/test/integration/epp/login_test.rb @@ -35,7 +35,8 @@ class EppLoginTest < EppTestCase XML assert_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => "session=#{new_session_id}" + post '/epp/session/login', params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => "session=#{new_session_id}" } end assert_epp_response :completed_successfully session = EppSession.last @@ -70,7 +71,8 @@ class EppLoginTest < EppTestCase XML assert_no_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, HTTP_COOKIE: "session=#{session.session_id}" + post '/epp/session/login', params: { frame: request_xml }, + headers: { HTTP_COOKIE: "session=#{session.session_id}" } end assert_epp_response :use_error end @@ -103,7 +105,8 @@ class EppLoginTest < EppTestCase XML assert_no_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' + post '/epp/session/login', params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=new-session-id' } end assert_epp_response :authentication_error_server_closing_connection end @@ -135,7 +138,8 @@ class EppLoginTest < EppTestCase XML - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' + post '/epp/session/login', params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=new-session-id' } user.reload assert_epp_response :completed_successfully @@ -171,7 +175,8 @@ class EppLoginTest < EppTestCase XML assert_no_difference 'EppSession.count' do - post '/epp/session/login', { frame: request_xml }, 'HTTP_COOKIE' => 'session=new-session-id' + post '/epp/session/login', params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=new-session-id' } end assert_epp_response :session_limit_exceeded_server_closing_connection end diff --git a/test/models/epp_session_test.rb b/test/models/epp_session_test.rb index 398753bd1..3fd184e0b 100644 --- a/test/models/epp_session_test.rb +++ b/test/models/epp_session_test.rb @@ -53,7 +53,7 @@ class EppSessionTest < ActiveSupport::TestCase travel_to Time.zone.parse('2010-07-05') EppSession.delete_all - EppSession.limit_per_registrar.times do + EppSession.sessions_per_registrar.times do EppSession.create!(session_id: SecureRandom.hex, user: users(:api_goodnames), updated_at: Time.zone.parse('2010-07-05')) From 495e1a4ae169a7f200b63e49087cbd3ce0448e71 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 13 Nov 2020 13:36:53 +0500 Subject: [PATCH 14/15] Fix env usage in EppSession model --- app/models/epp_session.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb index 7567bf3d2..6bec39ee3 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -7,7 +7,7 @@ class EppSession < ApplicationRecord self.timeout = (ENV['epp_session_timeout_seconds'] || 300).to_i.seconds class_attribute :sessions_per_registrar - self.sessions_per_registrar = (ENV['epp_session_limit_per_registrar'] || 4).to_i + self.sessions_per_registrar = (ENV['epp_sessions_per_registrar'] || 4).to_i alias_attribute :last_access, :updated_at From 13b0d45e255ff44834407b00e77f7ffac0057ae0 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 13 Nov 2020 14:38:24 +0500 Subject: [PATCH 15/15] Check for 4 sessions only in inexpired ones --- app/models/epp_session.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb index 6bec39ee3..a6fb97ed2 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -11,13 +11,21 @@ class EppSession < ApplicationRecord alias_attribute :last_access, :updated_at + scope :not_expired, + lambda { + where(':now <= (updated_at + interval :interval)', now: Time.zone.now, interval: interval) + } + def self.limit_reached?(registrar) - count = where(user_id: registrar.api_users.ids).count + count = where(user_id: registrar.api_users.ids).not_expired.count count >= sessions_per_registrar end + def self.interval + "#{timeout.parts.first.second} #{timeout.parts.first.first}" + end + def self.expired - interval = "#{timeout.parts.first.second} #{timeout.parts.first.first}" where(':now > (updated_at + interval :interval)', now: Time.zone.now, interval: interval) end