auth_cookie_lifetime seems to be ignored

[vpenne@…]

(With TracAccountManager==0.4.4)

For safety, I want users to be automatically logged off after some inactivity (for example, 10 minutes).

I set auth_cookie_lifetime to 600 or any other positive value, but it doesn't seem to be taken in account. Even after hours of inactivity, a logged on user will remain logged on.

It seems only restarting the browser or clicking on "Logout" actually log off.

[Ryan J Ollos]

I haven't tried to reproduce with AccountManagerPlugin yet, but the functionality works correctly in Trac using htpasswd authentication.

Could you confirm that you have trac.web.auth.LoginModule disabled in your [components] section? Would you kindly post your [accountmanager] section from trac.ini? I will try to reproduce the issue with your configuration.

[anonymous]

Hi,

here it is :

[account-manager]
authentication_url = 
db_htdigest_realm = 
force_passwd_change = true
hash_method = HtDigestHashMethod
htdigest_file = 
htdigest_realm = 
htpasswd_file = /var/local/trac/passwords
htpasswd_hash_type = crypt
password_file = 
password_store = HtPasswdStore
persistent_sessions = False
refresh_passwd = False
verify_email = False

[components]
acct_mgr.admin.accountmanageradminpanel = enabled
acct_mgr.api.accountmanager = enabled
acct_mgr.db.sessionstore = enabled
acct_mgr.guard.accountguard = enabled
acct_mgr.htfile.htdigeststore = enabled
acct_mgr.htfile.htpasswdstore = enabled
acct_mgr.http.httpauthstore = enabled
acct_mgr.macros.accountmanagerwikimacros = enabled
acct_mgr.notification.accountchangelistener = enabled
acct_mgr.notification.accountchangenotificationadminpanel = enabled
acct_mgr.pwhash.htdigesthashmethod = enabled
acct_mgr.pwhash.htpasswdhashmethod = enabled
acct_mgr.register.basiccheck = enabled
acct_mgr.register.bottrapcheck = enabled
acct_mgr.register.emailcheck = enabled
acct_mgr.register.emailverificationmodule = enabled
acct_mgr.register.regexpcheck = enabled
acct_mgr.register.registrationmodule = disabled
acct_mgr.register.usernamepermcheck = enabled
acct_mgr.svnserve.svnservepasswordstore = enabled
acct_mgr.web_ui.accountmodule = enabled
acct_mgr.web_ui.loginmodule = enabled
acct_mgr.web_ui.resetpwstore = enabled
includemacro.macros.includemacro = enabled
mastertickets.api.masterticketssystem = enabled
mastertickets.web_ui.masterticketsmodule = enabled
odtexport.* = enabled
permredirect.* = enabled
roadmapplugin.roadmap.filterroadmap = disabled
roadmapplugin.roadmap.sortroadmap = disabled
trac.web.auth.loginmodule = disabled
tractoc.macro.tocmacro = enabled
tracwysiwyg.* = enabled
wikinotification.* = enabled
ctxtnavadd.* = enabled

As you can see trac.web.auth.loginmodule is indeed disabled.

Thanks

[Jun Omae]

For safety, I want users to be automatically logged off after some inactivity (for example, 10 minutes).

The value of auth_cookie_lifetime is used only for expire attribute of the cookie at ​trac:source:tags/trac-1.0.6/trac/web/auth.py@:190-191#L182. Changes of the option wouldn't lead expiration of the cookies which have been sent to the users.

You should delete auth_cookie records to forcibly log out.

[Jun Omae]

Also, that is not a AccountManagerPlugin issue.

[Vincent Penne <ziggy@…>]

Replying to jun66j5:

The value of auth_cookie_lifetime is used only for expire attribute of the cookie at ​trac:source:tags/trac-1.0.6/trac/web/auth.py@:190-191#L182. Changes of the option wouldn't lead expiration of the cookies which have been sent to the users.

You should delete auth_cookie records to forcibly log out.

So, if I uderstand what you are saying, to do what I want, auth_cookie_lifetime is not the solution. But then, is there an existing option that does what I want ? Or do I need somehow to modify trac ?

I'm sorry if this is not an AccountManagerPligin issue. I noticed most of the auth mechanismn was overriden in AccountManagerPlugin so I thought it was the right place to discuss about it :)

[Ryan J Ollos]

My earlier tests were not extensive enough. It is like Jun has described. Using basic authentication with [trac] auth_cookie_lifetime set to a small value, after the time has elapsed the login link will be displayed and authenticated permissions have been revoked. However, on following the login link I'm not prompted to authenticate and have elevated permissions again. I suppose the issue is related to ​trac:#791.

[Ryan J Ollos]

Replying to jun66j5:

You should delete auth_cookie records to forcibly log out.

That sounds like something the user could do, but is there anything the administrator can do to force logout, and is there anything AccountManagerPlugin can do to force logout?

I also wonder if the documentation for [trac] auth_cookie_lifetime might be slightly misleading: ​wiki:TracIni#trac-section: This value determines how long the browser will cache authentication information, and therefore, after how much inactivity a user will have to log in again.

[Jun Omae]

Rethinking about that. auth_cookie.time is used to store created time of the auth cookie. I think we could check using the column that the auth cookie is alive when auth_cookie_lifetime option is set to positive value. Thoughts?

trac/web/auth.py

@@ -233 +233 @@
         # latter is overridden in AccountManager.
         if self.check_ip:
             sql = "SELECT name FROM auth_cookie WHERE cookie=%s AND ipnr=%s"
