[PLAT-319] Redact log (#28)

* Add configurable secret keywords * add proper testing tools, add test for adding different types of sensitive keywords * wip * Add `sensitive_keywords` info to documentation * Remove extra args * Fix tests * Remove .github dir from gitignore * Reintroduce replace_keys args It is used in another context with a custom value to redact headers * Testing nested keys * Extract redacting to a `Redactor`, override `PierLogging::Logger`s log method * Test redactor for hash * Move require declaration to Redactor class * Test redaction for arrays * Fix identation * Double quotes strings * Rename var for clarity * Redacting all params Ougai is very flexible and data can be in any position. * Add logger test without assertions * Add assertions * Refactor test * Move sensitive_keyword to LoggerConfiguration * Override `_log` not `log`, fix Heisembug * version bump and fix log readability * Add squad to list of authors * Remove unused test * remove extraneous redaction from request logger, base logger already redacts everything * Bump version Co-authored-by: Leonardo Bighetti <[email protected]> Co-authored-by: Bruno Arakaki <[email protected]>
pier-digital · Jun 2, 2021 · 8742fb7 · 8742fb7
1 parent 33bb767
commit 8742fb7
Show file tree

Hide file tree

Showing 11 changed files with 205 additions and 73 deletions.
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,10 +1,10 @@
 PATH
   remote: .
   specs:
-    pier_logging (0.3.3)
-      awesome_print
+    pier_logging (0.4.0)
+      amazing_print
       facets
-      ougai
+      ougai (>= 2.0.0)
       rails
 
 GEM
@@ -69,7 +69,7 @@ GEM
       minitest (>= 5.1)
       tzinfo (~> 2.0)
       zeitwerk (~> 2.3)
-    awesome_print (1.9.2)
+    amazing_print (1.3.0)
     builder (3.2.4)
     byebug (11.1.3)
     concurrent-ruby (1.1.8)
@@ -92,7 +92,7 @@ GEM
     minitest (5.14.4)
     mocha (1.12.0)
     nio4r (2.5.7)
-    nokogiri (1.11.5)
+    nokogiri (1.11.6)
       mini_portile2 (~> 2.5.0)
       racc (~> 1.4)
     oj (3.11.5)
@@ -145,7 +145,7 @@ GEM
     thor (1.1.0)
     tzinfo (2.0.4)
       concurrent-ruby (~> 1.0)
-    websocket-driver (0.7.3)
+    websocket-driver (0.7.4)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
     zeitwerk (2.4.2)

diff --git a/lib/pier_logging.rb b/lib/pier_logging.rb
@@ -1,6 +1,6 @@
 require "rails"
 require "ougai"
-require "awesome_print"
+require "amazing_print"
 require "facets/hash/traverse"
 require "pier_logging/version"
 require "pier_logging/logger"
@@ -10,6 +10,7 @@
 require "pier_logging/formatter/readable"
 require "pier_logging/helpers/headers"
 require "pier_logging/helpers/env_config"
+require "pier_logging/helpers/redactor"
 
 module PierLogging
   def self.logger_configuration
@@ -29,12 +30,13 @@ def self.configure_request_logger
   end
 
   class LoggerConfiguration
-    attr_reader :app_name, :env, :formatter
+    attr_reader :app_name, :env, :formatter, :sensitive_keywords
 
     def initialize
       @app_name = nil
       @env = nil
       @formatter = Formatter::Json.new
+      @sensitive_keywords = []
     end
 
     def app_name=(app_name)
@@ -51,11 +53,22 @@ def formatter=(formatter)
       raise ArgumentError, "Config 'formatter' must be a 'Ougai::Formatters::Base'" unless formatter.is_a?(Ougai::Formatters::Base)
       @formatter = formatter
     end
+
+    def sensitive_keywords=(keywords)
+      keywords.map! do |kw|
+        if kw.is_a? Regexp
+          kw
+        else
+          Regexp.new(kw.to_s)
+        end
+      end
+      @sensitive_keywords += keywords
+    end
   end
 
   class RequestLoggerConfiguration
     attr_reader :enabled, :user_info_getter, :hide_request_body_for_paths, :hide_response_body_for_paths,
-                :log_request_body, :log_response, :hide_request_headers, :correlation_id_getter, :sensitive_keywords
+                :log_request_body, :log_response, :hide_request_headers, :correlation_id_getter
 
     def initialize
       @user_info_getter = ->(_ = nil) { nil }

diff --git a/lib/pier_logging/helpers/redactor.rb b/lib/pier_logging/helpers/redactor.rb
@@ -0,0 +1,62 @@
+# Requiring only the part that we need
+require 'facets/hash/traverse'
+
+module PierLogging
+  module Helpers
+    class Redactor
+      REDACT_REPLACE_KEYS = [
+        /passw(or)?d/i,
+        /^pw$/,
+        /^pass$/i,
+        /secret/i,
+        /token/i,
+        /api[-._]?key/i,
+        /session[-._]?id/i,
+        /^connect\.sid$/
+      ].freeze
+      REDACT_REPLACE_BY = '*'.freeze
+
+      class << self
+        def redact(obj, replace_keys = nil, replace_by = REDACT_REPLACE_BY)
+          replace_keys ||= sensitive_keywords
+          if obj.is_a?(Array)
+            redact_array(obj, replace_keys, replace_by)
+          elsif obj.is_a?(Hash)
+            redact_hash(obj, replace_keys, replace_by)
+          elsif obj.respond_to?(:to_hash)
+            redact_hash(obj.to_hash, replace_keys, replace_by)
+          else
+            obj
+          end
+        end
+
+        private
+
+        def sensitive_keywords
+            REDACT_REPLACE_KEYS + PierLogging.logger_configuration.sensitive_keywords
+        end
+
+        def redact_array(arr, replace_keys, replace_by = REDACT_REPLACE_BY)
+          raise StandardError, 'Could not redact_array for non-array objects' unless arr.is_a? Array
+          arr.map { |el| redact(el, replace_keys, replace_by) }
+        end
+
+        def redact_hash(hash, replace_keys, replace_by = REDACT_REPLACE_BY)
+          raise StandardError, 'Could not redact_hash for non-hash objects' unless hash.is_a? Hash
+          hash.traverse do |k,v|
+            should_redact = replace_keys.any?{ |regex| k =~ regex }
+            if (should_redact)
+              [k, replace_by]
+            else
+              case v
+              when Array then [k, redact_array(v, replace_keys, replace_by)]
+              else
+                [k, v]
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/pier_logging/logger.rb b/lib/pier_logging/logger.rb
@@ -12,5 +12,16 @@ def initialize(*args)
     def create_formatter
       PierLogging.logger_configuration.formatter
     end
+
+    def _log(severity, *args)
+      redacted_args = redact_data(args)
+      super(severity, *redacted_args)
+    end
+
+    private
+
+    def redact_data(data)
+      PierLogging::Helpers::Redactor.redact(data)
+    end
   end
-end
+end
diff --git a/lib/pier_logging/request_logger.rb b/lib/pier_logging/request_logger.rb
@@ -1,19 +1,5 @@
-# Requiring only the part that we need
-require 'facets/hash/traverse'
 module PierLogging
   class RequestLogger
-    REDACT_REPLACE_KEYS = [
-      /passw(or)?d/i,
-      /^pw$/,
-      /^pass$/i,
-      /secret/i,
-      /token/i,
-      /api[-._]?key/i,
-      /session[-._]?id/i,
-      /^connect\.sid$/
-    ].freeze
-    REDACT_REPLACE_BY = '*'.freeze
-
     attr_reader :logger
 
     def initialize(app, logger = PierLogging::Logger.new(STDOUT))
@@ -44,7 +30,7 @@ def log(args)
       env, status, type, body, starts_at, ends_at, _ = args
       request = Rack::Request.new(env)
       request_headers = get_request_headers_from_env(env)
-      logger.info redact_object({
+      info = {
         message: build_message_from_request(request),
         type: 'http',
         duration: ((ends_at - starts_at)*1000).to_i,
@@ -64,7 +50,8 @@ def log(args)
           body: response_body(request.path, body),
           type: type['Content-Type'],
         }
-      })
+      }
+      logger.info info
     rescue StandardError => error
       # We should never fall in this part as the only errors that could result in this are errors
       # in our logger (inside this same method)
