Registry 700

.codeclimate.yml

@@ -1,5 +1,4 @@
----
-engines:
+plugins:
   brakeman:
     enabled: true
   bundler-audit:
@@ -14,8 +13,6 @@ engines:
       languages:
       - ruby
       - javascript
-      - python
-      - php
   eslint:
     enabled: true
   fixme:
@@ -33,24 +30,9 @@ engines:
     checks:
       IrresponsibleModule:
         enabled: false
-ratings:
-  paths:
-  - Gemfile.lock
-  - "**.erb"
-  - "**.haml"
-  - "**.rb"
-  - "**.rhtml"
-  - "**.slim"
-  - "**.css"
-  - "**.coffee"
-  - "**.inc"
-  - "**.js"
-  - "**.jsx"
-  - "**.module"
-  - "**.php"
-  - "**.py"
-exclude_paths:
-- config/
-- db/
-- spec/
-- vendor/
+exclude_patterns:
+- "config/"
+- "db/"
+- "vendor/"
+- "spec/"
+- "test/"

.reek

@@ -34,7 +34,6 @@ UncommunicativeVariableName:
   - Admin::SettingsController#create
   - Epp::DomainsController#renew
   - Epp::DomainsController#update
-  - Epp::SessionsController#connection_limit_ok?
   - Epp::SessionsController#login
   - EppController
   - EppController#create_full_selectors
@@ -172,7 +171,6 @@ DuplicateMethodCall:
   - Epp::PollsController#ack_poll
   - Epp::PollsController#poll
   - Epp::PollsController#req_poll
-  - Epp::SessionsController#connection_limit_ok?
   - Epp::SessionsController#ip_white?
   - Epp::SessionsController#login
   - Epp::SessionsController#login_params
@@ -538,7 +536,6 @@ IrresponsibleModule:
   - DomainStatus
   - DomainTransfer
   - Epp::Contact
-  - EppSession
   - Invoice
   - InvoiceItem
   - Keyrelay
@@ -960,7 +957,6 @@ FeatureEnvy:
   - ActionDispatch::Flash#call
   - Ransack::Adapters::ActiveRecord::Context#evaluate
   - EppConstraint#matches?
-  - Requests::SessionHelpers#sign_in_to_epp_area
 TooManyMethods:
   exclude:
   - Epp::ContactsController
@@ -1027,7 +1023,6 @@ PrimaDonnaMethod:
   - Contact
   - Domain
   - Epp::Domain
-  - EppSession
   - RegistrantVerification
   - Registrar
 BooleanParameter:

.rubocop_todo.yml

@@ -655,11 +655,6 @@ Performance/StringReplacement:
     - 'app/models/directo.rb'
     - 'app/models/dnskey.rb'
 
-# Offense count: 1
-Security/MarshalLoad:
-  Exclude:
-    - 'app/models/epp_session.rb'
-
 # Offense count: 4
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.

app/controllers/epp/sessions_controller.rb

@@ -74,7 +74,7 @@ def login
       success = false
     end
 
