search

circleci / rollcage

A Clojure client for Rollbar
Eclipse Public License 1.0
50 stars 28 forks source link

Properly report ring request parameters #21

Open jeremyvdw opened 6 years ago

jeremyvdw commented 6 years ago

As of version 1.0.131, the parameters of the request is not reported to Rollbar.

From ring wiki, base request parameters look like this:

{:http-method :get
 :uri "/search"
 :query-string "q=clojure"}

It would then make sense that those are to be reported to rollcage client like:

{:url "/search"
 :params {:http-method :get
          :query-string "q=clojure"}

Also ring's wrap-parameters middleware adds three keys to the map: :query-params, :form-params and :params. As a large number of people are using this middleware, it sounds reasonable to add those as arguments sent to Rollbar.

gordonsyme commented 6 years ago

Thank you for the PR!

I do have some concerns about sending query string/parameters and form parameters in the general case, it's fairly common for those to contain secrets such as API tokens or other credentials.

jeremyvdw commented 6 years ago

@gordonsyme I do agree, that's a perfectly valid concern.

Official rollbar-gem tackle this issue with a configurable scrubber: https://github.com/rollbar/rollbar-gem/blob/efe6468af071f0a37751033dc9e35dbec224be6c/lib/rollbar/configuration.rb#L103

To keep changes minimal, I can propose to update :forms-params and :params to obfuscate values of a defined list of keys (i.e: :password, :password_confirmation, :secret, etc..).