django-axes
django-axes copied to clipboard
Do not record password field value in logs (security concern)
When we have failed attempts, I can see the POST data under the form data section. My concern is that one can form a pattern based on the user's attempts to log in. This can give insights on the password details. I'd like to only store the username and not the password field in the form data.
Please let me know how that can be achieved.
Thanks.
Hi @felixcheruiyot! GitHub issues are usually places to indicate and bring up problems, and to ideally propose solutions or fixes to those problems. It's usually not provided for free-of-charge support from package maintainers or users, though discussion on points of concern is also of course encouraged.
Thanks, @aleksihakli for pointing the need for collaboration.
The most likely solution would provide an optional list of post data items that I want to excempt somewhere here https://github.com/jazzband/django-axes/blob/e83a86b51ae5542f2308564c85373144c64860df/axes/handlers/database.py#L114 . I'll create a pull request early next week.
Thanks.
That seems like a good solution @felixcheruiyot! One option would be to include sanitation of the request in it's own method or function that is easy to parametrize and test, which would also allow it to be used from multiple places. A plain function in e.g. axes/helpers.py could be a good place for this?
That seems like a good solution @felixcheruiyot! One option would be to include sanitation of the request in its own method or function that is easy to parametrize and test, which would also allow it to be used from multiple places. A plain function in e.g.
axes/helpers.pycould be a good place for this?
That will work and will make it testable.
Is okay if we make this optional with this setting and default?
AXES_LOG_PASSWORD_ON_FAILURES = False
Of course, I'll reference the password field from AXES_PASSWORD_FORM_FIELD
This means an update to the documentation. Is that right?
Sounds good to me. I think the best approach is to open a PR with the idea and we can adjust the implementation from there if needed 👍
I started a PR for this ( #738 ) but then determined that the existing AXES_PASSWORD_FORM_FIELD really meets my needs. @felixcheruiyot, are you trying to remove multiple parameters? If so, that PR might be a start.
@mcoconnor I think the AXES_SENSITIVE_PARAMETERS approach will work. For now, I'm interested in only the password field, but other guys might want to remove others like security questions, etc. So I support your implementation approach on this issue.