Mercurial > p > roundup > code
changeset 5211:f4b6a2a3e605
Fix expiration dates and expire csrf tokens properly
In client.py: add explicit expiration of csrf tokens to
handle_csrf. There is a clean_up() that runs on every client
connection before handle)csrf is invoked, but it only cleans every
hour. With short lived tokens this is insufficient. Also remove
debugging.
In templating.py fix values for seconds/week and minutes per week. The
original values were shifted/transposed and an order of magnitude off.
In test_templating.py again fix seconds/week constant.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sun, 19 Mar 2017 17:10:13 -0400 |
| parents | 7da56980754d |
| children | d4cc71beb102 |
| files | roundup/cgi/client.py roundup/cgi/templating.py test/test_templating.py |
| diffstat | 3 files changed, 28 insertions(+), 14 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/cgi/client.py Sun Mar 19 15:32:14 2017 -0400 +++ b/roundup/cgi/client.py Sun Mar 19 17:10:13 2017 -0400 @@ -1086,18 +1086,25 @@ logger.warning(self._("required csrf field missing for user%s"), user) return True + # Expire old csrf tokens now so we don't use them. These will + # be committed after the otks.destroy below. Note that the + # self.clean_up run as part of determine_user() will run only + # once an hour. If we have short lived (e.g. 5 minute) keys + # they will live too long if we depend on clean_up. So we do + # our own. + otks.clean() + key=self.form['@csrf'].value uid = otks.get(key, 'uid', default=None) sid = otks.get(key, 'sid', default=None) - if __debug__: - ts = otks.get(key, '__timestamp', default=None) - print("Found key %s for user%s sess: %s, ts %s, time %s"%(key, uid, sid, ts, time.time())) - current_session = self.session_api._sid - # The key has been used or compromised. Delete it to prevent replay. + # The key has been used or compromised. + # Delete it to prevent replay. otks.destroy(key) self.db.commit() + current_session = self.session_api._sid + ''' # I think now that LogoutAction redirects to # self.base ([tracker] web parameter in config.ini),
--- a/roundup/cgi/templating.py Sun Mar 19 15:32:14 2017 -0400 +++ b/roundup/cgi/templating.py Sun Mar 19 17:10:13 2017 -0400 @@ -97,12 +97,13 @@ # That's the cleanup period hardcoded in otk.clean(). # If a user wants a 10 minute lifetime calculate # 10 minutes newer than 1 week ago. - # lifetime - 10800 (number of minutes in a week) + # lifetime - 10080 (number of minutes in a week) # convert to seconds and add (possible negative number) - # from time.time(). + # to current time (time.time()). + ts = time.time() + ((lifetime - 10080) * 60) otks.set(key, uid=client.db.getuid(), sid=client.session_api._sid, - __timestamp=time.time() + ((lifetime - 10800) * 60) ) + __timestamp=ts ) client.db.commit() return key
--- a/test/test_templating.py Sun Mar 19 15:32:14 2017 -0400 +++ b/test/test_templating.py Sun Mar 19 17:10:13 2017 -0400 @@ -143,8 +143,13 @@ ''' # the value below is number of seconds in a week. - week_seconds = 648000 + week_seconds = 604800 + + otks=self.client.db.getOTKManager() + for test in [ 'module', 'template', 'default_time' ]: + print "Testing:", test + if test == 'module': # test the module function nonce1 = anti_csrf_nonce(self, self.client, lifetime=1) @@ -162,7 +167,6 @@ greater_than = week_seconds - 10 * 60 self.assertEqual(len(nonce1), 64) - otks=self.client.db.getOTKManager() uid = otks.get(nonce1, 'uid', default=None) sid = otks.get(nonce1, 'sid', default=None) @@ -171,7 +175,11 @@ self.assertEqual(uid, 10) self.assertEqual(sid, self.client.session_api._sid) - ts = time.time() + now = time.time() + + print "now, timestamp, greater, difference", \ + now, timestamp, greater_than, now - timestamp + # lower bound of the difference is above. Upper bound # of difference is run time between time.time() in @@ -179,9 +187,7 @@ # that assigns ts above. I declare that difference # to be less than 1 second for this to pass. self.assertEqual(True, - greater_than < ts - timestamp < (greater_than + 1) ) - - print "completed", test + greater_than <= now - timestamp < (greater_than + 1) ) def test_string_url_quote(self): ''' test that urlquote quotes the string '''