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

Consider logging user phone number and/or name less frequently (or not at all) #340

Closed
toolness opened this issue Oct 25, 2018 · 0 comments · Fixed by #341
Closed

Consider logging user phone number and/or name less frequently (or not at all) #340

toolness opened this issue Oct 25, 2018 · 0 comments · Fixed by #341

Comments

@toolness
Copy link
Collaborator

toolness commented Oct 25, 2018

Currently our logging output frequently mentions the phone number and/or name of our users, largely due to the fact that our JustfixUser.__str__() implementation includes them.

In the OWASP Logging Cheat Sheet, under "Data to exclude", it's mentioned that for "Non sensitive personal data (e.g. personal names, telephone numbers, email addresses)":

Consider using personal data de-identification techniques such as deletion, scrambling or pseudonymization of direct and indirect identifiers where the individual's identity is not required, or the risk is considered too great.

In the case of logging, I don't think it's especially required for us to mention the user's name and/or phone number; we could mention e.g. the user pk instead, and while it might make things a bit more annoying to read in some situations, it would still allow us to look up users in the admin (if e.g. we noticed a particular user was having lots of problems submitting a form).

For the most part this should just involve changing the implementation of JustfixUser.__str__(), and then changing a bunch of tests that rely on it (which would admittedly be annoying, especially since we can't really rely on a stable primary key during tests).

Update: Oh derp, we could just stringify the username, and set the username to a hash of the user's phone number instead of their actual phone number, which would de-identify it. And then tests could still rely on the stringification being stable too.

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 a pull request may close this issue.

1 participant