Modify

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#9088 closed defect (fixed)

[patch] Expire trac_auth_session cookie before calling LoginModule._do_logout

Reported by: Jan Janak Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: logout custom redirect
Cc: Trac Release: 0.12

Description

If the user enables metanav.logout.redirect in the configuration file then the function LoginModule._do_logout never returns--it performs the redirect internally.

Therefore cookie 'trac_auth_session' needs to be expired before calling the function.

Attachments (1)

expire_trac_auth_session.patch (1.1 KB) - added by Jan Janak 6 years ago.
Expire cookie trac_auth_session before calling LoginModule._do_logout

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Jan Janak

Expire cookie trac_auth_session before calling LoginModule._do_logout

comment:1 Changed 6 years ago by Steffen Hoffmann

(In [10591]) AccountManagerPlugin: Expire session cookie before calling _do_logout(), refs #9088.

We need to do cookie expiration before, since this method will redirect internally and never return, if the logout.redirect option is set in metanav section of trac.ini (available since Trac 0.12).

comment:2 Changed 6 years ago by Steffen Hoffmann

Keywords: logout custom redirect added
Status: newassigned
Summary: Expire trac_auth_session cookie before calling LoginModule._do_logout[patch] Expire trac_auth_session cookie before calling LoginModule._do_logout

Ah, I forgot to give you proper credit in the commit message. Sorry, no done by intention, just in a rush to fix it up. Thanks for spotting this and reporting here anyway.

comment:3 in reply to:  2 Changed 6 years ago by Jan Janak

Replying to hasienda:

Ah, I forgot to give you proper credit in the commit message. Sorry, no done by intention, just in a rush to fix it up. Thanks for spotting this and reporting here anyway.

No worries, thanks for a quick commit!

By the way, cookie trac_auth_session still does not get expired properly. Instead of being expired in _do_logout, the cookie gets extended again when the user logs out. I figured that the code that extends it inside _get_name_for_cookie gets called somehow, probably after _do_logout.

I have been looking into it but I haven't found the exact cause yet.

comment:4 Changed 6 years ago by Jan Janak

Resolution: fixed
Status: assignedclosed

comment:5 Changed 6 years ago by Steffen Hoffmann

(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:6 Changed 6 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:7 Changed 5 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.

comment:8 Changed 5 years ago by Steffen Hoffmann

(In [11100]) AccountManagerPlugin: Refresh cookie value, refs #9088 and #9547.

New option cookie_refresh_pct determines, how often cookie ID's will get changed on average.

Modify Ticket

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

Add Comment


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

 
Note: See TracTickets for help on using tickets.