diff options
author | Stan Hu <stanhu@gmail.com> | 2019-08-20 18:12:28 +0000 |
---|---|---|
committer | Mayra Cabrera <mcabrera@gitlab.com> | 2019-08-20 18:12:28 +0000 |
commit | e632ae80845f849f93e4d85ef9f836a4792844c9 (patch) | |
tree | 5715c29c7a6b6ed93e16f8ab0f59b229af3ba646 | |
parent | a493cccdda6d2ab0fd21e60a144504a6cf6747ee (diff) | |
download | gitlab-ce-e632ae80845f849f93e4d85ef9f836a4792844c9.tar.gz |
Standardize remote_ip and path keys for auth.log and api_json.log
Current `auth.log` uses `fullpath` and `ip`, while `api_json.log` uses
`remote_ip` and `path` for the same fields. Let's standardize these
namings to make it easier for people working with the data.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/66167
-rw-r--r-- | app/controllers/concerns/invisible_captcha.rb | 4 | ||||
-rw-r--r-- | config/initializers/rack_attack_logging.rb | 4 | ||||
-rw-r--r-- | doc/administration/logs.md | 2 | ||||
-rw-r--r-- | lib/api/api.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/action_rate_limiter.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/grape_logging/loggers/client_env_logger.rb | 16 | ||||
-rw-r--r-- | spec/controllers/projects/raw_controller_spec.rb | 4 | ||||
-rw-r--r-- | spec/controllers/registrations_controller_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/action_rate_limiter_spec.rb | 4 | ||||
-rw-r--r-- | spec/requests/rack_attack_global_spec.rb | 12 |
10 files changed, 36 insertions, 20 deletions
diff --git a/app/controllers/concerns/invisible_captcha.rb b/app/controllers/concerns/invisible_captcha.rb index c9f66e5c194..45c0a5c58ef 100644 --- a/app/controllers/concerns/invisible_captcha.rb +++ b/app/controllers/concerns/invisible_captcha.rb @@ -41,9 +41,9 @@ module InvisibleCaptcha request_information = { message: message, env: :invisible_captcha_signup_bot_detected, - ip: request.ip, + remote_ip: request.ip, request_method: request.request_method, - fullpath: request.fullpath + path: request.fullpath } Gitlab::AuthLogger.error(request_information) diff --git a/config/initializers/rack_attack_logging.rb b/config/initializers/rack_attack_logging.rb index 7eb34bd69e5..b43fff24bb0 100644 --- a/config/initializers/rack_attack_logging.rb +++ b/config/initializers/rack_attack_logging.rb @@ -7,9 +7,9 @@ ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, r rack_attack_info = { message: 'Rack_Attack', env: req.env['rack.attack.match_type'], - ip: req.ip, + remote_ip: req.ip, request_method: req.request_method, - fullpath: req.fullpath + path: req.fullpath } if %w(throttle_authenticated_api throttle_authenticated_web).include? req.env['rack.attack.matched'] diff --git a/doc/administration/logs.md b/doc/administration/logs.md index a57ef8ddc7d..9030c941cad 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -88,7 +88,7 @@ Introduced in GitLab 10.0, this file lives in It helps you see requests made directly to the API. For example: ```json -{"time":"2018-10-29T12:49:42.123Z","severity":"INFO","duration":709.08,"db":14.59,"view":694.49,"status":200,"method":"GET","path":"/api/v4/projects","params":[{"key":"action","value":"git-upload-pack"},{"key":"changes","value":"_any"},{"key":"key_id","value":"secret"},{"key":"secret_token","value":"[FILTERED]"}],"host":"localhost","ip":"::1","ua":"Ruby","route":"/api/:version/projects","user_id":1,"username":"root","queue_duration":100.31,"gitaly_calls":30,"gitaly_duration":5.36} +{"time":"2018-10-29T12:49:42.123Z","severity":"INFO","duration":709.08,"db":14.59,"view":694.49,"status":200,"method":"GET","path":"/api/v4/projects","params":[{"key":"action","value":"git-upload-pack"},{"key":"changes","value":"_any"},{"key":"key_id","value":"secret"},{"key":"secret_token","value":"[FILTERED]"}],"host":"localhost","remote_ip":"::1","ua":"Ruby","route":"/api/:version/projects","user_id":1,"username":"root","queue_duration":100.31,"gitaly_calls":30,"gitaly_duration":5.36} ``` This entry above shows an access to an internal endpoint to check whether an diff --git a/lib/api/api.rb b/lib/api/api.rb index e500a93b31e..219ed45eff6 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -18,7 +18,7 @@ module API formatter: Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp.new, include: [ GrapeLogging::Loggers::FilterParameters.new(LOG_FILTERS), - GrapeLogging::Loggers::ClientEnv.new, + Gitlab::GrapeLogging::Loggers::ClientEnvLogger.new, Gitlab::GrapeLogging::Loggers::RouteLogger.new, Gitlab::GrapeLogging::Loggers::UserLogger.new, Gitlab::GrapeLogging::Loggers::QueueDurationLogger.new, diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb index fdb06d00548..0e8707af631 100644 --- a/lib/gitlab/action_rate_limiter.rb +++ b/lib/gitlab/action_rate_limiter.rb @@ -49,9 +49,9 @@ module Gitlab request_information = { message: 'Action_Rate_Limiter_Request', env: type, - ip: request.ip, + remote_ip: request.ip, request_method: request.request_method, - fullpath: request.fullpath + path: request.fullpath } if current_user diff --git a/lib/gitlab/grape_logging/loggers/client_env_logger.rb b/lib/gitlab/grape_logging/loggers/client_env_logger.rb new file mode 100644 index 00000000000..3acc6f6a2ef --- /dev/null +++ b/lib/gitlab/grape_logging/loggers/client_env_logger.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# This is a fork of +# https://github.com/aserafin/grape_logging/blob/master/lib/grape_logging/loggers/client_env.rb +# to use remote_ip instead of ip. +module Gitlab + module GrapeLogging + module Loggers + class ClientEnvLogger < ::GrapeLogging::Loggers::Base + def parameters(request, _) + { remote_ip: request.env["HTTP_X_FORWARDED_FOR"] || request.env["REMOTE_ADDR"], ua: request.env["HTTP_USER_AGENT"] } + end + end + end + end +end diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index b958f419a19..8b43d1264b2 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -67,9 +67,9 @@ describe Projects::RawController do attributes = { message: 'Action_Rate_Limiter_Request', env: :raw_blob_request_limit, - ip: '0.0.0.0', + remote_ip: '0.0.0.0', request_method: 'GET', - fullpath: "/#{project.full_path}/raw/#{file_path}" + path: "/#{project.full_path}/raw/#{file_path}" } expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index fed4fc810f2..35487682462 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -129,9 +129,9 @@ describe RegistrationsController do { message: auth_log_message, env: :invisible_captcha_signup_bot_detected, - ip: '0.0.0.0', + remote_ip: '0.0.0.0', request_method: 'POST', - fullpath: '/users' + path: '/users' } end diff --git a/spec/lib/gitlab/action_rate_limiter_spec.rb b/spec/lib/gitlab/action_rate_limiter_spec.rb index 8dbad32dfb4..8b510a475d2 100644 --- a/spec/lib/gitlab/action_rate_limiter_spec.rb +++ b/spec/lib/gitlab/action_rate_limiter_spec.rb @@ -74,9 +74,9 @@ describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do { message: 'Action_Rate_Limiter_Request', env: type, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: fullpath + path: fullpath } end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index d832963292c..478f09a7881 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -112,9 +112,9 @@ describe 'Rack Attack global throttles' do arguments = { message: 'Rack_Attack', env: :throttle, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: get_args.first, + path: get_args.first, user_id: user.id, username: user.username } @@ -213,9 +213,9 @@ describe 'Rack Attack global throttles' do arguments = { message: 'Rack_Attack', env: :throttle, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: '/users/sign_in' + path: '/users/sign_in' } expect(Gitlab::AuthLogger).to receive(:error).with(arguments) @@ -377,9 +377,9 @@ describe 'Rack Attack global throttles' do arguments = { message: 'Rack_Attack', env: :throttle, - ip: '127.0.0.1', + remote_ip: '127.0.0.1', request_method: 'GET', - fullpath: '/dashboard/snippets', + path: '/dashboard/snippets', user_id: user.id, username: user.username } |