dblock / iex-ruby-client

IEX Finance API Ruby Client
MIT License
118 stars 60 forks source link

Support logging filtering #86

Closed agrberg closed 3 years ago

agrberg commented 3 years ago

Some of the IEX endpoints use the secret token and it will be logged if you've enabled the logger for Faraday (config.logger is set or passed in when you create an instance of the IEX::Api::Client).

I'd like to filter these tokens out of the logs which can be done with a hard code change to https://github.com/dblock/iex-ruby-client/blob/ca706ff/lib/iex/cloud/connection.rb#L33 to something like:

if logger
  connection.response(:logger, logger) do |logger|
    logger.filter /\btoken=T?[sp]k_\w+/i, 'token=[REMOVED]' # https://regexper.com/#%2F%5Cbtoken%3DT%3F%5Bsp%5Dk_%5Cw%2B%2Fi
  end
end

in order to filter out the secret key. I'm not sure if this software is intended to be this opinionated so another option is to enable it to be configurable. Is there a recommended approach? My first instinct to ensure backward compatibility is to allow the config logger to be either a logger instance or hash or with keys in %i[instance options proc]. Then that code could be changed to:

if logger
  if logger.is_a?(Hash)
    connection.response :logger, logger[:instance], logger[:options] || {}, &logger[:proc]
  else
    connection.response :logger, logger
  end
end

Perhaps there's a better solution where we can expose the logger but I'm not sure.

For my usage in a Rails app I am able to work around this with the following:

    original_formatter = Rails.logger.formatter
    config.logger = Rails.logger.dup
    config.logger.formatter = ->(severity, datetime, progname, msg) do
      filtered_message = msg.gsub(/\btoken=T?[sp]k_\w+/i, 'token=[REMOVED]') # https://regexper.com/#%2F%5Cbtoken%3DT%3F%5Bsp%5Dk_%5Cw%2B%2Fi
      original_formatter.call(severity, datetime, progname, filtered_message.dump)
    end

Faraday information from docs

dblock commented 3 years ago

This is really an issue for IEX to help solve, we need to take the token out of the query string and into an HTTP header. Have you tried contacting them about it?

I think for the purposes of this library I am A-OK with the first proposal as long as someone can restore the old behavior by assigning a logger or doing something else in the configuration. Want to try a PR?

agrberg commented 3 years ago

I added a request for sending tokens via HTTP header to https://iexcloud.io/console/roadmap but I was unable to see the issue after submission. Perhaps it needs to be accepted but if I find our more I'll post a link here.

Added the approach I suggested in https://github.com/dblock/iex-ruby-client/pull/87. I think the test might be a little brittle but I also anticipate Faraday and its logger middleware changing less frequently than this project.