Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9099 closed defect (fixed)

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

Reported by: janakj Owned by: hasienda
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 janakj 3 years ago.
Expires trac_auth_session when trac_auth is expired, i.e., after authentication

Download all attachments as: .zip

Change History (5)

Changed 3 years ago by janakj

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

comment:1 Changed 3 years ago by hasienda

  • Keywords cookie lifetime added
  • Status changed from new to assigned

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

comment:2 Changed 3 years ago by hasienda

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

(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 3 years ago by hasienda

(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 3 years ago by hasienda

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

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.