Simplify API for Session

- Allow to extend session management with a domain
- Let security manage the logging
- Let check differentiate expired session and session that does not exist

issue7134
review39171002

CHANGELOG

@@ -1,3 +1,4 @@
+* Simplify API for Session: new, remove, check and reset
 * Ensure that all buttons are registered in ir.model.button
 * Make trytond-admin ask for admin email
 * Add option to set admin email with trytond-admin

trytond/ir/session.py

@@ -46,29 +46,62 @@ class Session(ModelSQL):
         return token_hex(nbytes)
 
     @classmethod
-    def check(cls, user, key):
-        "Check user key and delete old one"
+    def new(cls, values=None):
+        "Create a new session for the transaction user and return the key."
+        if values is None:
+            values = {}
+        session, = cls.create([values])
+        return session.key
+
+    @classmethod
+    def remove(cls, key, domain=None):
+        "Delete the key session and return the login."
+        domain = [
+            ('key', '=', key),
+            domain or [],
+            ]
+        sessions = cls.search(domain)
+        if not sessions:
+            return
+        session, = sessions
+        name = session.create_uid.login
+        cls.delete(sessions)
+        return name
+
+    @classmethod
+    def check(cls, user, key, domain=None):
+        """
+        Check user key and delete old one.
+        Return True if key is still valid, False if the key is expired and None
+        if the key does not exist.
+        """
         now = datetime.datetime.now()
         timeout = datetime.timedelta(
             seconds=config.getint('session', 'timeout'))
         sessions = cls.search([
                 ('create_uid', '=', user),
+                domain or [],
                 ])
-        find = False
+        find = None
+        to_delete = []
         for session in sessions:
             timestamp = session.write_date or session.create_date
             if abs(timestamp - now) < timeout:
                 if session.key == key:
                     find = True
             else:
-                cls.delete([session])
+                if find is None and session.key == key:
+                    find = False
+                to_delete.append(session)
+        cls.delete(to_delete)
         return find
 
     @classmethod
-    def reset(cls, session):
-        "Reset session timestamp"
+    def reset(cls, key, domain=None):
+        "Reset key session timestamp"
         sessions = cls.search([
-                ('key', '=', session),
+                ('key', '=', key),
+                domain or [],
                 ])
         cls.write(sessions, {})
 

trytond/protocols/dispatcher.py

@@ -60,9 +60,6 @@ def login(request, database_name, user, parameters, language=None):
     except RateLimitException:
         session = None
         code = 429
-    msg = 'successful login' if session else 'bad login or password'
-    logger.info('%s \'%s\' from %s using %s on database \'%s\'',
-        msg, user, request.remote_addr, request.scheme, database_name)
     if not session:
         abort(code)
     return session
@@ -71,12 +68,9 @@ def login(request, database_name, user, parameters, language=None):
 @app.auth_required
 def logout(request, database_name):
     auth = request.authorization
-    name = security.logout(
+    security.logout(
         database_name, auth.get('userid'), auth.get('session'),
         context={'_request': request.context})
-    logger.info('logout \'%s\' from %s using %s on database \'%s\'',
-        name, request.remote_addr, request.scheme, database_name)
-    return True
 
 
 @app.route('/', methods=['POST'])

trytond/protocols/wrappers.py

@@ -60,13 +60,15 @@ class Request(_Request):
         auth = self.authorization
         if not auth:
             return None
+        context = {'_request': self.context}
         if auth.type == 'session':
             user_id = security.check(
                 database_name, auth.get('userid'), auth.get('session'),
-                context={'_request': self.context})
+                context=context)
         else:
             user_id = security.login(
-                database_name, auth.username, auth, cache=False)
+                database_name, auth.username, auth, cache=False,
+                context=context)
         return user_id
 
     @cached_property

trytond/security.py

@@ -1,11 +1,15 @@
 # This file is part of Tryton.  The COPYRIGHT file at the top level of
 # this repository contains the full copyright notices and license terms.
+import logging
+
 from trytond.pool import Pool
 from trytond.config import config
 from trytond.transaction import Transaction
 from trytond import backend
 from trytond.exceptions import LoginException, RateLimitException
 
+logger = logging.getLogger(__name__)
+
 
 def _get_pool(dbname):
     database_list = Pool.database_list()
@@ -15,6 +19,11 @@ def _get_pool(dbname):
     return pool
 
 
+def _get_remote_addr(context):
+    if context and '_request' in context:
+        return context['_request'].get('remote_addr')
+
+
 def login(dbname, loginname, parameters, cache=True, context=None):
     DatabaseOperationalError = backend.get('DatabaseOperationalError')
     for count in range(config.getint('database', 'retry'), -1, -1):
@@ -32,14 +41,20 @@ def login(dbname, loginname, parameters, cache=True, context=None):
                 transaction.commit()
                 raise
         break
+    session = None
     if user_id:
         if not cache:
-            return user_id
-        with Transaction().start(dbname, user_id):
-            Session = pool.get('ir.session')
-            session, = Session.create([{}])
-            return user_id, session.key
-    return
+            session = user_id
+        else:
+            with Transaction().start(dbname, user_id):
+                Session = pool.get('ir.session')
+                session = user_id, Session.new()
+        logger.info("login succeeded for '%s' from '%s' on database '%s'",
+            loginname, _get_remote_addr(context), dbname)
+    else:
+        logger.error("login failed for '%s' from '%s' on database '%s'",
+            loginname, _get_remote_addr(context), dbname)
+    return session
 
 
 def logout(dbname, user, session, context=None):
@@ -49,19 +64,13 @@ def logout(dbname, user, session, context=None):
             pool = _get_pool(dbname)
             Session = pool.get('ir.session')
             try:
-                sessions = Session.search([
-                        ('key', '=', session),
-                        ])
-                if not sessions:
-                    return
-                session, = sessions
-                name = session.create_uid.login
-                Session.delete(sessions)
+                name = Session.remove(session)
             except DatabaseOperationalError:
                 if count:
                     continue
                 raise
-        return name
+    logger.info("logout for '%s' from '%s' on database '%s'",
+        name, _get_remote_addr(context), dbname)
 
 
 def check(dbname, user, session, context=None):
@@ -71,13 +80,23 @@ def check(dbname, user, session, context=None):
             pool = _get_pool(dbname)
             Session = pool.get('ir.session')
             try:
-                if not Session.check(user, session):
-                    return
-                else:
-                    return user
+                find = Session.check(user, session)
+                break
             except DatabaseOperationalError:
                 if count:
                     continue
                 raise
             finally:
                 transaction.commit()
+    if find is None:
+        logger.error("session failed for '%s' from '%s' on database '%s'",
+            user, _get_remote_addr(context), dbname)
+        return
+    elif not find:
+        logger.info("session expired for '%s' from '%s' on database '%s'",
+            user, _get_remote_addr(context), dbname)
+        return
+    else:
+        logger.debug("session valid for '%s' from '%s' on database '%s'",
+            user, _get_remote_addr(context), dbname)
+        return user