Problem with storing authenticated sessions in DB

[kbogdanov@…]

I've been installing a new instance of TRAC (version 0.10dev) and I discovered that session settings wasn't stored for an authenticated user. So, it seems I've found the source of this problem - it come with changeset:3256 in the source:/trunk/trac/web/session.py​

As you can see - at line 153 we're cheking if this session is new and should be stored in the database. But due to the last changes we got line 137 and because of it attribute "_new" is never equal "True" at line 153.

I'm not sure if I fixed it properly, but my temporary resoltion is to move the line 137:

self._new = False

into the if body to the line 124.

[Christian Boos]

The current code looks correct to me: inside promote_session, in either case of the if, you end up with (sid,authenticated) == (self.req.authname,1) so doing an INSERT of these values in save would violate the primary key constraint on the session table. That's why it's prevented by the test against self._new.

I've also not been able to reproduce the issue. Maybe detailing the steps to reproduce the error and a relevant dump of the session table before and after would help to understand the issue.

[kbogdanov@…]

Ok, here is detailed description:

As I said, I was installing completely new instance of TRAC. I use apache2, python2.4 and PostgreSQL 8.1.3. For authentication I installed TracAccountManager plugin (trunk). I'd used such configuration with TRAC-0.9 before and everyting worked correctly.

So, I configured TRAC to work with multiple repositories, base URL is ​https://projects.

Apache config for this virtual host contains:

<Location />
   SetHandler      mod_python

   PythonHandler   trac.web.modpython_frontend
   PythonOption    TracEnvParentDir                /srv/trac

   PythonOption    TracUriRoot                     /
   SetEnv          PYTHON_EGG_CACHE                /var/cache/apache2/python/

   Order allow,deny
   Allow from all
</Location>

Alias /trac/ /usr/share/trac/htdocs

<LocationMatch /trac/>
   SetHandler None
</LocationMatch>

Here are detailed steps:

• Open TRAC repository Misc as anonymous user (​https://projects/Misc)
• Check session table, it's empty:

  > psql -d tracmisc
  tracmisc=# select * from session;
     sid   | authenticated | last_visit
  ---------+---------------+------------
  (0 rows)
  

• Check trac.log:

  2006-05-15 21:18:36,918  Trac[loader] DEBUG: Loading plugin webadmin.plugin from /usr/lib/python2.4/site-packages/TracAccountManager-0.1.2-py2.4.egg
  2006-05-15 21:18:37,095 Trac[session] DEBUG: Trying to restore session - sid: 4c53a05020e82762d255eaad, authenticated: 0, _new: True
  

The last line is my own debug code placed in get_session method

• Login as user bogdans (​https://projects/Misc/login)
• Check session table - it's still empty
• Check trac.log:

  2006-05-15 21:19:01,813 Trac[session] DEBUG: Promoting anonymous session 4c53a05020e82762d255eaad to authenticated session for user bogdans
  2006-05-15 21:19:01,815 Trac[session] DEBUG: Trying to restore session - sid: bogdans, authenticated: 1, _new: False
  

• Try to set and store settings for user bogdans (​https://projects/Misc/settings)
• Check session table - it's still empty, but now sesson parameters are stored in sessing_attribute table:

  tracmisc=# select * from session_attribute;
     sid   | authenticated | name  |         value
  ---------+---------------+-------+------------------------
   bogdans |             1 | email | kbogdanov@redsolex.com
   bogdans |             1 | name  | Kirill
  (2 rows)
  

1. Check trac.log:

   2006-05-15 21:19:27,043 Trac[session] DEBUG: Trying to restore session - sid: bogdans, authenticated: 1, _new: False
   2006-05-15 21:19:27,062 Trac[session] INFO: Refreshing session bogdans
   2006-05-15 21:19:27,063 Trac[session] DEBUG: Purging old, expired, sessions.
   

As you can see, method save has never been called with _new = True even for the anonymous session. promote_session method was called after user logon, so I decided that self._new = False is wrong there. Now I see you're right about it, but therefore somewhere else, when we create an anonymous session sid we should call save for this session.

[Christian Boos]

Thanks to your detailed description, I was able to reproduce the issue — it happens when one tries to promote an anonymous session, but there's actually no anonymous session stored.

I'm working on a fix.

[Christian Boos]

Can you test the following patch?

Index: trac/web/session.py
===================================================================
--- trac/web/session.py	(revision 3296)
+++ trac/web/session.py	(working copy)
@@ -113,16 +113,20 @@
 
         db = self.env.get_db_cnx()
         cursor = db.cursor()
-        cursor.execute("SELECT COUNT(*) FROM session WHERE sid=%s "
-                       "AND authenticated=1", (self.req.authname,))
-        if cursor.fetchone()[0]:
+        cursor.execute("SELECT COUNT(*) FROM session "
+                       "WHERE sid=%s OR sid=%s ", (sid, self.req.authname))
+        sessions = cursor.fetchone()[0]
+        self.env.log.debug('For %s -- %s, nb sessions=%d' % \
+                           (sid, self.req.authname, sessions))
+        
+        if sessions == 2:
             # If there's already an authenticated session for the user, we
             # simply delete the anonymous session
             cursor.execute("DELETE FROM session WHERE sid=%s "
                            "AND authenticated=0", (sid,))
             cursor.execute("DELETE FROM session_attribute WHERE sid=%s "
                            "AND authenticated=0", (sid,))
-        else:
+        elif sessions == 1:
             # Otherwise, update the session records so that the session ID is
             # the user name, and the authenticated flag is set
             self.env.log.debug('Promoting anonymous session %s to '
@@ -134,6 +138,11 @@
             cursor.execute("UPDATE session_attribute SET sid=%s,"
                            "authenticated=1 WHERE sid=%s",
                            (self.req.authname, sid))
+        else:
+            # Maybe we didn't have an anonymous session for this sid
+            cursor.execute("INSERT INTO session (sid,last_visit,authenticated)"
+                           " VALUES(%s,%s,1)",
+                           (self.req.authname, int(time.time())))
         self._new = False
         db.commit()

[kbogdanov@…]

Yep, thanks! It seems you're fixed it. Everything works fine. I've created another user, logged in and checked session table - new authenticated session has been added.

Thanks a lot!

[Christian Boos]

I modified the patch a bit in r3302, to avoid trying to do an UPDATE when there's only the authenticated session (the elif sessions == 1: case in the above).