diff --git a/app/controllers/epp/sessions_controller.rb b/app/controllers/epp/sessions_controller.rb index b1c7fbbfb..04603dbe7 100644 --- a/app/controllers/epp/sessions_controller.rb +++ b/app/controllers/epp/sessions_controller.rb @@ -79,19 +79,32 @@ 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 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 + new_password = params[:parsed_frame].at_css('newPW')&.text + password_change = 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 - end epp_session = EppSession.new epp_session.session_id = epp_session_id @@ -100,8 +113,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 +138,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/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/app/models/epp_session.rb b/app/models/epp_session.rb index f1b641aa0..a6fb97ed2 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -6,19 +6,26 @@ class EppSession < ApplicationRecord class_attribute :timeout self.timeout = (ENV['epp_session_timeout_seconds'] || 300).to_i.seconds + class_attribute :sessions_per_registrar + self.sessions_per_registrar = (ENV['epp_sessions_per_registrar'] || 4).to_i + alias_attribute :last_access, :updated_at - def self.limit_per_registrar - 4 - end + 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).where('updated_at >= ?', Time.zone.now - 1.second).count - count >= limit_per_registrar + 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 diff --git a/config/application.yml.sample b/config/application.yml.sample index ab64ed35e..e979e772a 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_sessions_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/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..dd7a750c9 --- /dev/null +++ b/test/integration/epp/login_test.rb @@ -0,0 +1,189 @@ +require 'test_helper' + +class EppLoginTest < EppTestCase + setup do + @original_sessions_per_registrar_setting = EppSession.sessions_per_registrar + end + + teardown do + EppSession.sessions_per_registrar = @original_sessions_per_registrar_setting + end + + def test_logging_in_with_correct_credentials_creates_new_session + user = users(:api_bestnames) + new_session_id = 'new-session-id' + + 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 + assert_difference 'EppSession.count' do + post '/epp/session/login', params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => "session=#{new_session_id}" } + end + assert_epp_response :completed_successfully + session = EppSession.last + assert_equal new_session_id, session.session_id + assert_equal user, session.user + end + + 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 + + assert_no_difference 'EppSession.count' do + post '/epp/session/login', params: { frame: request_xml }, + headers: { HTTP_COOKIE: "session=#{session.session_id}" } + end + assert_epp_response :use_error + end + + 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 + + + + + #{user.username} + #{wrong_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 + + assert_no_difference 'EppSession.count' do + 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 + + 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 + + + + + #{user.username} + #{user.plain_text_password} + #{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', params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=new-session-id' } + user.reload + + assert_epp_response :completed_successfully + assert_equal new_password, user.plain_text_password + end + + def test_user_cannot_login_when_max_allowed_sessions_per_registrar_is_exceeded + user = users(:api_bestnames) + eliminate_effect_of_existing_epp_sessions + EppSession.sessions_per_registrar = 1 + EppSession.create!(session_id: 'any', user: 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 + + assert_no_difference 'EppSession.count' do + 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 + + private + + def eliminate_effect_of_existing_epp_sessions + EppSession.delete_all + 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 diff --git a/test/models/epp_session_test.rb b/test/models/epp_session_test.rb index 8ed63f6ab..3fd184e0b 100644 --- a/test/models/epp_session_test.rb +++ b/test/models/epp_session_test.rb @@ -49,15 +49,11 @@ 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 - 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'))