rollcage icon indicating copy to clipboard operation
rollcage copied to clipboard

Properly report ring request parameters

Open jeremyvdw opened this issue 7 years ago • 2 comments

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.

jeremyvdw avatar Nov 21 '17 15:11 jeremyvdw

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.

gordonsyme avatar Nov 22 '17 22:11 gordonsyme

@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..).

jeremyvdw avatar Nov 23 '17 09:11 jeremyvdw