@@ -78,7 +65,7 @@ def get_request_headers_from_env(env)
       headers = env.select { |k,v| k[0..4] == 'HTTP_'}.
         transform_keys { |k| k[5..-1].split('_').join('-').upcase }
 
-      return redact_object(headers, hide_request_headers, nil) if hide_request_headers.present?
+      return PierLogging::Helpers::Redactor.redact(headers, hide_request_headers, nil) if hide_request_headers.present?
 
       headers
     end
@@ -129,44 +116,6 @@ def get_body_object(body)
       body
     end
 
-    def sensitive_keywords
-      REDACT_REPLACE_KEYS + PierLogging.request_logger_configuration.sensitive_keywords
-    end
-
-    def redact_object(obj, replace_keys = nil, replace_by = REDACT_REPLACE_BY)
-      replace_keys ||= sensitive_keywords
-      if obj === Array
-        redact_array(obj, replace_keys, replace_by)
-      elsif obj === Hash
-        redact_hash(obj, replace_keys, replace_by)
-      elsif obj.respond_to?(:to_hash)
-        redact_hash(obj.to_hash, replace_keys, replace_by)
-      else
-        obj
-      end
-    end
-
-    def redact_array(arr, replace_keys, replace_by = REDACT_REPLACE_BY)
-      raise StandardError, 'Could not redact_array for non-array objects' unless arr.is_a? Array
-      arr.map { |el| redact_object(el, replace_keys, replace_by) }
-    end
-
-    def redact_hash(hash, replace_keys, replace_by = REDACT_REPLACE_BY)
-      raise StandardError, 'Could not redact_hash for non-hash objects' unless hash.is_a? Hash
-      hash.traverse do |k,v|
-        should_redact = replace_keys.any?{ |regex| k =~ regex }
-        if (should_redact)
-          [k, replace_by]
-        else
-          case v
-          when Array then [k, redact_array(v, replace_keys, replace_by)]
-          else
-            [k, v]
-          end
-        end
-      end
-    end
-
     def determine_body_from_exception(exception)
       { message: exception.message }
     end

