From 950d962d95d214bdd1bdd64d8771af96d4d618cf Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 13 May 2019 12:31:37 +0000 Subject: [PATCH] [IMP] core: add env to various auth methods Allows accessing various keys, especially whether this is an interactive login or not. Also have the xml-rpc `login` delegate to `authenticate` instead of having its own half-assed implementation. And remove some dead code: as far as I can tell, Session.authenticate is never called with a uid. --- addons/auth_ldap/models/res_users.py | 8 ++--- addons/auth_oauth/models/res_users.py | 4 +-- .../website_sale_wishlist/models/res_users.py | 4 +-- odoo/addons/base/models/res_users.py | 31 ++++++++++--------- odoo/http.py | 25 +++++++-------- odoo/service/common.py | 13 +++----- odoo/service/security.py | 7 ----- odoo/tests/common.py | 5 +-- 8 files changed, 42 insertions(+), 55 deletions(-) diff --git a/addons/auth_ldap/models/res_users.py b/addons/auth_ldap/models/res_users.py index 637ecd58507..29bbb147a8e 100644 --- a/addons/auth_ldap/models/res_users.py +++ b/addons/auth_ldap/models/res_users.py @@ -10,9 +10,9 @@ class Users(models.Model): _inherit = "res.users" @classmethod - def _login(cls, db, login, password): + def _login(cls, db, login, password, user_agent_env): try: - return super(Users, cls)._login(db, login, password) + return super(Users, cls)._login(db, login, password, user_agent_env=user_agent_env) except AccessDenied as e: with registry(db).cursor() as cr: cr.execute("SELECT id FROM res_users WHERE lower(login)=%s", (login,)) @@ -28,9 +28,9 @@ class Users(models.Model): return Ldap._get_or_create_user(conf, login, entry) raise e - def _check_credentials(self, password): + def _check_credentials(self, password, env): try: - super(Users, self)._check_credentials(password) + return super(Users, self)._check_credentials(password, env) except AccessDenied: if self.env.user.active: Ldap = self.env['res.company.ldap'] diff --git a/addons/auth_oauth/models/res_users.py b/addons/auth_oauth/models/res_users.py index 0e54a23b0aa..2ee46b6b38c 100644 --- a/addons/auth_oauth/models/res_users.py +++ b/addons/auth_oauth/models/res_users.py @@ -109,9 +109,9 @@ class ResUsers(models.Model): # return user credentials return (self.env.cr.dbname, login, access_token) - def _check_credentials(self, password): + def _check_credentials(self, password, env): try: - return super(ResUsers, self)._check_credentials(password) + return super(ResUsers, self)._check_credentials(password, env) except AccessDenied: res = self.sudo().search([('id', '=', self.env.uid), ('oauth_access_token', '=', password)]) if not res: diff --git a/addons/website_sale_wishlist/models/res_users.py b/addons/website_sale_wishlist/models/res_users.py index 6aeec156c04..d1887d170ac 100644 --- a/addons/website_sale_wishlist/models/res_users.py +++ b/addons/website_sale_wishlist/models/res_users.py @@ -5,9 +5,9 @@ from odoo.http import request class ResUsers(models.Model): _inherit = "res.users" - def _check_credentials(self, password): + def _check_credentials(self, password, env): """Make all wishlists from session belong to its owner user.""" - result = super(ResUsers, self)._check_credentials(password) + result = super(ResUsers, self)._check_credentials(password, env) if request and request.session.get('wishlist_ids'): self.env["product.wishlist"]._check_wishlist_from_session() return result diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index ab48d6270dd..a988b450408 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -318,7 +318,7 @@ class Users(models.Model): ) self.invalidate_cache(['password'], [uid]) - def _check_credentials(self, password): + def _check_credentials(self, password, env): """ Validates the current user's password. Override this method to plug additional authentication methods. @@ -630,7 +630,7 @@ class Users(models.Model): return self._order @classmethod - def _login(cls, db, login, password): + def _login(cls, db, login, password, user_agent_env): if not password: raise AccessDenied() ip = request.httprequest.environ['REMOTE_ADDR'] if request else 'n/a' @@ -642,7 +642,7 @@ class Users(models.Model): if not user: raise AccessDenied() user = user.with_user(user) - user._check_credentials(password) + user._check_credentials(password, user_agent_env) tz = request.httprequest.cookies.get('tz') if request else None if tz in pytz.all_timezones and (not user.tz or not user.login_date): # first login or missing tz -> set tz to browser tz @@ -667,7 +667,7 @@ class Users(models.Model): :param dict user_agent_env: environment dictionary describing any relevant environment attributes """ - uid = cls._login(db, login, password) + uid = cls._login(db, login, password, user_agent_env=user_agent_env) if user_agent_env and user_agent_env.get('base_location'): with cls.pool.cursor() as cr: env = api.Environment(cr, uid, {}) @@ -693,14 +693,13 @@ class Users(models.Model): db = cls.pool.db_name if cls.__uid_cache[db].get(uid) == passwd: return - cr = cls.pool.cursor() - try: + with contextlib.closing(cls.pool.cursor()) as cr: self = api.Environment(cr, uid, {})[cls._name] with self._assert_can_auth(): - self._check_credentials(passwd) + if not self.env.user.active: + raise AccessDenied() + self._check_credentials(passwd, {'interactive': False}) cls.__uid_cache[db][uid] = passwd - finally: - cr.close() def _get_session_token_fields(self): return {'id', 'login', 'password', 'active'} @@ -739,11 +738,15 @@ class Users(models.Model): :raise: odoo.exceptions.AccessDenied when old password is wrong :raise: odoo.exceptions.UserError when new password is not set or empty """ - self.check(self._cr.dbname, self._uid, old_passwd) - if new_passwd: - # use self.env.user here, because it has uid=SUPERUSER_ID - return self.env.user.write({'password': new_passwd}) - raise UserError(_("Setting empty passwords is not allowed for security reasons!")) + if not old_passwd: + raise AccessDenied() + if not new_passwd: + raise UserError(_("Setting empty passwords is not allowed for security reasons!")) + + # alternatively: use identitycheck wizard? + self._check_credentials(old_passwd, {'interactive': True}) + # use self.env.user here, because it has uid=SUPERUSER_ID + return self.env.user.write({'password': new_passwd}) def preference_save(self): return { diff --git a/odoo/http.py b/odoo/http.py index dcd36d06f81..40da5a5b131 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -974,7 +974,7 @@ class OpenERPSession(sessions.Session): return self.__setitem__(k, v) object.__setattr__(self, k, v) - def authenticate(self, db, login=None, password=None, uid=None): + def authenticate(self, db, login=None, password=None): """ Authenticate the current user with the given db, login and password. If successful, store the authentication parameters in the @@ -984,25 +984,24 @@ class OpenERPSession(sessions.Session): to authenticate the user. """ - if uid is None: - wsgienv = request.httprequest.environ - env = dict( - base_location=request.httprequest.url_root.rstrip('/'), - HTTP_HOST=wsgienv['HTTP_HOST'], - REMOTE_ADDR=wsgienv['REMOTE_ADDR'], - ) - uid = odoo.registry(db)['res.users'].authenticate(db, login, password, env) - else: - security.check(db, uid, password) + wsgienv = request.httprequest.environ + env = dict( + interactive=True, + base_location=request.httprequest.url_root.rstrip('/'), + HTTP_HOST=wsgienv['HTTP_HOST'], + REMOTE_ADDR=wsgienv['REMOTE_ADDR'], + ) + uid = odoo.registry(db)['res.users'].authenticate(db, login, password, env) + self.rotate = True self.db = db self.uid = uid self.login = login - self.session_token = uid and security.compute_session_token(self, request.env) + self.session_token = security.compute_session_token(self, request.env) request.uid = uid request.disable_db = False - if uid: self.get_context() + self.get_context() return uid def check_security(self): diff --git a/odoo/service/common.py b/odoo/service/common.py index 42ec4ca3738..fa435c5197a 100644 --- a/odoo/service/common.py +++ b/odoo/service/common.py @@ -7,8 +7,6 @@ import odoo.tools from odoo.exceptions import AccessDenied from odoo.tools.translate import _ -from . import security - _logger = logging.getLogger(__name__) RPC_VERSION_1 = { @@ -19,17 +17,14 @@ RPC_VERSION_1 = { } def exp_login(db, login, password): - # TODO: legacy indirection through 'security', should use directly - # the res.users model - res = security.login(db, login, password) - msg = res and 'successful login' or 'bad login or password' - _logger.info("%s from '%s' using database '%s'", msg, login, db.lower()) - return res or False + return exp_authenticate(db, login, password, None) def exp_authenticate(db, login, password, user_agent_env): + if user_agent_env is None: + user_agent_env = {} res_users = odoo.registry(db)['res.users'] try: - return res_users.authenticate(db, login, password, user_agent_env) + return res_users.authenticate(db, login, password, {**user_agent_env, 'interactive': False}) except AccessDenied: return False diff --git a/odoo/service/security.py b/odoo/service/security.py index 703fd3687f3..23d89b6c691 100644 --- a/odoo/service/security.py +++ b/odoo/service/security.py @@ -4,13 +4,6 @@ import odoo import odoo.exceptions -def login(db, login, password): - res_users = odoo.registry(db)['res.users'] - try: - return res_users._login(db, login, password) - except odoo.exceptions.AccessDenied: - return False - def check(db, uid, passwd): res_users = odoo.registry(db)['res.users'] return res_users.check(db, uid, passwd) diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 191f25003a3..71418c5cc2f 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -1319,12 +1319,9 @@ class HttpCaseCommon(BaseCase): return db = get_db_name() - uid = self.registry['res.users'].authenticate(db, user, password, None) + uid = self.registry['res.users'].authenticate(db, user, password, {'interactive': False}) env = api.Environment(self.cr, uid, {}) - # self.session.authenticate(db, user, password, uid=uid) - # OpenERPSession.authenticate accesses the current request, which we - # don't have, so reimplement it manually... session = self.session session.db = db