diff --git a/app/controllers/epp/base_controller.rb b/app/controllers/epp/base_controller.rb index ee8592c82..51561f1ee 100644 --- a/app/controllers/epp/base_controller.rb +++ b/app/controllers/epp/base_controller.rb @@ -10,7 +10,8 @@ module Epp before_action :latin_only before_action :validate_against_schema before_action :validate_request - before_action :update_epp_session, if: -> { signed_in? } + before_action :enforce_epp_session_timeout, if: :signed_in? + before_action :iptables_counter_update, if: :signed_in? around_action :wrap_exceptions @@ -349,32 +350,21 @@ module Epp raise 'EPP session id is empty' unless epp_session_id.present? end - def update_epp_session - iptables_counter_update - - if session_timeout_reached? - @api_user = current_user # cache current_user for logging - epp_session.destroy - + def enforce_epp_session_timeout + if epp_session.timed_out? epp_errors << { - msg: t('session_timeout'), - code: '2201' + code: '2201', + msg: 'Authorization error: Session timeout', } - - handle_errors and return + handle_errors + epp_session.destroy! else - epp_session.update_column(:updated_at, Time.zone.now) + epp_session.update_last_access end end - def session_timeout_reached? - timeout = ENV['epp_session_timeout_seconds'].to_i.seconds - epp_session.updated_at < (Time.zone.now - timeout) - end - def iptables_counter_update return if ENV['iptables_counter_enabled'].blank? && ENV['iptables_counter_enabled'] != 'true' - return if current_user.blank? counter_update(current_user.registrar_code, ENV['iptables_server_ip']) end diff --git a/app/models/epp/expired_sessions.rb b/app/models/epp/expired_sessions.rb new file mode 100644 index 000000000..f9fefac46 --- /dev/null +++ b/app/models/epp/expired_sessions.rb @@ -0,0 +1,13 @@ +module Epp + class ExpiredSessions + attr_reader :sessions + + def initialize(sessions) + @sessions = sessions + end + + def clear + sessions.find_each(&:destroy!) + end + end +end diff --git a/app/models/epp_session.rb b/app/models/epp_session.rb index 6427c503c..f833b4894 100644 --- a/app/models/epp_session.rb +++ b/app/models/epp_session.rb @@ -3,6 +3,11 @@ class EppSession < ApplicationRecord validates :session_id, uniqueness: true, presence: true + class_attribute :timeout + self.timeout = ENV['epp_session_timeout_seconds'].to_i.seconds + + alias_attribute :last_access, :updated_at + def self.limit_per_registrar 4 end @@ -11,4 +16,21 @@ class EppSession < ApplicationRecord count = where(user_id: registrar.api_users.ids).where('updated_at >= ?', Time.zone.now - 1.second).count count >= limit_per_registrar 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 + + def update_last_access + touch + end + + def timed_out? + (updated_at + self.class.timeout).past? + end + + def expired? + timed_out? + end end diff --git a/test/integration/epp/base_test.rb b/test/integration/epp/base_test.rb index 456e7b41e..2610ddb98 100644 --- a/test/integration/epp/base_test.rb +++ b/test/integration/epp/base_test.rb @@ -7,6 +7,14 @@ class DummyEppController < Epp::BaseController end class EppBaseTest < EppTestCase + setup do + @original_session_timeout = EppSession.timeout + end + + teardown do + EppSession.timeout = @original_session_timeout + end + def test_internal_error Rails.application.routes.draw do post 'epp/command/internal_error', to: 'dummy_epp#internal_error', @@ -81,6 +89,62 @@ class EppBaseTest < EppTestCase assert_epp_response :authorization_error end + def test_deletes_session_when_timed_out + now = Time.zone.parse('2010-07-05') + travel_to now + timeout = 0.second + EppSession.timeout = timeout + session = epp_sessions(:api_bestnames) + session.update!(updated_at: now - timeout - 1.second) + + authentication_enabled_epp_request_xml = <<-XML + + + + + + #{domains(:shop).name} + + + + + XML + post '/epp/command/info', { frame: authentication_enabled_epp_request_xml }, + 'HTTP_COOKIE' => "session=#{session.session_id}" + + assert_epp_response :authorization_error + assert_nil EppSession.find_by(session_id: session.session_id) + end + + def test_session_last_access_is_updated_when_not_timed_out + now = Time.zone.parse('2010-07-05') + travel_to now + timeout = 1.seconds + EppSession.timeout = timeout + session = epp_sessions(:api_bestnames) + session.last_access = now - timeout + + authentication_enabled_epp_request_xml = <<-XML + + + + + + #{domains(:shop).name} + + + + + XML + + post '/epp/command/info', { frame: authentication_enabled_epp_request_xml }, + 'HTTP_COOKIE' => "session=#{session.session_id}" + session.reload + + assert_epp_response :completed_successfully + assert_equal now, session.last_access + end + private def valid_command_path diff --git a/test/models/epp_session_test.rb b/test/models/epp_session_test.rb index 6f90e2445..8ed63f6ab 100644 --- a/test/models/epp_session_test.rb +++ b/test/models/epp_session_test.rb @@ -3,6 +3,11 @@ require 'test_helper' class EppSessionTest < ActiveSupport::TestCase setup do @epp_session = epp_sessions(:api_bestnames) + @original_session_timeout = EppSession.timeout + end + + teardown do + EppSession.timeout = @original_session_timeout end def test_valid @@ -60,4 +65,39 @@ class EppSessionTest < ActiveSupport::TestCase refute EppSession.limit_reached?(registrars(:bestnames)) end + + def test_expired_scope + now = Time.zone.parse('2010-07-05') + travel_to now + session = epp_sessions(:api_bestnames) + timeout = 0.seconds + EppSession.timeout = timeout + + session.update!(last_access: now - timeout - 1.second) + assert_includes EppSession.expired, session, 'Expired session should be returned' + + session.update!(last_access: now - timeout) + + assert_not_includes EppSession.expired, session, 'Unexpired session should not be returned' + end + + def test_expired_when_timed_out + now = Time.zone.parse('2010-07-05') + travel_to now + timeout = 0.seconds + EppSession.timeout = timeout + @epp_session.last_access = now - timeout - 1.second + + assert @epp_session.expired? + end + + def test_not_expired_when_not_timed_out + now = Time.zone.parse('2010-07-05') + travel_to now + timeout = 0.seconds + EppSession.timeout = timeout + @epp_session.last_access = now - timeout + + assert_not @epp_session.expired? + end end diff --git a/test/tasks/epp/clear_expired_sessions_test.rb b/test/tasks/epp/clear_expired_sessions_test.rb index bd6c5c14f..a839afb9a 100644 --- a/test/tasks/epp/clear_expired_sessions_test.rb +++ b/test/tasks/epp/clear_expired_sessions_test.rb @@ -1,11 +1,19 @@ require 'test_helper' class EppClearExpiredSessionsTaskTest < ActiveSupport::TestCase + setup do + @original_session_timeout = EppSession.timeout + end + + teardown do + EppSession.timeout = @original_session_timeout + end + def test_clears_expired_epp_sessions - idle_timeout = 0.second - EppSession.idle_timeout = idle_timeout + timeout = 0.second + EppSession.timeout = timeout session = epp_sessions(:api_bestnames) - session.update!(updated_at: Time.zone.now - idle_timeout - 1.second) + session.update!(updated_at: Time.zone.now - timeout - 1.second) run_task