One of my conditions for letting us delete most of our users in bug 564568 is that we start keeping track of the last time someone logged in. We should add that field soon.
This is a good idea. Other things to track as well: - timestamp of last attempted log in - IP address of last attempted log in - number of attempts since last successful log in - a `locked` field that is a timestamp, default to null CCing mcoates for any modifications or additions to the above. Mcoates: Let's leave out logic, algorithms and logging from this bug - this is just to implement db fields.
While you're in the user's app, what do you think of this one, chenb? If a user starts failing login's, I'd like to see it in the logs (attempts are already logged, but not the number of times). You can add the locked field, but you don't have to implement it yet. I'll file a follow up bug for that and we can figure out some implementation. Remember adding fields goes in the migrations/ directory.
Assignee: nobody → chenba
Sounds good. I'll add the fields to the `auth_user` table unless someone protests. @clouserw so according to comment #1 I should only add the fields? Not even start logging the log in attempts in them fields?
add fields to `user` please. auth_user is django's thing. You can do the simple stuff. Just don't worry about locking an account.
http://github.com/chenba/zamboni/commit/45f96290ebc7eb22c73c6794c5d2114d5a39632a Added four columns to `users`: `last_login_attempt`, `last_login_ip`, `failed_login_attempts`, and `locked`. `last_login_ip` is char(15) because that's what Django's IPAddressField use.
http://github.com/chenba/zamboni/commit/07290d3075d108a998dff2148a03885df4640796 Changes based on jbalogh's feedback. Scope and logic remains the same.
http://github.com/chenba/zamboni/commit/f07cf3b05f225959f8070a82de1ec11e6d59b6b6 Added the `last_login_attempt_ip` column that clouserw wanted. Also bumped the failed attempts field to an unsigned MEDIUMINT. Hopefully 16777215 is high enough. :) Boundary is still checked. The IP address fields are now IPv6 friendly, as CHAR(45). The user login debug logging is now in the log_login_attempt method. However, this does mean that if the username is incorrect, the login attempt is not logged, since there's no UserProfile instance.
http://github.com/chenba/zamboni/commit/7b10c325e2cb8b66465e0193383706c712370d6a No longer piggyback on jbalogh's user login tests in a lame fashion. Actual tests now check all four fields. Thanks for pointing that out.
Thanks chenb! http://github.com/jbalogh/zamboni/commit/bb1cc255001cc5c116cfdeec7dac13249667c712 QA: if you want to test this, you'll have to look in the db at the new columns
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.