Improvements and Bug Fixes for gae-sessions v1.07

demo-with-google-logins/app.yaml

@@ -4,7 +4,13 @@ runtime: python
 api_version: 1
 
 handlers:
+  #cron job,  this should be first.
+- url: /cleanup
+  script: cleanup.py
+  login: admin
+
 - url: /stats.*
   script: $PYTHON_LIB/google/appengine/ext/appstats/ui.py
+
 - url: /.*
-  script: main.py
+  script: main.py

demo-with-google-logins/cleanup.py

@@ -0,0 +1,11 @@
+from google.appengine.ext import webapp
+from gaesessions import delete_expired_sessions
+
+
+#This is called once a week to cleanup stale session data.
+class cleanup(webapp.RequestHandler):
+
+    def get(self):
+        #Remove all expired sessions from the session store
+        while not delete_expired_sessions():
+            pass

demo-with-google-logins/cron.yaml

@@ -0,0 +1,6 @@
+cron:
+
+#- Remove expired sessions from the session store. 
+  url: /cleanup
+  schedule: every 7 days
+

demo/app.yaml

@@ -4,6 +4,11 @@ runtime: python
 api_version: 1
 
 handlers:
+  #cron job,  this should be first.
+- url: /cleanup
+  script: cleanup.py
+  login: admin
+
 - url: /stats.*
   script: $PYTHON_LIB/google/appengine/ext/appstats/ui.py
 - url: /.*

demo/cleanup.py

@@ -0,0 +1,11 @@
+from google.appengine.ext import webapp
+from gaesessions import delete_expired_sessions
+
+
+#This is called once a week to cleanup stale session data.
+class cleanup(webapp.RequestHandler):
+
+    def get(self):
+        #Remove all expired sessions from the session store
+        while not delete_expired_sessions():
+            pass

demo/cron.yaml

@@ -1,4 +1,6 @@
 cron:
-- description: daily session cleanup
-  url: /cleanup_sessions
-  schedule: every 24 hours
+
+#- Remove expired sessions from the session store. 
+  url: /cleanup
+  schedule: every 7 days
+

gaesessions/__init__.py

@@ -14,13 +14,13 @@
 from google.appengine.ext import db
 
 # Configurable cookie options
-COOKIE_NAME_PREFIX = "DgU"  # identifies a cookie as being one used by gae-sessions (so you can set cookies too)
+COOKIE_NAME_PREFIX = "D"  # identifies a cookie as being one used by gae-sessions (so you can set cookies too)
 COOKIE_PATH = "/"
 DEFAULT_COOKIE_ONLY_THRESH = 10240  # 10KB: GAE only allows ~16000B in HTTP header - leave ~6KB for other info
 DEFAULT_LIFETIME = datetime.timedelta(days=7)
 
 # constants
-SID_LEN = 43  # timestamp (10 chars) + underscore + md5 (32 hex chars)
+SID_LEN = 32  # timestamp (4 chars) + underscore (1 char) + os.urandom(SID_LEN-5)
 SIG_LEN = 44  # base 64 encoded HMAC-SHA256
 MAX_COOKIE_LEN = 4096
 EXPIRE_COOKIE_FMT = ' %s=; expires=Wed, 01-Jan-1970 00:00:00 GMT; Path=' + COOKIE_PATH
@@ -46,10 +46,30 @@ def set_current_session(session):
 def is_gaesessions_key(k):
     return k.startswith(COOKIE_NAME_PREFIX)
 
+def packInt(i):
+    i=int(i)
+    ret=''
+    #Read off one byte at a time and build a byte array.
+    while i:
+        ret = chr((i & 0b11111111)) + ret
+        i = i >> 8
+    return ret
+
+def unpackInt(d):
+    ret=0
+    x=len(d)-1
+    #Read though the byte array
+    for c in d:
+        #turn this byte into an int,  
+        #and then move it over to the right place
+        ret += (ord(c) << 8 * x)
+        x-=1
+    return ret
 
 class SessionModel(db.Model):
     """Contains session data.  key_name is the session ID and pdump contains a
     pickled dictionary which maps session variables to their values."""
+    sid = db.ByteStringProperty()
     pdump = db.BlobProperty()
 
 
@@ -74,6 +94,7 @@ def __init__(self, sid=None, lifetime=DEFAULT_LIFETIME, no_datastore=False,
         self.no_datastore = no_datastore
         self.cookie_only_thresh = cookie_only_threshold
         self.base_key = cookie_key
+        self.expiration = None
 
         if sid:
             self.__set_sid(sid, False)
@@ -97,22 +118,32 @@ def __read_cookie(self):
                 return  # no session yet
             self.cookie_keys.sort()
             data = ''.join(cookie[k].value for k in self.cookie_keys)
-            i = SIG_LEN + SID_LEN
-            sig, sid, b64pdump = data[:SIG_LEN], data[SIG_LEN:i], data[i:]
-            pdump = b64decode(b64pdump)
-            actual_sig = Session.__compute_hmac(self.base_key, sid, pdump)
-            if sig == actual_sig:
-                self.__set_sid(sid, False)
-                # check for expiration and terminate the session if it has expired
+            #If we have blob then it needs to be authenticated with an HMAC.    
+            i = SIG_LEN + SID_LEN            
+            #Is this just an SID value?
+            if len(data) < i:
+                self.__set_sid(data, False)
+                #Check the timeout,  if a hacker has modified this value
+                #then this SID won't be in the database and he won't be able to use it.
                 if self.get_expiration() != 0 and time.time() > self.get_expiration():
                     return self.terminate()
-
-                if pdump:
-                    self.data = self.__decode_data(pdump)
+                self.data = None 
+            else:                
+                sig, sid, b64pdump = data[:SIG_LEN], data[SIG_LEN:i], data[i:]
+                pdump = b64decode(b64pdump)
+                actual_sig = Session.__compute_hmac(self.base_key, sid, pdump)
+                if sig == actual_sig:
+                    self.__set_sid(sid, False)
+                    # check for expiration and terminate the session if it has expired
+                    if self.get_expiration() != 0 and time.time() > self.get_expiration():
+                        return self.terminate()
+
+                    if pdump:
+                        self.data = self.__decode_data(pdump)
+                    else:
+                        self.data = None  # data is in memcache/db: load it on-demand
                 else:
-                    self.data = None  # data is in memcache/db: load it on-demand
-            else:
-                logging.warn('cookie with invalid sig received from %s: %s' % (os.environ.get('REMOTE_ADDR'), b64pdump))
+                    logging.warn('cookie with invalid sig received from %s: %s' % (os.environ.get('REMOTE_ADDR'), b64pdump))
         except (CookieError, KeyError, IndexError, TypeError):
             # there is no cookie (i.e., no session) or the cookie is invalid
             self.terminate(False)
@@ -133,8 +164,13 @@ def make_cookie_headers(self):
         else:
             m = MAX_DATA_PER_COOKIE
             fmt = COOKIE_FMT
-        sig = Session.__compute_hmac(self.base_key, self.sid, self.cookie_data)
-        cv = sig + self.sid + b64encode(self.cookie_data)
+        #Are we using cookies for session state?
+        if self.cookie_data != '':
+            sig = Session.__compute_hmac(self.base_key, self.sid, self.cookie_data)
+            cv = sig + self.sid + b64encode(self.cookie_data)
+        else:
+            #Looks like we just need the session id.
+            cv = self.sid
         num_cookies = 1 + (len(cv) - 1) / m
         if self.get_expiration() > 0:
             ed = "expires=%s; " % datetime.datetime.fromtimestamp(self.get_expiration()).strftime(COOKIE_DATE_FMT)
@@ -146,18 +182,19 @@ def make_cookie_headers(self):
         old_cookies = xrange(num_cookies, len(self.cookie_keys))
         key = COOKIE_NAME_PREFIX + '%02d'
         cookies_to_ax = [EXPIRE_COOKIE_FMT % (key % i) for i in old_cookies]
-        return cookies + cookies_to_ax
-
+        return  cookies + cookies_to_ax
+        
     def is_active(self):
         """Returns True if this session is active (i.e., it has been assigned a
         session ID and will be or has been persisted)."""
+        self.ensure_data_loaded()
         return self.sid is not None
 
     def is_ssl_only(self):
         """Returns True if cookies set by this session will include the "Secure"
         attribute so that the client will only send them over a secure channel
         like SSL)."""
-        return self.sid is not None and self.sid[-33] == 'S'
+        return self.sid is not None and self.sid[-1] == 'S'
 
     def is_accessed(self):
         """Returns True if any value of this session has been accessed."""
@@ -172,13 +209,16 @@ def ensure_data_loaded(self):
     def get_expiration(self):
         """Returns the timestamp at which this session will expire."""
         try:
-            return int(self.sid[:-33])
+            #Lets unpack this value once.
+            if not self.expiration:
+                self.expiration = int(unpackInt(self.sid[:4]))
+            return self.expiration
         except:
             return 0
 
     def __make_sid(self, expire_ts=None, ssl_only=False):
         """Returns a new session ID."""
-        # make a random ID (random.randrange() is 10x faster but less secure?)
+        # Get the current time
         if expire_ts is None:
             expire_dt = datetime.datetime.now() + self.lifetime
             expire_ts = int(time.mktime((expire_dt).timetuple()))
@@ -188,7 +228,11 @@ def __make_sid(self, expire_ts=None, ssl_only=False):
             sep = 'S'
         else:
             sep = '_'
-        return ('%010d' % expire_ts) + sep + hashlib.md5(os.urandom(16)).hexdigest()
+        #A time stamp is 4 bytes,  and the seperator is 1
+        sid=packInt(expire_ts) + os.urandom(SID_LEN - 5)
+        #The ssl flag is always at the ends
+        sid=str(sid).encode('base64')[:SID_LEN - 1] + sep
+        return sid
 
     @staticmethod
     def __encode_data(d):
@@ -269,7 +313,7 @@ def __set_sid(self, sid, make_cookie=True):
             self.__clear_data()
         self.sid = sid
         self.db_key = db.Key.from_path(SessionModel.kind(), sid, namespace='')
-
+        
         # set the cookie if requested
         if make_cookie:
             self.cookie_data = ''  # trigger the cookie to be sent
@@ -313,6 +357,7 @@ def save(self, persist_even_if_using_cookie=False):
         evaluates to True, memcache and the datastore will also be used.  If the
         no_datastore option is set, then the datastore will never be used.
 
+    SessionModel(key_name=make_sid(), pdump="2").put()
         Normally this method does not need to be called directly - a session is
         automatically saved at the end of the request if any changes were made.
         """
@@ -342,6 +387,7 @@ def save(self, persist_even_if_using_cookie=False):
             return
         try:
             SessionModel(key_name=self.sid, pdump=pdump).put()
+            pass
         except Exception, e:
             logging.warning("unable to persist session to datastore for sid=%s (%s)" % (self.sid, e))
 
@@ -465,7 +511,7 @@ def __call__(self, environ, start_response):
         def my_start_response(status, headers, exc_info=None):
             _tls.current_session.save()  # store the session if it was changed
             for ch in _tls.current_session.make_cookie_headers():
-                headers.append(('Set-Cookie', ch))
+                headers.append(('Set-Cookie',   ch))
             return start_response(status, headers, exc_info)
 
         # let the app do its thing
@@ -500,16 +546,41 @@ def process_response(self, request, response):
         return response
 
 
+def make_sid( expire_ts=None, ssl_only=False):
+    """Returns a new session ID."""
+    # Get the current time
+    if expire_ts is None:
+        expire_dt = datetime.datetime.now()
+        expire_ts = int(time.mktime((expire_dt).timetuple()))
+    else:
+        expire_ts = int(expire_ts)
+    if ssl_only:
+        sep = 'S'
+    else:
+        sep = '_'
+    #A time stamp is 4 bytes,  and the seperator is 1,  with 26 bytes of PRNG making it 32 bytes. 
+    sid=packInt(expire_ts) + os.urandom(SID_LEN)
+    #The ssl flag is always at the end.
+    #Throw away what we don't need, 
+    #it probably a waste of CPU to check what the base64 will be.
+    sid=str(sid).encode('base64')[:SID_LEN-1]+sep
+    return sid
+
 def delete_expired_sessions():
     """Deletes expired sessions from the datastore.
     If there are more than 500 expired sessions, only 500 will be removed.
     Returns True if all expired sessions have been removed.
     """
-    now_str = unicode(int(time.time()))
+    #We want the timestamp part of the SID in base64
+    #base 10 comparisons work a lot like base64 comparisons
+    now_str = str(packInt(int(time.time()))).encode("base64").replace("=","")
     q = db.Query(SessionModel, keys_only=True, namespace='')
+    #What would this key look like?
     key = db.Key.from_path('SessionModel', now_str + u'\ufffd', namespace='')
+    #Find all keys that where created before this now_str timestamp
     q.filter('__key__ < ', key)
     results = q.fetch(500)
     db.delete(results)
     logging.info('gae-sessions: deleted %d expired sessions from the datastore' % len(results))
-    return len(results) < 500
+    #Are there expired sessions remaining?
+    return q.count(1) <= 0