diff --git a/lib/pier_logging/version.rb b/lib/pier_logging/version.rb
@@ -1,3 +1,3 @@
 module PierLogging
-  VERSION = "0.3.3"
+  VERSION = "0.4.1"
 end
diff --git a/pier_logging.gemspec b/pier_logging.gemspec
@@ -6,8 +6,8 @@ require "pier_logging/version"
 Gem::Specification.new do |spec|
   spec.name          = "pier_logging"
   spec.version       = PierLogging::VERSION
-  spec.authors       = ["Mauricio Banduk"]
-  spec.email         = ["[email protected]"]
+  spec.authors       = ["Mauricio Banduk", "Bruno Arakaki", "Leonardo Bighetti", "Tiago Macedo"]
+  spec.email         = ["[email protected]", "[email protected]", "[email protected]", "[email protected]"]
 
   spec.summary       = %q{Structured log used on Pier Applications}
   spec.description   = %q{Defines a basic structure for general and request logging}
@@ -23,8 +23,8 @@ Gem::Specification.new do |spec|
   spec.executables   = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
   spec.require_paths = ["lib"]
 
-  spec.add_dependency "ougai"
-  spec.add_dependency "awesome_print"
+  spec.add_dependency "ougai", ">=2.0.0"
+  spec.add_dependency "amazing_print"
   spec.add_dependency "rails"
   spec.add_dependency "facets"
 

diff --git a/test/pier_logging/helper/redactor_test.rb b/test/pier_logging/helper/redactor_test.rb
@@ -0,0 +1,44 @@
+require "test_helper"
+
+class PierLogging::Helpers::RedactorTest < Minitest::Test
+  subject { PierLogging::Helpers::Redactor }
+
+  context ".redact" do
+    setup do
+      PierLogging.logger_configuration.sensitive_keywords = [:sensitive_key]
+    end
+    context "with a hash" do
+      setup do
+        @hash = {
+          sensitive_key: "Amar é fogo que arde sem se ver",
+          not_sensitive: "Vai Rabetão tão tão no chão",
+          password: "Que não seja imortal, posto que é chama"
+        }
+      end
+      should "redact only sensitive stuff" do
+        response = subject.redact(@hash)
+
+        assert_equal "*", response[:sensitive_key]
+        assert_equal "Vai Rabetão tão tão no chão", response[:not_sensitive]
+        assert_equal "*", response[:password]
+      end
+    end
+
+    context "with an array of hashs" do
+      setup do
+        @array = [
+          {sensitive_key: "Amar é fogo que arde sem se ver"},
+          {not_sensitive: "Vai Rabetão tão tão no chão"},
+          {password: "Que não seja imortal, posto que é chama"}
+        ]
+      end
+      should "redact only sensitive stuff" do
+        response = subject.redact(@array)
+
+        assert_equal "*", response[0][:sensitive_key]
+        assert_equal "Vai Rabetão tão tão no chão", response[1][:not_sensitive]
+        assert_equal "*", response[2][:password]
+      end
+    end
+  end
+end
diff --git a/...ging/request_logger_configuration_test.rb → ...pier_logging/logger_configuration_test.rb b/...ging/request_logger_configuration_test.rb → ...pier_logging/logger_configuration_test.rb
@@ -1,7 +1,7 @@
 require "test_helper"
 
-class PierLogging::RequestLoggerConfigurationTest < Minitest::Test
-  subject { PierLogging::RequestLoggerConfiguration.new }
+class PierLogging::LoggerConfigurationTest < Minitest::Test
+  subject { PierLogging::LoggerConfiguration.new }
 
   context "#sensitive_keywords" do
     should 'transform strings to regexps when adding them' do