django-axes icon indicating copy to clipboard operation
django-axes copied to clipboard

Do not record password field value in logs (security concern)

Open felixcheruiyot opened this issue 5 years ago • 7 comments

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.

felixcheruiyot avatar Oct 13 '20 04:10 felixcheruiyot

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.

aleksihakli avatar Nov 05 '20 18:11 aleksihakli

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.

felixcheruiyot avatar Nov 14 '20 09:11 felixcheruiyot

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?

aleksihakli avatar Nov 14 '20 12:11 aleksihakli

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.py could 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?

felixcheruiyot avatar Nov 14 '20 13:11 felixcheruiyot

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 👍

aleksihakli avatar Dec 02 '20 16:12 aleksihakli

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 avatar Apr 15 '21 00:04 mcoconnor

@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.

felixcheruiyot avatar Apr 29 '21 12:04 felixcheruiyot