Modify

Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#9099 closed defect (fixed)

[PATCH] Expire trac_auth_session cookie whenever trac_auth cookie gets expired.

Reported by: Jan Janak Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: cookie lifetime
Cc: Trac Release: 0.12

Description

AccountManager with Trac 0.12 does not expire trac_auth_session cookie properly. When the user logs out, AccountManager module extends trac_auth_session cookie again, instead of expiring it. Hence the UA keeps resubmitting trac_auth_session with all requests until the next authentication attempt. That can confuse versions of AccountManager without the fix in #9095.

The problem is caused by auth.LoginModule._do_logout. AccountManager correctly expires trac_auth_session cookie in its own version of _do_logout and then calls auth.LoginModule._do_logout. That function attempts to obtain the value of req.authname which does not exists yet and that triggers AccountManager.authenticate() which internally calls _get_name_for_cookie which extends the previously expired trac_auth_session cookie again. Hence, trac_auth_session will be extended in the 302 response, instead of expired.

The attached patch fixes the issue by overriding function _expire_cookie in AccountManager. The overriding function expires trac_auth_session and then calls auth.LoginModule._expire_cookie. That ensures that both cookies get expired at the same time, i.e., after AccountManager.authenticate.

At the same time we remove the call to _expire_session_cookie from AccountManager._do_logout and because the function only calls its super method, the overriding method is no longer needed and can be removed.

Attachments (1)

expire_cookie.patch (1.8 KB) - added by Jan Janak 13 years ago.
Expires trac_auth_session when trac_auth is expired, i.e., after authentication

Download all attachments as: .zip

Change History (5)

Changed 13 years ago by Jan Janak

Attachment: expire_cookie.patch added

Expires trac_auth_session when trac_auth is expired, i.e., after authentication

comment:1 Changed 13 years ago by Steffen Hoffmann

Keywords: cookie lifetime added
Status: newassigned

Ok, looks sane, but will need to double-check for side-effects with Trac 0.11 too.

comment:2 Changed 13 years ago by Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

(In [10606]) AccountManagerPlugin: Another (final?) fix to session cookie handling, refs #8927, #9088, #9095 and #9099.

Overriding the supplemental method _expire_cookie() looks saner than overriding (super method) trac.auth.LoginModule._do_logout() and still calling it later on as well.

Thanks to janakj for another non-trivial contribution.

comment:3 Changed 13 years ago by Steffen Hoffmann

(In [10618]) AccountManagerPlugin: Publish maintenance release 0.3.2, closes #9051, #9082, #9088, #9091, #9092, #9093, #9095, #9099, #9107, #9108 and #9109.

This is an update for current stable at 0.3.1 with a number of fixes for issues reported within the last weeks.

While they will go into acct_mgr-0.4 too, current code isn't ready for release yet and will introduce a number of backwards-incompatible changes. So don't hurry for acct_mgr-0.4 right now.

Just noticed what I'd call a bug in signatures.py and removed unreasonable dependency on identical absolute path for successful check. Looks like nobody else tried this by now, right? Hey folks!

comment:4 Changed 12 years ago by Steffen Hoffmann

(In [11086]) AccountManagerPlugin: Ensure sensible browser cookie lifetime setting, refs #8927, #9088, #9095, #9099 and #9547.

I think this is now the most intuitive setting of default cookie lifetime: auth_cookie_lifetime from section [trac] gets overwritten with AcctMgr default (30 d) as long as it's found equal to the Trac default (0).

Remember, that the AcctMgr feature still has to be switched on with the boolean option persistent_sessions, that defaults to False, if unset.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.