Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support logging filtering #86

Closed
agrberg opened this issue Oct 25, 2020 · 2 comments · Fixed by #87
Closed

Support logging filtering #86

agrberg opened this issue Oct 25, 2020 · 2 comments · Fixed by #87

Comments

@agrberg
Copy link
Collaborator

agrberg commented Oct 25, 2020

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
Copy link
Owner

dblock commented Oct 25, 2020

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
Copy link
Collaborator Author

agrberg commented Oct 25, 2020

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 #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants