aserafin / grape_logging

Request logging for Grape!
MIT License
147 stars 76 forks source link

Filtering for all loggers #59

Open borgand opened 5 years ago

borgand commented 5 years ago

Currently only FilterParameters logger exists to filter sensitive information from request parameters, but similarly sensitive information can also be contained in Request Headers as well as Response (e.g. the Authorization header, which could contain access token).

I have subclassed the FilterParameters logger for now, to achieve such result, but probably it would be best to move the filtering to Logger::Base.

  class FilterResponseLogger < GrapeLogging::Loggers::FilterParameters
    def parameters(_, response)
      response && response.respond_to?(:body) ? { response: clean_response(response) } : {}
    end

    private
    def clean_response(response)
      case response.header['Content-Type']
      when 'application/json'
        body = JSON.parse(response.body.first) #it's stored inside an array
        parameter_filter.filter(body).reject{ |key, _value| @exceptions.include?(key) }
      else
        # Better to not log, than log with sensitive data
        "FilterResponseLogger: Don't know how to filter '#{response.header['Content-Type']}' response, skipping."
      end
    rescue
      # Can't really log the error message here, as that could itself contain sensitive data
      # e.g. JSON.parse with invalid data will put whole data into the error message.
      "FilterResponseLogger: Error filtering response, skipping."
    end
  end

  class FilterRequestHeaderLogger < GrapeLogging::Loggers::FilterParameters
    HTTP_PREFIX = 'HTTP_'.freeze

    def parameters(request, _)
      headers = {}

      request.env.each_pair do |k, v|
        next unless k.to_s.start_with? HTTP_PREFIX

        k = k[HTTP_PREFIX.size..-1].split('_').each(&:capitalize!).join('-')
        headers[k] = v
      end

      { headers: clean_headers(headers) }
    end

    private
    def clean_headers(headers)
      parameter_filter.filter(headers).reject{ |key, _value| @exceptions.include?(key) }
    rescue
      # Can't really log the error message here, as that could itself contain sensitive data
      # e.g. JSON.parse with invalid data will put whole data into the error message.
      "FilterRequestHeaderLogger: Error filtering request headers, skipping."
    end
  end

What do you think?

toshe commented 5 years ago

I think it's a great idea. Can you do a pull request?

borgand commented 5 years ago

Hi, thanks for confirming the idea.

I'm a bit tied up both personally and professionally at the moment, so can't promise the PR any time soon.