hackmcgill / dashboard

🐥 McHacks dashboard
https://app.mchacks.ca
MIT License
30 stars 9 forks source link

feat: implement sentry #951

Closed jamesxu123 closed 3 years ago

jamesxu123 commented 3 years ago

Tickets:

Type of change:

Please delete options that aren't relevant.

How did you do this?

How to test:

Questions:

PR Checklist:

Screenshots:

close #948

krubenok commented 3 years ago

What privacy implications does Sentry introduce? Presumably it drops cookies and allows linking errors to a particular end user? Does Sentry receive personally identifiable information, or just abstract identifiers that we'd need to correlate with the database to get names/info?

Would be good to write this up in the docs somewhere and add it to the privacy policy page.

jamesxu123 commented 3 years ago

That's a good point. Quick inspection of the Sentry dashboard shows that PII is limited to IP address and browser/platform details. We can turn off IP logging and I don't think knowing IP addresses helps with debugging anyways. However, it may be useful to determine if a problem is occurring with one user or not.

Default scrubbing should remove things like passwords/credit cards and similarly sensitive things.

E.g. image

Other non-sensitive web requests may be logged, but we would have access to it anyways from hackerAPI so I don't think this changes our privacy policies much?

Let me know what you think.

jamesxu123 commented 3 years ago

Also unfortunately sentry seems to have broken react-toastify. Not sure what is causing this but it will need some investigation. image

krubenok commented 3 years ago

Default scrubbing should remove things like passwords/credit cards and similarly sensitive things.

PII (at least in my experience) includes Names, emails, and basically any other user generated content. If we're sharing that kind of stuff with sentry it should be called out in our privacy policy as an additional data handler. Food for thought to actually consider who gets what data when they sign up for an account and use the system.

jamesxu123 commented 3 years ago

Could we edit the privacy policy to read

In order to participate at McHacks, we need to collect a minimum of information in order to properly run the event. Information will only be used within the services that McHacks utilizes for the Application Dashboard. Such services might include but are not limited to instances of a MongoDB server, Netlify, Google Cloud Platform, Heroku, and Sentry.

Does that address the sharing of PII with Sentry?

krubenok commented 3 years ago

I think that's part of it. I haven't spent enough time in Sentry to actual evaluate how they process the data and if the settings are appropriate. So long as it's being actively considered and they're getting the minimum data for effective troubleshooting it's probably fine. Just asking to be intentional about who gets what data and who has access to that data.

jamesxu123 commented 3 years ago

Great, sounds good. I'll make a PR to edit the privacy policy for the static site then.

Edit: to clarify I believe sentry has rather strict rules about data protection on their end. On our side, access to this data would probably be limited to dev team members who would have access to the database anyways.

krubenok commented 3 years ago

FWIW, there should probably be a link to the relevant privacy policy on the dashboard site somewhere (or just merge the two sites into one 🤷 )