From 29c00a56daa377e40f4d28e86a10f105617fd243 Mon Sep 17 00:00:00 2001 From: Toufik Benjaa Date: Tue, 20 Mar 2018 16:36:26 +0100 Subject: [PATCH] [IMP] http: Sessions implicit deactivation - Store a token inside sessions to allow implicit session deactivation when needed. backport of @da1f153d61d747d9357694382fe04f96c0ca886a @c8243e71c6da37547a19f61c58f25d5d03e13d38 --- addons/auth_crypt/auth_crypt.py | 5 ++++ addons/auth_oauth/res_users.py | 5 ++++ addons/web/controllers/main.py | 10 ++----- openerp/addons/base/res/res_users.py | 33 +++++++++++++++++++++ openerp/http.py | 44 ++++++++++++++++++---------- openerp/service/security.py | 11 +++++++ openerp/tests/common.py | 3 +- 7 files changed, 88 insertions(+), 23 deletions(-) diff --git a/addons/auth_crypt/auth_crypt.py b/addons/auth_crypt/auth_crypt.py index 7c9d0c5fa2c..c8ac4876a55 100644 --- a/addons/auth_crypt/auth_crypt.py +++ b/addons/auth_crypt/auth_crypt.py @@ -3,6 +3,7 @@ import logging from passlib.context import CryptContext import openerp +from openerp import api from openerp.osv import fields, osv from openerp.addons.base.res import res_users @@ -95,3 +96,7 @@ class res_users(osv.osv): internally """ return default_crypt_context + + @api.model + def _get_session_token_fields(self): + return super(res_users, self)._get_session_token_fields() | {'password_crypt'} diff --git a/addons/auth_oauth/res_users.py b/addons/auth_oauth/res_users.py index 60fdd7319a1..f89234a743e 100644 --- a/addons/auth_oauth/res_users.py +++ b/addons/auth_oauth/res_users.py @@ -6,6 +6,7 @@ import urllib2 import json import openerp +from openerp import api from openerp.addons.auth_signup.res_users import SignupError from openerp.osv import osv, fields from openerp import SUPERUSER_ID @@ -125,4 +126,8 @@ class res_users(osv.Model): if not res: raise + @api.model + def _get_session_token_fields(self): + return super(res_users, self)._get_session_token_fields() | {'oauth_access_token'} + # diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index a1ead7e5d1a..158ae695268 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -37,6 +37,7 @@ from openerp.tools.misc import str2bool, xlwt from openerp import http from openerp.http import request, serialize_exception as _serialize_exception, content_disposition from openerp.exceptions import AccessError, UserError +from openerp.service.report import exp_report, exp_report_get _logger = logging.getLogger(__name__) @@ -1479,7 +1480,6 @@ class Reports(http.Controller): def index(self, action, token): action = json.loads(action) - report_srv = request.session.proxy("report") context = dict(request.context) context.update(action["context"]) @@ -1492,15 +1492,11 @@ class Reports(http.Controller): report_ids = action['datas'].pop('ids') report_data.update(action['datas']) - report_id = report_srv.report( - request.session.db, request.session.uid, request.session.password, - action["report_name"], report_ids, - report_data, context) + report_id = exp_report(request.session.db, request.session.uid, action["report_name"], report_ids, report_data, context) report_struct = None while True: - report_struct = report_srv.report_get( - request.session.db, request.session.uid, request.session.password, report_id) + report_struct = exp_report_get(request.session.db, request.session.uid, report_id) if report_struct["state"]: break diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index 2c5bce22725..d573b9d4658 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -2,11 +2,13 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import itertools import logging +import hmac from functools import partial from itertools import repeat from lxml import etree from lxml.builder import E +from hashlib import sha256 import openerp from openerp import api @@ -378,6 +380,8 @@ class res_users(osv.osv): for id in ids: if id in self.__uid_cache[db]: del self.__uid_cache[db][id] + if any(key in values for key in self._get_session_token_fields(cr, uid)): + self._invalidate_session_cache(cr, uid) self.context_get.clear_cache(self) self.has_group.clear_cache(self) return res @@ -511,6 +515,35 @@ class res_users(osv.osv): finally: cr.close() + @api.model + def _get_session_token_fields(self): + return {'id', 'login', 'password', 'active'} + + @tools.ormcache('sid') + def _compute_session_token(self, sid): + """ Compute a session token given a session id and a user id """ + # retrieve the fields used to generate the session token + session_fields = ', '.join(sorted(self._get_session_token_fields())) + self.env.cr.execute("""SELECT %s, (SELECT value FROM ir_config_parameter WHERE key='database.secret') + FROM res_users + WHERE id=%%s""" % (session_fields), (self.id,)) + if self.env.cr.rowcount != 1: + self._invalidate_session_cache() + return False + data_fields = self.env.cr.fetchone() + # generate hmac key + key = (u'%s' % (data_fields,)).encode('utf-8') + # hmac the session id + data = sid.encode('utf-8') + h = hmac.new(key, data, sha256) + # keep in the cache the token + return h.hexdigest() + + @api.model + def _invalidate_session_cache(self): + """ Clear the session cache """ + self._compute_session_token.clear_cache(self) + def change_password(self, cr, uid, old_passwd, new_passwd, context=None): """Change current user password. Old password must be provided explicitly to prevent hijacking an existing user session, or for cases where the cleartext diff --git a/openerp/http.py b/openerp/http.py index 05778f0d290..22a9ce5ccf3 100644 --- a/openerp/http.py +++ b/openerp/http.py @@ -313,7 +313,15 @@ class WebRequest(object): # case, the request cursor is unusable. Rollback transaction to create a new one. if self._cr: self._cr.rollback() - self.env.clear() + # With the session patch, we now clear the environment only if it exists + # We do so by checking if the environment is stored in request.__dict__ + # Which is how lazy_property works. + # We do this, to avoid creating the environment before it is needed. + # For example some auth='none' controllers do "request.uid = request.session.uid" then + # "request.env.user ..." which is broken if we create the environment by doing self.env.clear() + # since it will not be linked to a user. + if self.__dict__.get('env'): + self.env.clear() result = self.endpoint(*a, **kw) if isinstance(result, Response) and result.is_qweb: # Early rendering of lazy responses to benefit from @service_model.check protection @@ -1123,7 +1131,7 @@ class OpenERPSession(werkzeug.contrib.sessions.Session): self.db = db self.uid = uid self.login = login - self.password = password + self.session_token = uid and security.compute_session_token(self, request.env) request.uid = uid request.disable_db = False @@ -1138,7 +1146,20 @@ class OpenERPSession(werkzeug.contrib.sessions.Session): """ if not self.db or not self.uid: raise SessionExpiredException("Session expired") - security.check(self.db, self.uid, self.password) + # We create our own environment instead of the request's one. + # This is due to the fact that the member "uid" on the request object isn't set yet. + # If we try to use the request's environment, the session checking will never succeed since + # the environment isn't bound to any user and it needs to be. + env = openerp.api.Environment(request.cr, self.uid, self.context) + # == BACKWARD COMPATIBILITY TO CONVERT OLD SESSION TYPE TO THE NEW ONES ! REMOVE ME AFTER 11.0 == + if self.get('password'): + security.check(self.db, self.uid, self.password) + self.session_token = security.compute_session_token(self, env) + self.pop('password') + # ================================================================================================= + # here we check if the session is still valid + if not security.check_session(self, env): + raise SessionExpiredException("Session expired") def logout(self, keep_db=False): for k in self.keys(): @@ -1151,7 +1172,7 @@ class OpenERPSession(werkzeug.contrib.sessions.Session): self.setdefault("db", None) self.setdefault("uid", None) self.setdefault("login", None) - self.setdefault("password", None) + self.setdefault("session_token", None) self.setdefault("context", {}) def get_context(self): @@ -1211,12 +1232,6 @@ class OpenERPSession(werkzeug.contrib.sessions.Session): @_login.setter def _login(self, value): self.login = value - @property - def _password(self): - return self.password - @_password.setter - def _password(self, value): - self.password = value def send(self, service_name, method, *args): """ @@ -1239,11 +1254,10 @@ class OpenERPSession(werkzeug.contrib.sessions.Session): Ensures this session is valid (logged into the openerp server) """ - if self.uid and not force: + if self.uid and self.session_token and not force: return - # TODO use authenticate instead of login - self.uid = self.proxy("common").login(self.db, self.login, self.password) - if not self.uid: + + if not self.uid or not security.check_session(self, request.env): raise AuthenticationError("Authentication failure") def ensure_valid(self): @@ -1272,7 +1286,7 @@ class OpenERPSession(werkzeug.contrib.sessions.Session): Use the registry and cursor in :data:`request` instead. """ self.assert_valid() - r = self.proxy('object').exec_workflow(self.db, self.uid, self.password, model, signal, id) + r = service_model.exec_workflow(self.db, self.uid, model, signal, id) return r def model(self, model): diff --git a/openerp/service/security.py b/openerp/service/security.py index 8ab38d3d659..28c85788a0a 100644 --- a/openerp/service/security.py +++ b/openerp/service/security.py @@ -11,3 +11,14 @@ def login(db, login, password): def check(db, uid, passwd): res_users = openerp.registry(db)['res.users'] return res_users.check(db, uid, passwd) + +def compute_session_token(session, env): + self = env['res.users'].browse(session.uid) + return self._compute_session_token(session.sid) + +def check_session(session, env): + self = env['res.users'].browse(session.uid) + if openerp.tools.misc.consteq(self._compute_session_token(session.sid), session.session_token): + return True + self._invalidate_session_cache() + return False diff --git a/openerp/tests/common.py b/openerp/tests/common.py index 3cb271e075d..866fbd012ff 100644 --- a/openerp/tests/common.py +++ b/openerp/tests/common.py @@ -26,6 +26,7 @@ import werkzeug import openerp from openerp import api +from openerp.service import security from openerp.modules.registry import RegistryManager _logger = logging.getLogger(__name__) @@ -291,7 +292,7 @@ class HttpCase(TransactionCase): session.db = db session.uid = uid session.login = user - session.password = password + session.session_token = uid and security.compute_session_token(session, self.env) session.context = Users.context_get(self.cr, uid) or {} session.context['uid'] = uid session._fix_lang(session.context)