diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 5aa6c40f1..87099d6fa 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -56,15 +56,7 @@ class ApplicationController < ActionController::Base end def user_for_paper_trail - if defined?(current_user) && current_user.present? - # Most of the time it's not loaded in correct time because PaperTrail before filter kicks in - # before current_user is defined. PaperTrail is triggered also at current_user - api_user_log_str(current_user) - elsif current_user.present? - "#{current_user.id}-#{current_user.username}" - else - 'public' - end + user_log_str(current_user) end def depp_current_user @@ -74,11 +66,8 @@ class ApplicationController < ActionController::Base ) end - def api_user_log_str(user) - if user.present? - "#{user.id}-api-#{user.username}" - else - 'api-public' - end + def user_log_str(user) + return 'public' if user.nil? + "#{user.id}-#{user.class}: #{user.username}" end end diff --git a/app/controllers/epp_controller.rb b/app/controllers/epp_controller.rb index f5133a74c..a61b155a1 100644 --- a/app/controllers/epp_controller.rb +++ b/app/controllers/epp_controller.rb @@ -120,7 +120,7 @@ class EppController < ApplicationController @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 = api_user_log_str(@current_user) + ::PaperTrail.whodunnit = user_log_str(@current_user) ::PaperSession.session = epp_session.session_id if epp_session.session_id.present? @current_user end @@ -350,6 +350,7 @@ class EppController < ApplicationController # rubocop: enable Style/PredicateName # rubocop: disable Metrics/CyclomaticComplexity + # rubocop: disable Metrics/PerceivedComplexity def write_to_epp_log # return nil if EPP_LOG_ENABLED request_command = params[:command] || params[:action] # error receives :command, other methods receive :action @@ -366,12 +367,13 @@ class EppController < ApplicationController request_successful: epp_errors.empty?, request_object: params[:epp_object_type], response: @response, - api_user_name: api_user_log_str(@api_user || current_user), + api_user_name: @api_user.try(:username) || current_user.try(:username) || 'api-public', api_user_registrar: @api_user.try(:registrar).try(:to_s) || current_user.try(:registrar).try(:to_s), ip: request.ip }) end # rubocop: enable Metrics/CyclomaticComplexity + # rubocop: enable Metrics/PerceivedComplexity def iptables_counter_update return if ENV['iptables_counter_enabled'].blank? && ENV['iptables_counter_enabled'] != 'true' diff --git a/app/models/concerns/versions.rb b/app/models/concerns/versions.rb index eff834218..2cbdca838 100644 --- a/app/models/concerns/versions.rb +++ b/app/models/concerns/versions.rb @@ -20,17 +20,15 @@ module Versions true end - # needs refactoring - # TODO: optimization work - # belongs_to :api_creator, class_name: 'ApiUser', foreign_key: :creator_str - # belongs_to :creator, class_name: 'User', foreign_key: :creator_str def creator return nil if creator_str.blank? - if creator_str =~ /^\d-api-/ - creator = ApiUser.find_by(id: creator_str) - else + if creator_str =~ /^\d+-AdminUser:/ creator = AdminUser.find_by(id: creator_str) + elsif creator_str =~ /^\d+-ApiUser:/ + creator = ApiUser.find_by(id: creator_str) + elsif creator_str =~ /^\d+-api-/ # depricated + creator = ApiUser.find_by(id: creator_str) end creator.present? ? creator : creator_str @@ -39,10 +37,12 @@ module Versions def updator return nil if updator_str.blank? - if updator_str =~ /^\d-api-/ - updator = ApiUser.find_by(id: updator_str) - else + if updator_str =~ /^\d+-AdminUser:/ updator = AdminUser.find_by(id: updator_str) + elsif updator_str =~ /^\d+-ApiUser:/ + updator = ApiUser.find_by(id: updator_str) + elsif updator_str =~ /^\d+-api-/ # depricated + updator = ApiUser.find_by(id: updator_str) end updator.present? ? updator : updator_str diff --git a/spec/epp/contact_spec.rb b/spec/epp/contact_spec.rb index dcd5e79db..b7b6bc976 100644 --- a/spec/epp/contact_spec.rb +++ b/spec/epp/contact_spec.rb @@ -85,7 +85,7 @@ describe 'EPP Contact', epp: true do log.request_command.should == 'create' log.request_object.should == 'contact' log.request_successful.should == true - log.api_user_name.should == '1-api-registrar1' + log.api_user_name.should == 'registrar1' log.api_user_registrar.should == 'registrar1' end diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb index b0c7535fc..d89c846b0 100644 --- a/spec/epp/domain_spec.rb +++ b/spec/epp/domain_spec.rb @@ -67,7 +67,7 @@ describe 'EPP Domain', epp: true do log.request_command.should == 'create' log.request_object.should == 'domain' log.request_successful.should == false - log.api_user_name.should == '1-api-registrar1' + log.api_user_name.should == 'registrar1' log.api_user_registrar.should == 'registrar1' log.request.should_not be_blank log.response.should_not be_blank @@ -1061,7 +1061,7 @@ describe 'EPP Domain', epp: true do log.request_command.should == 'transfer' log.request_object.should == 'domain' log.request_successful.should == true - log.api_user_name.should == '2-api-registrar2' + log.api_user_name.should == 'registrar2' log.api_user_registrar.should == 'registrar2' log.request.should_not be_blank log.response.should_not be_blank diff --git a/spec/epp/poll_spec.rb b/spec/epp/poll_spec.rb index 99e91103c..53f82221a 100644 --- a/spec/epp/poll_spec.rb +++ b/spec/epp/poll_spec.rb @@ -32,7 +32,7 @@ describe 'EPP Poll', epp: true do log.request_command.should == 'poll' log.request_object.should == 'poll' log.request_successful.should == true - log.api_user_name.should == '1-api-registrar1' + log.api_user_name.should == 'registrar1' log.api_user_registrar.should == @registrar1.name log.request.should_not be_blank log.response.should_not be_blank diff --git a/spec/epp/session_spec.rb b/spec/epp/session_spec.rb index 56910c1a3..d36e8f694 100644 --- a/spec/epp/session_spec.rb +++ b/spec/epp/session_spec.rb @@ -47,7 +47,7 @@ describe 'EPP Session', epp: true do log = ApiLog::EppLog.last log.request_command.should == 'login' log.request_successful.should == false - log.api_user_name.should == '2-api-inactive-user' + log.api_user_name.should == 'inactive-user' end it 'prohibits further actions unless logged in' do @@ -88,7 +88,7 @@ describe 'EPP Session', epp: true do log = ApiLog::EppLog.last log.request_command.should == 'login' log.request_successful.should == true - log.api_user_name.should == '1-api-gitlab' + log.api_user_name.should == 'gitlab' end it 'does not log in twice' do @@ -104,7 +104,7 @@ describe 'EPP Session', epp: true do log = ApiLog::EppLog.last log.request_command.should == 'login' log.request_successful.should == false - log.api_user_name.should == '1-api-gitlab' + log.api_user_name.should == 'gitlab' end it 'logs out epp user' do diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 4546ef580..dcb344c93 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -387,10 +387,10 @@ describe Domain do @api_user = Fabricate(:api_user) @user.id.should == 1 @api_user.id.should == 2 - ::PaperTrail.whodunnit = '2-api-testuser' + ::PaperTrail.whodunnit = '2-ApiUser: testuser' @domain = Fabricate(:domain) - @domain.creator_str.should == '2-api-testuser' + @domain.creator_str.should == '2-ApiUser: testuser' @domain.creator.should == @api_user @domain.creator.should_not == @user @@ -399,14 +399,14 @@ describe Domain do it 'should return api_creator when created by api user' do with_versioning do - @user = Fabricate(:admin_user) - @api_user = Fabricate(:api_user) - @user.id.should == 3 - @api_user.id.should == 4 - ::PaperTrail.whodunnit = '3-testuser' + @user = Fabricate(:admin_user, id: 1000) + @api_user = Fabricate(:api_user, id: 2000) + @user.id.should == 1000 + @api_user.id.should == 2000 + ::PaperTrail.whodunnit = '1000-AdminUser: testuser' @domain = Fabricate(:domain) - @domain.creator_str.should == '3-testuser' + @domain.creator_str.should == '1000-AdminUser: testuser' @domain.creator.should == @user @domain.creator.should_not == @api_user