Edgewall Software
Modify

Opened 9 years ago

Closed 9 years ago

Last modified 4 months ago

#12129 closed defect (fixed)

get_session fails for usernames with . (dot, period)

Reported by: hauke.muentinga@… Owned by: Jun Omae
Priority: high Milestone: 1.0.8
Component: web frontend Version: 1.0.7
Severity: critical Keywords:
Cc: Branch:
Release Notes:

Fix wrong restriction of sid for authenticated users, regression in [14120].

API Changes:
Internal Changes:

Description (last modified by Ryan J Ollos)

In my Trac instance I use the naming scheme firstname.lastname. With the upgrade to 1.0.7, after login users were greeted with a Traceback:

2015-07-19 13:54:31,017 Trac[main] ERROR: Internal Server Error: 
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/trac/web/main.py", line 550, in 
_dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/lib/python2.7/dist-packages/trac/web/main.py", line 208, in 
dispatch
    self._pre_process_request(req, chosen_handler)
  File "/usr/local/lib/python2.7/dist-packages/trac/web/main.py", line 371, in 
_pre_process_request
    chosen_handler = filter_.pre_process_request(req, chosen_handler)
  File "build/bdist.linux-x86_64/egg/acct_mgr/register.py", line 479, in pre_pr
ocess_request
    if not req.session.authenticated:
AttributeError: 'FakeSession' object has no attribute 'authenticated'

This is due to the validation of sid in trac.web.session.Session.get_session() with _valid_sid_re = re.compile(r'[_A-Za-z0-9\.]+\Z') introduced in [14120].

Attachments (1)

0001-Allow-periods-in-usernames.patch (1.5 KB ) - added by hauke.muentinga@… 9 years ago.
Patch

Download all attachments as: .zip

Change History (16)

by hauke.muentinga@…, 9 years ago

Patch

in reply to:  description comment:1 by anonymous, 9 years ago

Replying to hauke.muentinga@…:

This is due to the validation of sid in trac.web.session.Session.get_session() with _valid_sid_re = re.compile(r'[_A-Za-z0-9\.]+\Z') […]

That is actually my fixed regex, the broken one is: _valid_sid_re = re.compile(r'[_A-Za-z0-9]+\Z').

comment:2 by Jun Omae, 9 years ago

Priority: normalhigh

That's pretty bad. We shouldn't restrict sid for authenticated users because the user's name is used for sid. (e.g. ldap.user.name@REALM.EXAMPLE.COM, unicode username in #6318, …)

Log when trying to log in with jun.omae:

2015-07-21 15:30:39,523 Trac[util] DEBUG: args: ('3617a426ab2c1a6c0adc962ceec51dae', u'jun.omae', '192.168.11.46', 1437460239)
2015-07-21 15:30:39,523 Trac[util] DEBUG: prefetch: 0 rows
2015-07-21 15:30:39,524 Trac[util] DEBUG: SQL: SELECT authenticated FROM session WHERE sid=%s OR sid=%s
2015-07-21 15:30:39,524 Trac[util] DEBUG: args: ('0f944398718e7fe8ff58457f', u'jun.omae')
2015-07-21 15:30:39,524 Trac[util] DEBUG: prefetch: 0 rows
2015-07-21 15:30:39,524 Trac[util] DEBUG: SQL: INSERT INTO session (sid, last_visit, authenticated)
                          VALUES (%s, %s, 1)

2015-07-21 15:30:39,524 Trac[util] DEBUG: args: (u'jun.omae', 1437460239)
2015-07-21 15:30:39,529 Trac[util] DEBUG: prefetch: 0 rows
2015-07-21 15:30:39,529 Trac[util] DEBUG: SQL: SELECT authenticated FROM session WHERE sid=%s OR sid=%s
2015-07-21 15:30:39,529 Trac[util] DEBUG: args: ('0f944398718e7fe8ff58457f', u'jun.omae')
2015-07-21 15:30:39,529 Trac[util] DEBUG: prefetch: 1 rows
2015-07-21 15:30:39,529 Trac[main] ERROR: can't retrieve session: TracError: Session ID must be alphanumeric.
2015-07-21 15:30:39,531 Trac[main] DEBUG: Negotiated locale: None -> en_US
2015-07-21 15:30:39,536 Trac[main] ERROR: can't retrieve session: TracError: Session ID must be alphanumeric.
2015-07-21 15:30:39,757 Trac[main] DEBUG: Dispatching <RequestWithSession "GET '/'">

