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

fix: Sanitize raven breadcrumbs too #1193

Merged
merged 1 commit into from
Jul 6, 2018
Merged

fix: Sanitize raven breadcrumbs too #1193

merged 1 commit into from
Jul 6, 2018

Conversation

rjmackay
Copy link
Contributor

@rjmackay rjmackay commented Jul 5, 2018

This pull request makes the following changes:

  • Sanitize breadcrumbs too
  • Clean up regex to sanitize all options at once

Testing checklist:

  • Configure your client deployment in config.js with ravenUrl: 'https://407e538550a64857ac099f98ac8cceef@sentry.io/88046'

  • Open the client

  • Move around the UI and generate errors (data view has a few noisey errors)

  • Log in as a normal user (not admin)

  • Move around the UI and generate errors (data view has a few noisey errors)

  • Check Sentry errors in dashboard now show your user id

  • Log out

  • Trigger errors on the client and watch for sentry HTTP requests

  • Check Sentry errors in dashboard no longer show your user id

  • Check data sent to sentry.io in network tab does not contain sensitive info

    • The simplest issue to look for is the 'unhandled rejection' for 403's when trying to load users/contacts/etc without admin privileges
  • Check Sentry dashboard and confirm logged errors do not contain sensitive info

    • The simplest issue to look for is the 'unhandled rejection' for 403's when trying to load users/contacts/etc without admin privileges
  • I certify that I ran my checklist

Refs ushahidi/platform#1596

Ping @ushahidi/platform

Copy link
Contributor

@kinstella kinstella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and new forEachs seem cleaner. I tested this again, and I can't find Authorization or client_secret anywhere, (though I'm not 100% sure I'm looking in all the right places -- obviously, feel free to ping me for a retest if you want).

@rjmackay rjmackay merged commit 6c0d2e8 into master Jul 6, 2018
@tuxpiper tuxpiper deleted the 1596-ush-001 branch March 24, 2021 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants