Commits

dairiki committed 10dd6ad

Always lower-case and uniquify authnames.

Make sure to always lowercase the authname. It was being
lower-cased when recovering the authname from the cookie, but not
when generating the initial authname. This resulted --- unless
the users name was all lower case to start with --- in two
session being created upon initial login, one which was ignored
thereafter.

Always uniquify authnames. Since they are lowercased, there's
always a chance of collision, even when they include the identity
URL.

  • Participants
  • Parent commits 65c4d2c

Comments (0)

Files changed (1)

authopenid/authopenid.py

 import cPickle
 import re
 import time
+import itertools
 
 from trac.core import *
 from trac.config import Option, BoolOption, IntOption
 
                 self._commit_session(session, req) 
 
-                if self.combined_username and req.session['name']:
-                    remote_user = '%s <%s>' % (req.session['name'], remote_user)
-                else:
-                    if req.session.has_key('name'):
-                        remote_user = req.session['name']
+                if req.session.get('name'):
+                    authname = req.session['name']
+                    if self.combined_username:
+                        authname = '%s <%s>' % (authname, remote_user)
 
-                    # Check if we generated a colliding remote_user and make the user unique
-                    collisions = 0
-                    cremote_user = remote_user
-                    while True:
-                        ds = DetachedSession(self.env, remote_user)
-                        if not ds.has_key(self.openid_session_identity_url_key):
-                            # Old session, without the identity url set
-                            # Save the identity url then (bascially adopt the session)
-                            ds[self.openid_session_identity_url_key] = info.identity_url
-                            ds.save()
-                            break
-                        if ds[self.openid_session_identity_url_key] == info.identity_url:
-                            # No collision
-                            break
-                        # We got us a collision
-                        # Make the thing unique
-                        collisions += 1
-                        remote_user = "%s (%d)" % (cremote_user, collisions+1)
+                # Lower-case the authname.
+                #
+                # Really I don't think this should be done, but it was
+                # being done in the code which recovers the authname from
+                # the cookie, thus making sessions with upper-case
+                # in their session ids inaccessible.   Removing that
+                # lower-casing would fix the problem, but would introduce
+                # backward incompatibilities (for exisiting sessions).
+                # So we lower-case here as well.
+                authname = authname.lower()
 
-                req.authname = remote_user
+                # Make authname unique in case of collisions
+                #
+                # XXX: We ought to first look for an existing authenticated
+                # station with matching identity_url, and just use that
+                # for the authid.  (E.g. what if the user changes his
+                # fullname at the openid provider?)  However, trac does
+                # not seem to provide an API for searching sessions other
+                # than by sid/authname.
+                #
+                def authnames(base):
+                    yield base
+                    for attempt in itertools.count(2):
+                        yield "%s (%d)" % (base, attempt)
+
+                for authname in authnames(authname):
+                    ds = DetachedSession(self.env, authname)
+                    if ds.last_visit == 0 and len(ds) == 0:
+                        # At least in 0.12.2, this mean no session exists.
+                        break
+                    ds_identity = ds.get(self.openid_session_identity_url_key)
+                    if ds_identity == info.identity_url:
+                        # No collision
+                        break
+
+                req.authname = authname
 
                 db = self.env.get_db_cnx()
                 cursor = db.cursor()
                 cursor.execute("INSERT INTO auth_cookie (cookie,name,ipnr,time) "
-                               "VALUES (%s, %s, %s, %s)", (cookie, remote_user,
+                               "VALUES (%s, %s, %s, %s)", (cookie, authname,
                                self._get_masked_address(req.remote_addr), int(time.time())))
                 db.commit()