Opened 7 years ago

Closed 6 years ago

# Incorporate optional Single-Sign-On functionality

Reported by: Owned by: Steffen Hoffmann Steffen Hoffmann normal AccountManagerPlugin normal SSO login boftx@…, macjoost@…, Michael Renzmann, Ryan J Ollos, Erik M. Bray 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]


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.

### comment:1 Changed 7 years ago by Steffen Hoffmann

(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 7 years ago by Steffen Hoffmann

(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 7 years ago by Steffen Hoffmann

Status: new → 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 7 years ago by Steffen Hoffmann

(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 7 years ago by Steffen Hoffmann

[trac]


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 7 years ago by Steffen Hoffmann

(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 6 years ago by Steffen Hoffmann

(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 6 years ago by Steffen Hoffmann

Resolution: → fixed assigned → 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 6 years ago by Steffen Hoffmann

(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 6 years ago by Steffen Hoffmann

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

### comment:11 Changed 3 years ago by Ryan J Ollos

In 15086:

0.5dev: Replace dict lookup with equivalent property

Refs #9676.

### Modify Ticket

Change Properties