make trac.web.auth.LoginModule cookie path configurable

[jhammel@…]

Currently, the LoginModule sets (and deletes) a cookie on req.base_path:

        req.outcookie['trac_auth'] = cookie
        req.outcookie['trac_auth']['path'] = req.base_path or '/'

    def _expire_cookie(self, req):
        """Instruct the user agent to drop the auth cookie by setting the
        "expires" property to a date in the past.
        """
        req.outcookie['trac_auth'] = ''
        req.outcookie['trac_auth']['path'] = req.base_path or '/'
        req.outcookie['trac_auth']['expires'] = -10000
        if self.env.secure_cookies:
            req.outcookie['trac_auth']['secure'] = True

It would be nice for this to be configurable, so that, optionally the cookie path could be specified as an Option with the current behavior being the fallback if the Option is not set.

The reason I would like this is so that cookies can be parseable via multiple Trac environments on the same server, optionally. Currently, (in the soon to be released ​SharedCookieAuthPlugin) I work around this with a monkey-patch:

class GenericObject(object):
    def __init__(self, **kw):
        for key, item in kw.items():
            setattr(self, key, item)

def _do_login(self, req):
    kw = [ 'incookie', 'remote_user', 'authname', 
           'remote_addr', 'outcookie' ]
    kw = dict([ (i, getattr(req, i)) for i in kw ])
    kw['base_path'] = '/'
    fake_req = GenericObject(**kw)
    auth_login_module_do_login(self, fake_req)

[Remy Blank]

I'm not sure I understand the use case. Isn't the cookie a randomly-generated identifier that only makes sense in the context of a specific Trac instance?

[anonymous]

Replying to rblank:

I'm not sure I understand the use case. Isn't the cookie a randomly-generated identifier that only makes sense in the context of a specific Trac instance?

see ​SharedCookieAuthPlugin.

There are two basic cases of interest (probably more, but let's start with two):

1. cookies are associated with a single environment; that is, you want different auth for different Trac instances
2. cookies are readable across environments on the same server; that is, if you log into trac.example.com/foo you are also logged into trac.example.com/bar (single-sign on)

SharedCookieAuthPlugin is a very hacky way of doing this. On the other hand, it works, consumes existing code, and should be secure. I welcome better SSO solutions that are easily deployable, pluggable, and exist within the Trac framework and don't rely on other (usually less deployable) software.

[ebray]

+1 to this. We use a single-sign on auth system for Trac, but still have to modify trac.web.auth.LoginModule to set the cookie path to / so that it can work without fuss with other plugins that use it, such as ​AccountManagerPlugin.

[Christian Boos]

Patch welcomed, then.

[Jeff Hammel <jhammel@…>]

patch against 0.11-stable

[Jeff Hammel <jhammel@…>]

Replying to cboos:

Patch welcomed, then.

attached, login_cookie_path.r8732.diff​

[Christian Boos]

Thanks!

Two minor nits:

• instead of adding: from trac.config import Option , modify the previous import, add Option after BoolOption
• more important, the docstring is a bit scarce; without being too long, it should nevertheless give a hint about why someone should ever want to change the default.

[Christian Boos]

Well, feel free to update the patch as requested.

[jhammel@…]

revised patch vs 0.11-stable

[anonymous]

Replying to cboos:

Well, feel free to update the patch as requested.

Done, login_cookie_path.r8885.diff​.

If the docstring isn't adequate, suggestions welcome

[Remy Blank]

Slightly adapted patch applied in [9226], and named the option auth_cookie_path to be consistent with auth_cookie_lifetime.

[Steffen Hoffmann]

I've been unsuccessful in seeking to resolve an ​issue rendering ​th:SharedCookieAuthPlugin useless in recent Trac applications (Trac 0.13dev-r10883 tested, but should apply at least down to Trac 0.12 as well).

After many hours of code studies and testing I've dropped that approach and created an alternative solution instead, on-top of ​th:AccountManagerPlugin. This effort is tracked in ​th:#9676 now. Any feedback welcome.

[Ryan J Ollos]

See also #12251 and #12257.