Modify

Opened 3 years ago

Closed 21 months ago

Last modified 20 months ago

#9676 closed enhancement (fixed)

Incorporate optional Single-Sign-On functionality

Reported by: hasienda Owned by: hasienda
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: SSO login
Cc: boftx@…, macjoost@…, otaku42, rjollos, ebray Trac Release: 0.11

Description

A configurable authentication cookie path has been the pre-requisite to share authentication cookies between several Trac environments on a single host. I call it "cheap" Single-Sign-On, because IMHO it's not as mature as true SSO solutions based on a dedicated authentication backend like Kerberos.

Anyway, as per request in t:#8486 a new configuration option, has been introduced to Trac with changeset t:changeset:9226:

[trac]
auth_cookie_path = /var/www/trac

Until now SharedCookieAuthPlugin has been the only Trac plugin to provide some "magic glue" for utilizing this option. I tested it and had the same issue as some other users. According to my half-educated code studies, this is bound to fail at least in any recent Trac due to the way, Trac core is handling authentication cookies internally (see my comment to #5566 for details).

Sadly the author has been unable to reproduce the issue and later explicitly dropped maintenance for all his plugins. 2 1/2 years have been gone so far without a bug-fix, not to mention an improved plugin version for Trac >= 0.12 without the monkey-patching of auth.LoginModule done in that plugin. Btw, AcctMgr monkey-patches auth.LoginModule too, and this may even contribute to the failure, at least in my case, but I've not done any closer investigation into this.

Nevertheless I'm eager to get a working solution, and after some work to resolve cookie-related AcctMgr issues I dropped SharedCookieAuthPlugin and made an attempted inside this plugin. Figure, that by the time we get this into a decent matured state, a decision could still be made on how to proceed with SharedCookieAuthPlugin.

Attachments (0)

Change History (10)

comment:1 Changed 3 years ago by hasienda

(In [11127]) AccountManagerPlugin: Allow authentication cookie sharing, refs #9676.

Participating Trac environments will accept and trust cookie data offered by
another Trac environment as related to an authenticated sessions.
Option 'auth_cookie_path' in section [trac] must be set to the same value
in all participating Trac environments simultaneously.

OTOH Trac environments with equal 'auth_cookie_path' value on the same host
must share their trac_auth cookie or sessions would get terminated
prematurely when moving between them.

comment:2 Changed 3 years ago by hasienda

(In [11128]) AccountManagerPlugin: Rework cookie path settings evaluation, refs #9676.

Make better use of existing _get_cookie_path() method (simplified) and
revoke distributed authentication data on logout now.
Environments with default value for auth_cookie_path or even without that
option (i.e. Trac 0.11) shouldn't share authentication data anymore.

comment:3 Changed 3 years ago by hasienda

  • Status changed from new to assigned

Initial implementation was amazingly easy. It should already work well for normal sessions. acct_mgr-r11127 dropped cookies when used together with persistent session on switching between Trac environments, and I expect this to be resolved with latest changes, but YMMV.

SSO should spring into life by setting an appropriate, equal value for auth_cookie_path in two or more Trac environments on the same host (domain).

Hint: an inherited trac.ini file is perfect for sharing these common setting and more.

Authentication of the other environments remains untouched.

Make sure, that

  • AccountManager's LoginModule is enabled in all these environments
  • authentication setup is equally trustworthy amongst them too
  • username have to be identical, but AuthStores don't need to and
  • password synchronization is neither provided nor required at all

Sometimes I've seen unexpected login results before radically cleaning existing trac_auth browser cookies. Not sure, how to handle that, but this seems like a one-time-effect and doesn't reoccur, as long as cookie path setting remains unchanged later on.

Independent test results are highly appreciated now. Especially I'd like to know about any noticeable impact on performance. I've neither done testing with a big number of sharing Trac environments nor with Trac 0.11 by now.

comment:4 Changed 3 years ago by hasienda

(In [11131]) AccountManagerPlugin: Reduce cookie update frequency, refs #9676.

For persistant sessions cookies get checked on every request.
They've been updated on every check, but this seems to cause unintended
cookie invalidation due to race condition on concurrent requests.

Some comments in Trac core session update code made me think about this.
After adding a time stap to trac_auth_session cookie and limiting the
update rate to once per hour dropped persistant sessions seem gone.

On a heavily used server this might even correspond to significatly reduced
request processing overhead. Only I had to adapt the authentication cookie
(hash) refresh rate, because it'll get triggered only once per hour now too
instead of twice per request, in practice even less.

comment:5 Changed 3 years ago by hasienda

Just noticed that I had

[trac]
auth_cookie_lifetime = 0

in one of the environments involved into testing persistent sessions. This might interfere with the AcctMgr setting

[account-manager]
persistent session = true

but I'll check later to be sure, and if so, I'll put a fat waring out on our cookbook page.

comment:6 Changed 3 years ago by hasienda

(In [11184]) AccountManagerPlugin: Ensure closing environments opened for auth data distribution, refs #9676.

I noticed log entries with number of repetitions per event growing over time,
especially in mixed setup containing both types of environments, some
configured to share authentication and one or more kept with regular,
non-shared authentication.

Web server restart reset this, but only to afterwards see it grow gain.
With this small change the implementation finally meets expectations.

comment:7 Changed 21 months ago by hasienda

(In [12378]) AccountManagerPlugin: Prevent SSO functionality from stumbling on db issues, refs #9676 and #10625.

I've been exploring seemingly random errors for a couple of months, starting
with the introduction of single-sign-on support code in r11127.
It turned out to be caused by (SQLite) db access issues concealed behind more
than a dozen unique trace-backs, many of them even not yielding a hint on
their connection to attempted/prior Trac db access at all.

Sure, the forced environment shutdown was a first good candidate to sort them
out, but only turning on Trac environment caching made Trac finally running
stable with the new SSO code in place.

As a follow-up on changeset [11131] I've futher reduced the session cookie
update frequency after taxing the probability of a session cookie exploit at
approximately 1:2*1038 per forged HTTP request, to reduce db burden by this
feature as well.

comment:8 Changed 21 months ago by hasienda

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

(In [12398]) AccountManagerPlugin: Releasing version 0.4, pushing development to acct_mgr-0.5dev.

Availability of that code as stable release
closes #874, #3459, #4677, #5295, #5691, #6616, #7577, #8076, #8685, #8770, #8791, #8990, #9052, #9079, #9090, #9139, #9246, #9252, #9547, #9618, #9676, #9843, #9852, #9940, #10023, #10028, #10123, #10142, #10204, #10276, #10397, #10412, #10594, #10625 and #10644.

Some more issues have been worked-on, yet without confirmed resolution,
refs #5464 (for JiraToTracIntegration), #8927 and #10134.

And finally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily,
refs #843, #1600, #5964, #8217, #8933.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

comment:9 Changed 21 months ago by hasienda

(In [12429]) AccountManagerPlugin: Finally fix db issues for SSO functionality, refs #9676 and #10625.

This is an urgent follow-up to [12378], altering another critical occurrence
of cross-environment Trac db access, that I've been overlooking before.

Thanks to ms4py for testing and hinting on this remaining issue.

comment:10 Changed 20 months ago by hasienda

(In [12482]) AccountManagerPlugin: Publish maintenance release 0.4.1, closes #5964, #8545, #10134, #10625, #10700 and #10701.

This is an update for current stable acct_mgr-0.4 with a number of fixes for
issues resolved within the last weeks, i.e.:

  • a final fix for Single-Sign-On functionality (refs #9676),
  • a long-standing HttpAuth login issue and
  • one for acct_mgr.LoginModule, that is relevant if used with web-servers, that evaluate the REMOTE_USER environment variable.

Changeset [12468] is included, that may require a Trac db fix-up.
Run python ./contrib/fix-session_attribute-failed_logins.py <env> once on any
Trac environment, that had account locking enabled with time constraints
before.

Add Comment

Modify Ticket

Action
as 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.