-    if success && !connection_limit_ok?
+    if success && EppSession.limit_reached?(@api_user.registrar)
       epp_errors << {
         msg: 'Authentication error; server closing connection (connection limit reached)',
         code: '2501'
@@ -91,8 +91,10 @@ def login
         end
       end
 
-      epp_session[:api_user_id] = @api_user.id
-      epp_session.update_column(:registrar_id, @api_user.registrar_id)
+      epp_session = EppSession.new
+      epp_session.session_id = epp_session_id
+      epp_session.user = @api_user
+      epp_session.save!
       render_epp_response('login_success')
     else
       response.headers['X-EPP-Returncode'] = '2500'
@@ -113,17 +115,16 @@ def ip_white?
     true
   end
 
-  def connection_limit_ok?
-    return true if Rails.env.test? || Rails.env.development?
-    c = EppSession.where(
-      'registrar_id = ? AND updated_at >= ?', @api_user.registrar_id, Time.zone.now - 1.second
-    ).count
-
-    return false if c >= 4
-    true
-  end
-
   def logout
+    unless signed_in?
+      epp_errors << {
+        code: 2201,
+        msg: 'Authorization error'
+      }
+      handle_errors
+      return
+    end
+
     @api_user = current_user # cache current_user for logging
     epp_session.destroy
     response.headers['X-EPP-Returncode'] = '1500'

app/controllers/epp_controller.rb

@@ -4,11 +4,12 @@ class EppController < ApplicationController
   protect_from_forgery with: :null_session
   skip_before_action :verify_authenticity_token
 
+  before_action :ensure_session_id_passed
   before_action :generate_svtrid
   before_action :latin_only
   before_action :validate_against_schema
   before_action :validate_request
-  before_action :update_epp_session
+  before_action :update_epp_session, if: 'signed_in?'
 
   around_action :catch_epp_errors
 
@@ -86,41 +87,13 @@ def params_hash # TODO: THIS IS DEPRECATED AND WILL BE REMOVED IN FUTURE
     @params_hash ||= Hash.from_xml(params[:frame]).with_indifferent_access
   end
 
-  # SESSION MANAGEMENT
   def epp_session
-    cookies # Probably does some initialization
-    cookie = env['rack.request.cookie_hash'] || {}
-    EppSession.find_or_initialize_by(session_id: cookie['session'])
-  end
-
-  def update_epp_session
-    iptables_counter_update
-    e_s = epp_session
-    return if e_s.new_record?
-
-    if !Rails.env.development? && (e_s.updated_at < Time.zone.now - 5.minutes)
-      @api_user = current_user # cache current_user for logging
-      e_s.destroy
-      response.headers['X-EPP-Returncode'] = '1500'
-
-      epp_errors << {
-        msg: t('session_timeout'),
-        code: '2201'
-      }
-
-      handle_errors and return
-    else
-      e_s.update_column(:updated_at, Time.zone.now)
-    end
+    EppSession.find_by(session_id: epp_session_id)
   end
 
   def current_user
-    @current_user ||= ApiUser.find_by_id(epp_session[:api_user_id])
-    # by default PaperTrail uses before filter and at that
-    # time current_user is not yet present
-    ::PaperTrail.whodunnit = user_log_str(@current_user)
-    ::PaperSession.session = epp_session.session_id if epp_session.session_id.present?
-    @current_user
+    return unless signed_in?
+    epp_session.user
   end
 
   # ERROR + RESPONSE HANDLING
@@ -397,4 +370,42 @@ def resource
     name = self.class.to_s.sub("Epp::","").sub("Controller","").underscore.singularize
     instance_variable_get("@#{name}")
   end
+
+  private
+
+  def signed_in?
+    epp_session
+  end
+
+  def epp_session_id
+    cookies[:session] # Passed by mod_epp https://github.com/mod-epp/mod-epp#requestscript-interface
+  end
+
+  def ensure_session_id_passed
+    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
+      response.headers['X-EPP-Returncode'] = '1500'
+
+      epp_errors << {
+        msg: t('session_timeout'),
+        code: '2201'
+      }
+
+      handle_errors and return
+    else
+      epp_session.update_column(:updated_at, Time.zone.now)
+    end
+  end
+
+  def session_timeout_reached?
+    timeout = 5.minutes
+    epp_session.updated_at < (Time.zone.now - timeout)
+  end
 end

app/models/epp_session.rb

@@ -1,36 +1,14 @@
 class EppSession < ActiveRecord::Base
-  before_save :marshal_data!
+  belongs_to :user, required: true
 
-  belongs_to :registrar
-  # rubocop: disable Rails/ReadWriteAttribute
-  # Turned back to read_attribute, thus in Rails 4
-  # there is differences between self[:data] and read_attribute.
-  def data
-    @data ||= self.class.unmarshal(read_attribute(:data)) || {}
-  end
-  # rubocop: enable Rails/ReadWriteAttribute
-
-  def [](key)
-    data[key.to_sym]
-  end
+  validates :session_id, uniqueness: true, presence: true
 
-  def []=(key, value)
-    data[key.to_sym] = value
-    save!
+  def self.limit_per_registrar
+    4
   end
 
-  def marshal_data!
-    self.data = self.class.marshal(data)
-  end
-
-  class << self
-    def marshal(data)
-      ::Base64.encode64(Marshal.dump(data)) if data
-    end
-
-    def unmarshal(data)
-      return data unless data.is_a? String
-      Marshal.load(::Base64.decode64(data)) if data
-    end
+  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
   end
 end

db/migrate/20180206213435_change_epp_sessions_session_id_to_not_null.rb

@@ -0,0 +1,5 @@
+class ChangeEppSessionsSessionIdToNotNull < ActiveRecord::Migration
+  def change
+    change_column_null :epp_sessions, :session_id, false
+  end
+end

db/migrate/20180206234620_add_epp_sessions_user_id.rb

@@ -0,0 +1,5 @@
+class AddEppSessionsUserId < ActiveRecord::Migration
+  def change
+    add_reference :epp_sessions, :user, foreign_key: true
+  end
+end

db/migrate/20180207071528_extract_user_id_from_epp_sessions_data.rb

@@ -0,0 +1,10 @@
+class ExtractUserIdFromEppSessionsData < ActiveRecord::Migration
+  def change
+    EppSession.all.each do |epp_session|
+      user_id = Marshal.load(::Base64.decode64(epp_session.data_before_type_cast))[:api_user_id]
+      user = ApiUser.find(user_id)
+      epp_session.user = user
+      epp_session.save!
+    end
+  end
+end

db/migrate/20180207072139_remove_epp_sessions_data.rb

@@ -0,0 +1,5 @@
+class RemoveEppSessionsData < ActiveRecord::Migration
+  def change
+    remove_column :epp_sessions, :data, :string
+  end
+end

db/migrate/20180212123810_remove_epp_sessions_registrar_id.rb

@@ -0,0 +1,5 @@
+class RemoveEppSessionsRegistrarId < ActiveRecord::Migration
+  def change
+    remove_column :epp_sessions, :registrar_id, :integer
+  end
+end

db/migrate/20180212152810_add_epp_sessions_session_id_unique_constraint.rb

@@ -0,0 +1,13 @@
+class AddEppSessionsSessionIdUniqueConstraint < ActiveRecord::Migration
+  def up
+    execute <<-SQL
+      ALTER TABLE epp_sessions ADD CONSTRAINT unique_session_id UNIQUE (session_id)
+    SQL
+  end
+
+  def down
+    execute <<-SQL
+      ALTER TABLE epp_sessions DROP CONSTRAINT unique_session_id
+    SQL
+  end
+end

db/migrate/20180212154731_remove_epp_sessions_session_id_unique_index.rb

@@ -0,0 +1,5 @@
+class RemoveEppSessionsSessionIdUniqueIndex < ActiveRecord::Migration
+  def change
+    remove_index :epp_sessions, name: :index_epp_sessions_on_session_id
+  end
+end

db/migrate/20180213183818_change_epp_sessions_user_id_to_not_null.rb

@@ -0,0 +1,5 @@
+class ChangeEppSessionsUserIdToNotNull < ActiveRecord::Migration
+  def change
+    change_column_null :epp_sessions, :user_id, false
+  end
+end