Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9088 closed defect (fixed)

[patch] Expire trac_auth_session cookie before calling LoginModule._do_logout

Reported by: janakj Owned by: hasienda
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 janakj 3 years ago.
Expire cookie trac_auth_session before calling LoginModule._do_logout

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by janakj

Expire cookie trac_auth_session before calling LoginModule._do_logout

comment:1 Changed 3 years ago by hasienda

(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 follow-up: Changed 3 years ago by hasienda

  • Keywords logout custom redirect added
  • Status changed from new to assigned
  • Summary changed from Expire trac_auth_session cookie before calling LoginModule._do_logout to [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 3 years ago by janakj

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

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

comment:5 Changed 3 years ago by hasienda

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

comment:8 Changed 3 years ago by hasienda

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

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
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.