ylecuyer / survey-gizmo-ruby

Gem to use the Survey Gizmo API
MIT License
23 stars 26 forks source link

Masking of api tokens #90

Closed mkaydev closed 7 years ago

mkaydev commented 7 years ago

I just noticed that the SurveyGizmo::Logger is passed as logger to the connection, too. It logs requested urls (info level).

If the api token or api token secret contain special characters that need percent-encoding, the regular expression which masks the tokens from log message will not match.

I verified that the tokens can contain such characters and that they would appear unmasked (and percent encoded) in the logs.

apurvis commented 7 years ago

can you give an example of such a token (obviously don't post your actual token)? All my SG tokens have been alphanumeric so far; haven't see a special character ever.

mkaydev commented 7 years ago

Any token, that contains a special character from the RFC 3986 reserved character list, e.g.

api_token = 'king_of_the&whirled$' api_token_secret = 'dream/word'

If these tokens would be passed as URL parameter, they would be percent encoded as:

api_token=king_of_the%26whirled%24 api_token_secret=dream%2Fword

So far, I got one such token.

In our fork[1] (for supporting multiple configurations) I have now added additional masking calls that match CGI.escape(api_token) and CGI.escape(api_token_secret).

[1] https://github.com/playtestcloud/survey-gizmo-ruby/commit/2032609cc95ff01b12ea42fdfcd3b208634aa1d8

apurvis commented 7 years ago

cool; i'll expect this to be resolved by a PR from you

apurvis commented 7 years ago

I scoped your branch for a second. Please put this commit https://github.com/playtestcloud/survey-gizmo-ruby/commit/f0ec4227eb98e2202310ff444c50118a35116515 on its own pull request because otherwise it will be impossible to review your changes.

mkaydev commented 7 years ago

Yup, that's what I thought. As you suspected, the changes for multiple configurations were extensive.

I created a separate branch fresh from master and made the change in a way that is compatible with the master of the original repository.

https://github.com/jarthod/survey-gizmo-ruby/pull/91