-            args = (cookie.value, req.remote_addr)
+            args = [cookie.value, req.remote_addr]
         else:
             sql = "SELECT name FROM auth_cookie WHERE cookie=%s"
-            args = (cookie.value,)
+            args = [cookie.value]
+        if self.auth_cookie_lifetime > 0:
+            sql += " AND time >= %s"
+            args.append(int(time.time()) - self.auth_cookie_lifetime)
         for name, in self.env.db_query(sql, args):
             return name
 

[Ryan J Ollos]

It's probably that I don't understand the mechanism well enough, but it seems that even if we ensure the cookie is expired that doesn't clear the cached authentication information and force the user to re-authenticate. That is the behavior I see even after the patch is applied (basic auth and TracStandalone).

[Steffen Hoffmann]

While I disagree, that a ticket is a good discussion form if a discussion form at all, I'm thankful for this report. For discussion still use our mailing-list, please.

On the issue, it's too late for drafting and testing a patch here, but from looking at the source it might really be a plugin issue. Seems like the cookie refresh is only done right for persistent sessions. Odd enough I only remember testing expiration for persistent sessions as well. Tried to find a suspicious change in recent change history, but failed to find it, so cookie expiration might have never worked.

[Vincent Penne <ziggy@…>]

Replying to jun66j5:

Rethinking about that. auth_cookie.time is used to store created time of the auth cookie. I think we could check using the column that the auth cookie is alive when auth_cookie_lifetime option is set to positive value. Thoughts?

trac/web/auth.py

@@ -233 +233 @@
         # latter is overridden in AccountManager.
         if self.check_ip:
             sql = "SELECT name FROM auth_cookie WHERE cookie=%s AND ipnr=%s"
-            args = (cookie.value, req.remote_addr)
+            args = [cookie.value, req.remote_addr]
         else:
             sql = "SELECT name FROM auth_cookie WHERE cookie=%s"
-            args = (cookie.value,)
+            args = [cookie.value]
+        if self.auth_cookie_lifetime > 0:
+            sql += " AND time >= %s"
+            args.append(int(time.time()) - self.auth_cookie_lifetime)
         for name, in self.env.db_query(sql, args):
             return name
 

In addition to your proposition, I had also to change _get_name_for_cookie as such :

    def _get_name_for_cookie(self, req, cookie):
        name = self._cookie_to_name(req, cookie)
        if name is None:
            # The cookie is invalid (or has been purged from the
            # database), so tell the user agent to drop it as it is
            # invalid
            self._expire_cookie(req)
        elif self.auth_cookie_lifetime > 0:
            # refresh time
            self.env.db_transaction("UPDATE auth_cookie SET time = %s WHERE cookie = %s", 
                                    (int(time.time()), cookie.value, ))
        return name

(see the new else clause)

My addition refresh the time every time there's some activity. Without this, the cookie would expire unconditionally after trac: auth_cookie_lifetime even if the user had some activity. Initial tests seems to show it's working, but of course I don't really know what I'm doing here, I'm not so familiar with this code :)

I don't know if it ever was intended to take the user activity into account. If not, I would suggest to add a new setting specifically for that.

[Jun Omae]

Replying to Vincent Penne <ziggy@…>:

My addition refresh the time every time there's some activity. Without this, the cookie would expire unconditionally after trac: auth_cookie_lifetime even if the user had some activity. Initial tests seems to show it's working, but of course I don't really know what I'm doing here, I'm not so familiar with this code :)

No. We shouldn't refresh the time field. If the auth_cookie_lifetime option is set to 600 seconds, Trac keeps user's logged in only in 600 seconds even though the user's activities. Another option should be added for that you wants.

Documentation of the option says:

Lifetime of the authentication cookie, in seconds. This value determines how long the browser will cache authentication information, and therefore, after how much inactivity a user will have to log in again. The value of 0 makes the cookie expire at the end of the browsing session.

Please create a new ticket as enhancement with the details on trac.edgewall.org if you want another behaviors for auth cookie.

[Vincent Penne <ziggy@…>]

Replying to jun66j5:

No. We shouldn't refresh the time field. If the auth_cookie_lifetime option is set to 600 seconds, Trac keeps user's logged in only in 600 seconds even though the user's activities. Another option should be added for that you wants.

Documentation of the option says:

Lifetime of the authentication cookie, in seconds. This value determines how long the browser will cache authentication information, and therefore, after how much inactivity a user will have to log in again. The value of 0 makes the cookie expire at the end of the browsing session.

Please create a new ticket as enhancement with the details on trac.edgewall.org if you want another behaviors for auth cookie.

But precisely, the documentation says

and therefore, after how much inactivity a user will have to log in again

so it does sounds like it should be the behaviour I expect (and desire), doesn't it ? The documentation really talks about inactivity time (hence we need to keep track of the last activity time)

Am I missing something ?

[Ryan J Ollos]

The documentation can be a bit confusing depending on how one defines inactivity. However, there's a reason that activity is tracked relative to the last update of the session data. I doubt we want an extra database hit on every request.

Either way I don't think we'd get a true logout anyway due to the issue in ​​trac:#791. I'm not claiming to have a strong understanding of that issue though.

[Vincent Penne <ziggy@…>]

We could optimize the db hit by updating only if the time since last update is greater than some reasonable value (say, 10 seconds or a minute for example). In any case, this happens only if trac: auth_cookie_lifetime is set.

[Ryan J Ollos]

Like Jun has suggested, the discussion is more appropriate for Trac. I think the best place to continue this discussion would be the ​TracDev mailing list, so please create a thread there if you'd like to suggest a change for Trac.