587896 - Keep track of when users last logged in

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Administration, defect, P3)

Description

[Justin Scott [:fligtar]]

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.

Comment 1

[Wil Clouser [:clouserw]]

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.

Comment 2

[Wil Clouser [:clouserw]]

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.

Comment 3

[Barry Chen]

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?

Comment 4

[Wil Clouser [:clouserw]]

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.

Comment 5

[Wil Clouser [:clouserw]]

er, `users`

Comment 6

[Barry Chen]

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.

Comment 7

[Barry Chen]

http://github.com/chenba/zamboni/commit/07290d3075d108a998dff2148a03885df4640796

Changes based on jbalogh's feedback.  Scope and logic remains the same.

Comment 8

[Barry Chen]

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.

Comment 9

[Barry Chen]

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.

Comment 10

[Wil Clouser [:clouserw]]

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