From 1c32a02b644f584aacc04adbc0fb121f8fca44f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Wed, 7 Mar 2018 00:34:09 +0100 Subject: [PATCH] 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 + trytond/ir/session.py | 47 +++++++++++++++++++++++---- trytond/protocols/dispatcher.py | 8 +---- trytond/protocols/wrappers.py | 6 ++-- trytond/security.py | 57 ++++++++++++++++++++++----------- 5 files changed, 84 insertions(+), 35 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2c478dcc82..005fa97abd 100644 --- a/CHANGELOG +++ b/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 diff --git a/trytond/ir/session.py b/trytond/ir/session.py index 6f9541ebf8..c10adbb00d 100644 --- a/trytond/ir/session.py +++ b/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, {}) diff --git a/trytond/protocols/dispatcher.py b/trytond/protocols/dispatcher.py index d460561d9a..c874caf1e6 100644 --- a/trytond/protocols/dispatcher.py +++ b/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']) diff --git a/trytond/protocols/wrappers.py b/trytond/protocols/wrappers.py index 76eff6a9cd..ecb70b8b19 100644 --- a/trytond/protocols/wrappers.py +++ b/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 diff --git a/trytond/security.py b/trytond/security.py index 4b313fa021..2fa9ddb182 100644 --- a/trytond/security.py +++ b/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