comment:3 by Ryan J Ollos, 9 years ago

There were a few instances of this traceback in the t-h.o logs and issue reproduced with user rjollos-test. We should probably generate a milestone:1.0.8 release fairly soon to fix the issue.

in reply to:  3 comment:4 by Jun Omae, 9 years ago

Proposed changes in jomae.git@t12129.

We should probably generate a milestone:1.0.8 release fairly soon to fix the issue.

I agree.

comment:5 by Ryan J Ollos, 9 years ago

Changes work well for me. I can do the release on Thursday.

There are some test case failures though because multiple tests are run in the same case without reseting the database. Here is the log for one failure:

======================================================================
FAIL: test_new_session_promotion (trac.web.tests.session.SessionTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/Workspace/t11944/teo-rjollos.git/trac/web/tests/session.py", line 176, in test_new_session_promotion
    self._test_new_session_promotion('j.smith')
  File "/home/user/Workspace/t11944/teo-rjollos.git/trac/web/tests/session.py", line 172, in _test_new_session_promotion
    "SELECT sid, authenticated FROM session"))
AssertionError: Lists differ: [('j.smith', 1)] != [(u'j.smith', 1), (u'john', 1)...

Second list contains 1 additional elements.
First extra element 1:
(u'john', 1)

- [('j.smith', 1)]
+ [(u'j.smith', 1), (u'john', 1)]
Last edited 9 years ago by Ryan J Ollos (previous) (diff)

comment:6 by Jun Omae, 9 years ago

Thanks for the reviewing. I didn't run tests, stupidly. Updated jomae.git@t12129 branch.

comment:7 by Ryan J Ollos, 9 years ago

Looks good and tests passing for me. I'll deploy the fix to trac-hacks.org soon after you commit the change.

comment:8 by Jun Omae, 9 years ago

Release Notes: modified (diff)

Committed in [14191] and merged to trunk in [14192].

comment:9 by Ryan J Ollos, 9 years ago

Resolution: fixed
Status: newclosed

comment:10 by Ryan J Ollos, 9 years ago

Owner: set to Jun Omae

comment:11 by Ryan J Ollos, 9 years ago

I've found some errors in the logs days after having upgraded. Any ideas?

2015-07-27 04:09:41,414 Trac[main] ERROR: Exception caught while post-processing request:
Traceback (most recent call last):
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 274, in dispatch
    self._post_process_request(req)
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 387, in _post_process_request
    f.post_process_request(req, *(None,)*extra_arg_count)
  File "/srv/trac-hacks.org/pve/lib/python2.6/site-packages/TracAccountManager-0.4.3-py2.6.egg/acct_mgr/register.py", line 509, in post_process_request
    if not req.session.authenticated:
AttributeError: 'FakeSession' object has no attribute 'authenticated'
2015-07-27 04:09:41,415 Trac[main] ERROR: Internal Server Error:
Traceback (most recent call last):
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 551, in _dispatch_request
    dispatcher.dispatch(req)
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 209, in dispatch
    self._pre_process_request(req, chosen_handler)
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 372, in _pre_process_request
    chosen_handler = filter_.pre_process_request(req, chosen_handler)
  File "/srv/trac-hacks.org/pve/lib/python2.6/site-packages/TracAccountManager-0.4.3-py2.6.egg/acct_mgr/register.py", line 479, in pre_process_request
    if not req.session.authenticated:
AttributeError: 'FakeSession' object has no attribute 'authenticated'

comment:12 by Jun Omae, 9 years ago

Similar to #12062. How about adding same attributes of DetachedSession to the FakeSession?

  • trac/web/main.py

    diff --git a/trac/web/main.py b/trac/web/main.py
    index 909d75ec0..728141655 100644
    a b default_tracker = 'http://trac.edgewall.org'  
    6262
    6363class FakeSession(dict):
    6464    sid = None
     65    authenticated = False
     66    last_visit = 0
    6567    def save(self):
    6668        pass
    6769

comment:13 by Ryan J Ollos, 9 years ago

Seems like a good idea. If committed soon we can test on trac-hacks.org leading up to the 1.0.9 release to be sure it resolves all issues.

comment:14 by Ryan J Ollos, 9 years ago

#12172 created to deal with issue in comment:11.

comment:15 by Ryan J Ollos, 8 years ago

Description: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.