Opened 16 months ago

Closed 13 months ago

Last modified 12 months ago

#21649 closed New feature (fixed)

Add session signing based on the value of the user's password

Reported by: timo Owned by: timo
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Add another value ('_auth_user_hash', '<some hash stuff>') based on the HMAC of the user's password hash to session verification. This will have the benefit of allowing a user to log out their other sessions by changing the password. Of course, it should not log out the current session.

Change History (9)

comment:1 Changed 16 months ago by timo

  • Has patch set

PR is up for review.

One thing I'd like feedback on is whether we should try to make this more backwards-compatible. Currently: "As a side effect of this change, users will be logged out when upgrading from an older version of Django."

comment:2 Changed 16 months ago by PaulM

I think in the past we've done work to preserve sessions across version upgrades. This of course has the downside of making the full benefit of the patch take an extra version to manifest. I personally don't mind sessions going away during upgrade, but I believe that some users of Django may not be comfortable with that.

I'd defer to Jacob or Luke or another conservative core dev. The upgrade code path shouldn't be too complex in this case.

comment:3 Changed 16 months ago by timo

  • Patch needs improvement set

Florian and Shai were both against losing sessions for this change. Shai suggested have a separate middleware for session signing & verification so that users who don't want this behavior can opt-out. I'll look into that approach.

comment:4 Changed 13 months ago by timo

  • Patch needs improvement unset

Here's an updated PR that uses the middleware approach suggested above.

comment:5 Changed 13 months ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 13 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In fd23c06023a0585ee743c0752dc94da66694cf63:

Fixed #21649 -- Added optional invalidation of sessions when user password changes.

Thanks Paul McMillan, Aymeric Augustin, and Erik Romijn for reviews.

comment:7 Changed 13 months ago by Tim Graham <timograham@…>

In 5891fd3f89337fc190cf671575407233440d2736:

[1.7.x] Fixed #21649 -- Added optional invalidation of sessions when user password changes.

Thanks Paul McMillan, Aymeric Augustin, and Erik Romijn for reviews.

Backport of fd23c06023 from master

comment:8 Changed 12 months ago by Tim Graham <timograham@…>

In 11e30b684d1a74bf7cc3b3bd22c0ffbdaa28f0a0:

Fixed a KeyError on login with legacy sessions; refs #21649.

Thanks Loic for the report.

comment:9 Changed 12 months ago by Tim Graham <timograham@…>

In 548acd77fd6356073ad4fa514c3d61f6589da43b:

[1.7.x] Fixed a KeyError on login with legacy sessions; refs #21649.

Thanks Loic for the report.

Backport of 11e30b684d from master

Note: See TracTickets for help on using tickets.
Back to Top