From 950d962d95d214bdd1bdd64d8771af96d4d618cf Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 13 May 2019 12:31:37 +0000 Subject: [PATCH 01/13] [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 From dccbf425a1742c08564d4f7b223ce27f8746c832 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 6 Mar 2020 14:45:21 +0000 Subject: [PATCH 02/13] [FIX] core: ensure we create a new session for each tour The browser itself would get mostly cleaned up between tours, but the session object would not get cleaned, and apparently in some cases that could lead to an incoherent session: a tour would add data to the session which the next tour (logging in as a different user) would not (fully) override, leading to a session inconsistency and a Session Expired exception during the tour. Fix by not storing the session on the test object, the session is created during authentication then set on the opener & browser. --- .../account/tests/test_portal_attachment.py | 2 + addons/test_xlsx_export/tests/test_export.py | 4 +- addons/website/tests/test_page.py | 1 + odoo/tests/common.py | 55 +++++++++++-------- 4 files changed, 36 insertions(+), 26 deletions(-) diff --git a/addons/account/tests/test_portal_attachment.py b/addons/account/tests/test_portal_attachment.py index 02ac0cf04a7..2340fc75f2a 100644 --- a/addons/account/tests/test_portal_attachment.py +++ b/addons/account/tests/test_portal_attachment.py @@ -32,6 +32,8 @@ class TestPortalAttachment(AccountTestInvoicingHttpCommon): def test_01_portal_attachment(self): """Test the portal chatter attachment route.""" + self.authenticate(None, None) + # Test public user can't create attachment without token of document res = self.url_open( url='%s/portal/attachment/add' % self.base_url, diff --git a/addons/test_xlsx_export/tests/test_export.py b/addons/test_xlsx_export/tests/test_export.py index df0f85a5a4c..5ba8c0a0de5 100644 --- a/addons/test_xlsx_export/tests/test_export.py +++ b/addons/test_xlsx_export/tests/test_export.py @@ -22,8 +22,8 @@ class XlsxCreatorCase(common.HttpCase): super().setUp() self.model = self.env[self.model_name] - u = mail_new_test_user(self.env, login='fof', password='123456789', groups='base.group_user,base.group_allow_export') - self.authenticate('fof', '123456789') + mail_new_test_user(self.env, login='fof', password='123456789', groups='base.group_user,base.group_allow_export') + self.session = self.authenticate('fof', '123456789') self.worksheet = {} # mock worksheet diff --git a/addons/website/tests/test_page.py b/addons/website/tests/test_page.py index eb7a1ba9976..7d7d0928518 100644 --- a/addons/website/tests/test_page.py +++ b/addons/website/tests/test_page.py @@ -222,6 +222,7 @@ class WithContext(HttpCase): specific_page = self.page.copy({'website_id': self.env['website'].get_current_website().id}) specific_page.write({'is_published': False, 'arch': self.page.arch.replace('I am a generic page', 'I am a specific page')}) + self.authenticate(None, None) r = self.url_open(specific_page.url) self.assertEqual(r.status_code, 404, "Restricted users should see a 404 and not the generic one as we unpublished the specific one") diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 71418c5cc2f..066bb067148 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -1255,14 +1255,8 @@ class HttpCaseCommon(BaseCase): if self.registry_test_mode: self.registry.enter_test_mode(self.cr) self.addCleanup(self.registry.leave_test_mode) - # setup a magic session_id that will be rollbacked - self.session = odoo.http.root.session_store.new() - self.session_id = self.session.sid - self.session.db = get_db_name() - odoo.http.root.session_store.save(self.session) # setup an url opener helper self.opener = requests.Session() - self.opener.cookies['session_id'] = self.session_id @classmethod def start_browser(cls): @@ -1311,31 +1305,44 @@ class HttpCaseCommon(BaseCase): odoo.http.root.session_store.save(self.session) def authenticate(self, user, password): - # stay non-authenticated - if user is None: - if self.session: - odoo.http.root.session_store.delete(self.session) - self.browser.delete_cookie('session_id', domain=HOST) - return - db = get_db_name() - uid = self.registry['res.users'].authenticate(db, user, password, {'interactive': False}) - env = api.Environment(self.cr, uid, {}) - - session = self.session + if getattr(self, 'session', None): + odoo.http.root.session_store.delete(self.session) + self.session = session = odoo.http.root.session_store.new() session.db = db - session.uid = uid - session.login = user - session.session_token = uid and security.compute_session_token(session, env) - session.context = dict(env['res.users'].context_get() or {}) - session.context['uid'] = uid - session._fix_lang(session.context) + + if user: # if authenticated + uid = self.registry['res.users'].authenticate(db, user, password, {'interactive': False}) + env = api.Environment(self.cr, uid, {}) + session.uid = uid + session.login = user + session.session_token = uid and security.compute_session_token(session, env) + session.context = dict(env['res.users'].context_get() or {}) + session.context['uid'] = uid + session._fix_lang(session.context) odoo.http.root.session_store.save(session) + # Reset the opener: turns out when we set cookies['foo'] we're really + # setting a cookie on domain='' path='/'. + # + # But then our friendly neighborhood server might set a cookie for + # domain='localhost' path='/' (with the same value) which is considered + # a *different* cookie following ours rather than the same. + # + # When we update our cookie, it's done in-place, so the server-set + # cookie is still present and (as it follows ours and is more precise) + # very likely to still be used, therefore our session change is ignored. + # + # An alternative would be to set the cookie to None (unsetting it + # completely) or clear-ing session.cookies. + self.opener = requests.Session() + self.opener.cookies['session_id'] = session.sid if self.browser: self._logger.info('Setting session cookie in browser') - self.browser.set_cookie('session_id', self.session_id, '/', HOST) + self.browser.set_cookie('session_id', session.sid, '/', HOST) + + return session def browser_js(self, url_path, code, ready='', login=None, timeout=60, **kw): """ Test js code running in the browser From 6e6946577307e569319661d413952f7b61bafd57 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 28 Feb 2020 14:01:21 +0000 Subject: [PATCH 03/13] [ADD] base: enhanced security mode for users Similar to github's sudo mode, my understanding of the intent is to avoid third parties being able to perform sensitive / serious operations when leaving a logged-in machine unattended. This v1 only handles protecting "action methods" (methods called through buttons and intended to return action descriptors), although they can be used to "lock out" any methods they won't be able to prompt and the caller will likely be surprised. --- odoo/addons/base/models/res_users.py | 70 +++++++++++++++++-- odoo/addons/base/security/base_security.xml | 6 ++ odoo/addons/base/security/ir.model.access.csv | 2 + odoo/addons/base/views/res_users_views.xml | 32 +++++++++ 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index a988b450408..6f7509ac133 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -1,30 +1,33 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import contextlib import base64 -import pytz +import contextlib import datetime +import hmac import ipaddress import itertools +import json import logging -import hmac - +import time from collections import defaultdict from hashlib import sha256 from itertools import chain, repeat + +import decorator +import passlib.context +import pytz from lxml import etree from lxml.builder import E -import passlib.context from odoo import api, fields, models, tools, SUPERUSER_ID, _ from odoo.addons.base.models.ir_model import MODULE_UNINSTALL_FLAG from odoo.exceptions import AccessDenied, AccessError, UserError, ValidationError from odoo.http import request +from odoo.modules.module import get_module_resource from odoo.osv import expression from odoo.service.db import check_super from odoo.tools import partition, collections, frozendict, lazy_property, image_process -from odoo.modules.module import get_module_resource _logger = logging.getLogger(__name__) @@ -83,6 +86,38 @@ def parse_m2m(commands): ids.append(command) return ids +@decorator.decorator +def check_identity(fn, self): + """ Wrapped method should be an *action method* (called from a button + type=object), and requires extra security to be executed. This decorator + checks if the identity (password) has been checked in the last 10mn, and + pops up an identity check wizard if not. + + Prevents access outside of interactive contexts (aka with a request) + """ + if not request: + raise UserError(_("This method can only be accessed over HTTP")) + + if request.session.get('identity-check-last', 0) > time.time() - 10 * 60: + # update identity-check-last like github? + return fn(self) + + w = self.sudo().env['res.users.identitycheck'].create({ + 'request': json.dumps([ # assume self.env.context can be serialised to json + self.env.context, + self._name, + self.ids, + fn.__name__ + ]) + }) + return { + 'type': 'ir.actions.act_window', + 'res_model': 'res.users.identitycheck', + 'res_id': w.id, + 'target': 'new', + 'views': [(False, 'form')], + } + #---------------------------------------------------------- # Basic res.groups and res.users #---------------------------------------------------------- @@ -1420,6 +1455,29 @@ class UsersView(models.Model): } return res +class CheckIdentity(models.TransientModel): + _name = 'res.users.identitycheck' + _description = """ Wizard used to re-check the user's credentials (password) + +Might be useful before the more security-sensitive operations, users might be +leaving their computer unlocked & unattended. Re-checking credentials mitigates +some of the risk of a third party using such an unattended device to manipulate +the account. +""" + + request = fields.Char(readonly=True, groups='.') + password = fields.Char(required=True) + + def run_check(self): + if not request: + raise UserError(_("This method can only be accessed over HTTP")) + self.create_uid._check_credentials(self.password, {'interactive': True}) + self.password = False + + request.session['identity-check-last'] = time.time() + ctx, model, ids, method = json.loads(self.sudo().request) + return getattr(self.env(context=ctx)[model].browse(ids), method)() + #---------------------------------------------------------- # change password wizard #---------------------------------------------------------- diff --git a/odoo/addons/base/security/base_security.xml b/odoo/addons/base/security/base_security.xml index c2cb464bec4..e09ee4c6f94 100644 --- a/odoo/addons/base/security/base_security.xml +++ b/odoo/addons/base/security/base_security.xml @@ -171,6 +171,12 @@ [(1,'=',1)] + + users can only access their own id check + + [('create_uid', '=', user.id)] + + user rule diff --git a/odoo/addons/base/security/ir.model.access.csv b/odoo/addons/base/security/ir.model.access.csv index 5f787999036..9c10ed68115 100644 --- a/odoo/addons/base/security/ir.model.access.csv +++ b/odoo/addons/base/security/ir.model.access.csv @@ -69,6 +69,8 @@ "access_res_users_all","res_users all","model_res_users",,1,0,0,0 "access_res_users_group_erp_manager","res_users group_erp_manager","model_res_users","group_erp_manager",1,1,1,1 "access_res_users_log_all","res_users_log_all","model_res_users_log",,1,0,1,0 +"access_res_users_identitycheck_employee","id check employees","model_res_users_identitycheck","group_user",1,1,1,0 +"access_res_users_identitycheck_portal","id check portal","model_res_users_identitycheck","group_portal",1,1,1,0 "access_ir_actions_all","ir_actions_all","model_ir_actions_actions",,1,0,0,0 "access_ir_actions_group_system","ir_actions_group_system","model_ir_actions_actions","group_system",1,1,1,1 "access_ir_actions_act_window_all","ir_actions_act_window_all","model_ir_actions_act_window",,1,0,0,0 diff --git a/odoo/addons/base/views/res_users_views.xml b/odoo/addons/base/views/res_users_views.xml index 812bc8435ff..9b977a9832e 100644 --- a/odoo/addons/base/views/res_users_views.xml +++ b/odoo/addons/base/views/res_users_views.xml @@ -35,6 +35,38 @@ + + res.users.identitycheck + +
+ +
+
+
+
+   + Security Control +
+
+ Forgot password? +

Please confirm your password to continue

+

+ This is necessary for security-related changes. The authorization will last for a few minutes. +

+
+
+
+
+
+
+
+ +
+ + + res.groups.search From 083c70bbb63f27839b2c9a4b549e216947bc4dd1 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 21 May 2019 11:58:28 +0000 Subject: [PATCH 04/13] [IMP] core: replace dedicated uid cache by ormcache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this, invalidations to the UID cache is not synchronised between workers because it's an ad-hoc solution (so a user changing their password or an admin disabling a user would only lock out an attacker currently using the API of one of possibly several workers). Shift the entire thing to ormcache which already has proper support for synchronising cache invalidation between workers. Also simplify the cache invalidation mess in Users.write because the caches have been unified into a single registry-level LRU, so the half-dozen cache clears on specific ormcached methods & models is pretty much the same as repeatedly calling clear_caches on the current model. **However** registry.cache is trivially accessible from server actions and safe_eval as long as they provide access to a model (through `model.pool.cache`). Which is common, and an issue given we're very much putting sensible data in there. Fix this by renaming `Registry.cache` to `Registry.__cache`, this requires few editions and mangled names are not accessible from safe_eval contexts. The alternative would have been to add more bespoke handling of the uid cache to hook it into the cache invalidation propagation machinery. After discussion with (@)odony, fixing LRU access and using that seems cleaner and less error-prone. Note on lazy_property ===================== Make Registry.cache / Registry.__cache into a regular attribute: the overhead of the LRU is not that high (compared to that of the registry itself), it's rare that we *don't* need it, and it's assumed to be a persisted attribute (it's not just a cache) so making it a normal attribute seems fine; and lazy_property doesn't work for mangled names: the name of the property is mangled using the name of the definition class, but the name of the symbol (fget) is not mangled so lazy_property would set the __cache attribute but then Python would lookup _Registry__cache, creating a new cache every access. And we can't (always) mangle things correctly on `__get__(obj, owner)`: `owner` is just `type(obj)`, meaning in the case of inheritance the type we get is the type through which the property is accessed rather than the one it's defined on. So it would work in the cases where no inheritance is involved (such as Registry.__cache) but not in general (lest we want to play around walking the MRO ourselves to find the definition source, which doesn't seem worth it). lazy_property *could* be made to work properly on Python 3.6+: the descriptor protocol gains `__set_name__(name, owner)`, which is called with the properly mangled name — and with the definition class to boot (though there might still be issues when overriding lazy properties as the override will be mangled & named differently... or maybe that's a feature?). However we're still supporting 3.5 at this point, AFAIK, so that's not an option. Plus it feels unnecessary / not very useful. However add an assertion to `lazy_property` so it signals when we try to use it on a mangled method (as otherwise it kinda sorta work in the sense that the property / object is accessible but is in effect a slower way to write a regular property). --- addons/web/controllers/main.py | 2 +- odoo/addons/base/models/res_users.py | 39 +++++++++++----------------- odoo/modules/registry.py | 13 ++++------ odoo/service/security.py | 1 - odoo/tools/cache.py | 4 +-- odoo/tools/func.py | 2 ++ 6 files changed, 25 insertions(+), 36 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index 383308061de..b2f9b268dea 100644 --- a/addons/web/controllers/main.py +++ b/addons/web/controllers/main.py @@ -918,7 +918,7 @@ class Home(http.Controller): uid = request.env.user.id if request.env.user._is_system(): uid = request.session.uid = odoo.SUPERUSER_ID - request.env['res.users']._invalidate_session_cache() + request.env['res.users'].clear_caches() request.session.session_token = security.compute_session_token(request.session, request.env) return http.local_redirect(self._login_redirect(uid), keep_hash=True) diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index 6f7509ac133..a54c9c2b21f 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -249,7 +249,6 @@ class Users(models.Model): _description = 'Users' _inherits = {'res.partner': 'partner_id'} _order = 'name, login' - __uid_cache = defaultdict(dict) # {dbname: {uid: password}} # User can write on a few of his own fields (but not his groups for example) SELF_WRITEABLE_FIELDS = ['signature', 'action_id', 'company_id', 'email', 'name', 'image_1920', 'lang', 'tz'] @@ -580,26 +579,24 @@ class Users(models.Model): # DLE P139: Calling invalidate_cache on a new, well you lost everything as you wont be able to take it back from the cache # `test_00_equipment_multicompany_user` self.env['ir.model.access'].call_cache_clearing_methods() - self.env['ir.rule'].clear_caches() - self.has_group.clear_cache(self) - if any(key.startswith('context_') or key in ('lang', 'tz') for key in values): - self.context_get.clear_cache(self) - if any(key in values for key in ['active'] + USER_PRIVATE_FIELDS): - db = self._cr.dbname - for id in self.ids: - self.__uid_cache[db].pop(id, None) - if any(key in values for key in self._get_session_token_fields()): - self._invalidate_session_cache() + + # per-method / per-model caches have been removed so the various + # clear_cache/clear_caches methods pretty much just end up calling + # Registry._clear_cache + invalidation_fields = { + 'groups_id', 'active', 'lang', 'tz', 'company_id', + *USER_PRIVATE_FIELDS, + *self._get_session_token_fields() + } + if (invalidation_fields & values.keys()) or any(key.startswith('context_') for key in values): + self.clear_caches() return res def unlink(self): if SUPERUSER_ID in self.ids: raise UserError(_('You can not remove the admin user as it is used internally for resources created by Odoo (updates, module installation, ...)')) - db = self._cr.dbname - for id in self.ids: - self.__uid_cache[db].pop(id, None) - self._invalidate_session_cache() + self.clear_caches() return super(Users, self).unlink() @api.model @@ -719,22 +716,20 @@ class Users(models.Model): return uid @classmethod + @tools.ormcache('uid', 'passwd') def check(cls, db, uid, passwd): """Verifies that the given (uid, password) is authorized for the database ``db`` and raise an exception if it is not.""" if not passwd: # empty passwords disallowed for obvious security reasons raise AccessDenied() - db = cls.pool.db_name - if cls.__uid_cache[db].get(uid) == passwd: - return + with contextlib.closing(cls.pool.cursor()) as cr: self = api.Environment(cr, uid, {})[cls._name] with self._assert_can_auth(): if not self.env.user.active: raise AccessDenied() self._check_credentials(passwd, {'interactive': False}) - cls.__uid_cache[db][uid] = passwd def _get_session_token_fields(self): return {'id', 'login', 'password', 'active'} @@ -748,7 +743,7 @@ class Users(models.Model): FROM res_users WHERE id=%%s""" % (session_fields), (self.id,)) if self.env.cr.rowcount != 1: - self._invalidate_session_cache() + self.clear_caches() return False data_fields = self.env.cr.fetchone() # generate hmac key @@ -759,10 +754,6 @@ class Users(models.Model): # keep in the cache the token return h.hexdigest() - def _invalidate_session_cache(self): - """ Clear the sessions cache """ - self._compute_session_token.clear_cache(self) - @api.model def change_password(self, old_passwd, new_passwd): """Change current user password. Old password must be provided explicitly diff --git a/odoo/modules/registry.py b/odoo/modules/registry.py index f660fccf52c..19b631fc4c3 100644 --- a/odoo/modules/registry.py +++ b/odoo/modules/registry.py @@ -114,6 +114,7 @@ class Registry(Mapping): self._fields_by_model = None self._ordinary_tables = None self._constraint_queue = deque() + self.__cache = LRU(8192) # modules fully loaded (maintained during init phase by `loading` module) self._init_modules = set() @@ -218,6 +219,7 @@ class Registry(Mapping): """ from .. import models + self.clear_caches() lazy_property.reset_all(self) # Instantiate registered classes (via the MetaModel automatic discovery @@ -234,6 +236,7 @@ class Registry(Mapping): """ Complete the setup of models. This must be called after loading modules and before using the ORM. """ + self.clear_caches() lazy_property.reset_all(self) self.registry_invalidated = True env = odoo.api.Environment(cr, SUPERUSER_ID, {}) @@ -504,15 +507,9 @@ class Registry(Mapping): for table in missing_tables: _logger.error("Model %s has no table.", table2model[table]) - @lazy_property - def cache(self): - """ A cache for model methods. """ - # this lazy_property is automatically reset by lazy_property.reset_all() - return LRU(8192) - def _clear_cache(self): """ Clear the cache and mark it as invalidated. """ - self.cache.clear() + self.__cache.clear() self.cache_invalidated = True def clear_caches(self): @@ -618,7 +615,7 @@ class Registry(Mapping): self.setup_models(cr) self.registry_invalidated = False if self.cache_invalidated: - self.cache.clear() + self.__cache.clear() self.cache_invalidated = False @contextmanager diff --git a/odoo/service/security.py b/odoo/service/security.py index 23d89b6c691..c247c6d9ac5 100644 --- a/odoo/service/security.py +++ b/odoo/service/security.py @@ -17,5 +17,4 @@ def check_session(session, env): expected = self._compute_session_token(session.sid) if expected and odoo.tools.misc.consteq(expected, session.session_token): return True - self._invalidate_session_cache() return False diff --git a/odoo/tools/cache.py b/odoo/tools/cache.py index 0e244f63a4b..46652736b93 100644 --- a/odoo/tools/cache.py +++ b/odoo/tools/cache.py @@ -76,7 +76,7 @@ class ormcache(object): def lru(self, model): counter = STAT[(model.pool.db_name, model._name, self.method)] - return model.pool.cache, (model._name, self.method), counter + return model.pool._Registry__cache, (model._name, self.method), counter def lookup(self, method, *args, **kwargs): d, key0, counter = self.lru(args[0]) @@ -207,7 +207,7 @@ def log_ormcache_stats(sig=None, frame=None): for dbname, reg in sorted(Registry.registries.d.items()): # set logger prefix to dbname me.dbname = dbname - entries = Counter(k[:2] for k in reg.cache.d) + entries = Counter(k[:2] for k in reg._Registry__cache.d) # show entries sorted by model name, method name for key in sorted(entries, key=lambda key: (key[0], key[1].__name__)): model, method = key diff --git a/odoo/tools/func.py b/odoo/tools/func.py index deae2bdec04..17b5f85ab24 100644 --- a/odoo/tools/func.py +++ b/odoo/tools/func.py @@ -16,6 +16,8 @@ class lazy_property(object): get it again. """ def __init__(self, fget): + assert not fget.__name__.startswith('__'),\ + "lazy_property does not support mangled names" self.fget = fget def __get__(self, obj, cls): From 9f82605df111b096fc197a809860762e968cdeef Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 5 Jun 2019 09:03:06 +0000 Subject: [PATCH 05/13] [FIX] portal, website: login redirection * migrate website to overriding web_login less (still needed to flag it as website-enabled) and use _login_redirect for its login redirection override needs * modify portal and website to not replace / shortcut the redirection workflow, so it's possible to override those properly if / as necessary --- addons/portal/controllers/web.py | 2 +- addons/website/controllers/main.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/addons/portal/controllers/web.py b/addons/portal/controllers/web.py index d1c2c013aff..9e6e6175b32 100644 --- a/addons/portal/controllers/web.py +++ b/addons/portal/controllers/web.py @@ -16,7 +16,7 @@ class Home(Home): def _login_redirect(self, uid, redirect=None): if not redirect and not request.env['res.users'].sudo().browse(uid).has_group('base.group_user'): - return '/my' + redirect = '/my' return super(Home, self)._login_redirect(uid, redirect=redirect) @http.route('/web', type='http', auth="none") diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py index 84c169358e6..8f2bc697c62 100644 --- a/addons/website/controllers/main.py +++ b/addons/website/controllers/main.py @@ -95,17 +95,20 @@ class Website(Home): # while portal users are redirected to the frontend by default # ------------------------------------------------------ - @http.route(website=True, auth="public", sitemap=False) - def web_login(self, redirect=None, *args, **kw): - response = super(Website, self).web_login(redirect=redirect, *args, **kw) + def _login_redirect(self, uid, redirect=None): + """ Redirect regular users (employees) to the backend) and others to + the frontend + """ if not redirect and request.params['login_success']: - if request.env['res.users'].browse(request.uid).has_group('base.group_user'): + if request.env['res.users'].browse(uid).has_group('base.group_user'): redirect = b'/web?' + request.httprequest.query_string else: redirect = '/my' - return http.redirect_with_hash(redirect) - return response + return super()._login_redirect(uid, redirect=redirect) + @http.route(website=True, auth="public", sitemap=False) + def web_login(self, *args, **kw): + return super().web_login(*args, **kw) # ------------------------------------------------------ # Business # ------------------------------------------------------ From d0dbbe23f4b1f797a9ed8c7a4726146d6df902ad Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 22 May 2019 14:12:33 +0000 Subject: [PATCH 06/13] [ADD] base: API keys support * ability for a user to request / create keys associated to their user * overrides can block RPC solely through API keys, by overriding `_rpc_api_keys_only()` (to require API auth even in situations where the user has not requested it themselves) * hash keys just in case as we can do so and might as well, add a cleartext index (first 4 bytes of 20) to avoid blowing up the DB if a user decides to create millions of keys for some daft reason * users can delete their own keys, admins can delete (invalidate) anyone's keys * `scope` on API keys can be used to restrict usage to certain kind of applications, so API keys can be used for other things than global authentication. New keys manually created by users have no scope by default so they are valid everywhere (global keys). RPC auth (stateless XML-RPC/JSON-RPC) requires global keys Co-authored-by: Florimond Husquinet Co-authored-by: Olivier Dony --- .../views/res_config_settings_views.xml | 7 + odoo/addons/base/models/res_users.py | 176 ++++++++++++++++-- odoo/addons/base/security/base_security.xml | 23 +++ odoo/addons/base/security/ir.model.access.csv | 5 + odoo/addons/base/tests/test_xmlrpc.py | 112 +++++++++++ odoo/addons/base/views/res_users_views.xml | 104 ++++++++++- odoo/addons/test_apikeys/__init__.py | 0 odoo/addons/test_apikeys/__manifest__.py | 8 + .../test_apikeys/static/tests/apikey_flow.js | 114 ++++++++++++ odoo/addons/test_apikeys/tests/__init__.py | 1 + odoo/addons/test_apikeys/tests/test_flow.py | 37 ++++ odoo/addons/test_apikeys/views/assets.xml | 7 + 12 files changed, 581 insertions(+), 13 deletions(-) create mode 100644 odoo/addons/test_apikeys/__init__.py create mode 100644 odoo/addons/test_apikeys/__manifest__.py create mode 100644 odoo/addons/test_apikeys/static/tests/apikey_flow.js create mode 100644 odoo/addons/test_apikeys/tests/__init__.py create mode 100644 odoo/addons/test_apikeys/tests/test_flow.py create mode 100644 odoo/addons/test_apikeys/views/assets.xml diff --git a/addons/base_setup/views/res_config_settings_views.xml b/addons/base_setup/views/res_config_settings_views.xml index bda9ab28a96..ab4002a54ab 100644 --- a/addons/base_setup/views/res_config_settings_views.xml +++ b/addons/base_setup/views/res_config_settings_views.xml @@ -180,6 +180,13 @@ +
+
+
+
+
diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index a54c9c2b21f..2ae88636719 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import base64 +import binascii import contextlib import datetime import hmac @@ -9,6 +10,7 @@ import ipaddress import itertools import json import logging +import os import time from collections import defaultdict from hashlib import sha256 @@ -114,6 +116,7 @@ def check_identity(fn, self): 'type': 'ir.actions.act_window', 'res_model': 'res.users.identitycheck', 'res_id': w.id, + 'name': _("Security Control"), 'target': 'new', 'views': [(False, 'form')], } @@ -771,6 +774,10 @@ class Users(models.Model): # alternatively: use identitycheck wizard? self._check_credentials(old_passwd, {'interactive': True}) + + ip = request.httprequest.environ['REMOTE_ADDR'] if request else 'n/a' + _logger.info("Password change for '%s' (#%s) from %s", self.env.user.login, self.env.uid, ip) + # use self.env.user here, because it has uid=SUPERUSER_ID return self.env.user.write({'password': new_passwd}) @@ -1447,21 +1454,21 @@ class UsersView(models.Model): return res class CheckIdentity(models.TransientModel): + """ Wizard used to re-check the user's credentials (password) + + Might be useful before the more security-sensitive operations, users might be + leaving their computer unlocked & unattended. Re-checking credentials mitigates + some of the risk of a third party using such an unattended device to manipulate + the account. + """ _name = 'res.users.identitycheck' - _description = """ Wizard used to re-check the user's credentials (password) + _description = "Password Check Wizard" -Might be useful before the more security-sensitive operations, users might be -leaving their computer unlocked & unattended. Re-checking credentials mitigates -some of the risk of a third party using such an unattended device to manipulate -the account. -""" - - request = fields.Char(readonly=True, groups='.') - password = fields.Char(required=True) + request = fields.Char(readonly=True, groups='.') # no access + password = fields.Char() def run_check(self): - if not request: - raise UserError(_("This method can only be accessed over HTTP")) + assert request, "This method can only be accessed over HTTP" self.create_uid._check_credentials(self.password, {'interactive': True}) self.password = False @@ -1512,3 +1519,150 @@ class ChangePasswordUser(models.TransientModel): line.user_id.write({'password': line.new_passwd}) # don't keep temporary passwords in the database longer than necessary self.write({'new_passwd': False}) + +# API keys support +API_KEY_SIZE = 20 # in bytes +INDEX_SIZE = 8 # in hex digits, so 4 bytes, or 20% of the key +KEY_CRYPT_CONTEXT = passlib.context.CryptContext( + # default is 29000 rounds which is 25~50ms, which is probably unnecessary + # given in this case all the keys are completely random data: dictionary + # attacks on API keys isn't much of a concern + ['pbkdf2_sha512'], pbkdf2_sha512__rounds=6000, +) +hash_api_key = getattr(KEY_CRYPT_CONTEXT, 'hash', None) or KEY_CRYPT_CONTEXT.encrypt +class APIKeysUser(models.Model): + _inherit = 'res.users' + + api_key_ids = fields.One2many('res.users.apikeys', 'user_id', string="API Keys") + + def __init__(self, pool, cr): + init_res = super().__init__(pool, cr) + # duplicate list to avoid modifying the original reference + type(self).SELF_WRITEABLE_FIELDS = self.SELF_WRITEABLE_FIELDS + ['api_key_ids'] + type(self).SELF_READABLE_FIELDS = self.SELF_READABLE_FIELDS + ['api_key_ids'] + return init_res + + def _rpc_api_keys_only(self): + """ To be overridden if RPC access needs to be restricted to API keys, e.g. for 2FA """ + return False + + def _check_credentials(self, password, user_agent_env): + if user_agent_env['interactive']: + return super()._check_credentials(password, user_agent_env) + + if not self.env.user._rpc_api_keys_only(): + try: + return super()._check_credentials(password, user_agent_env) + except AccessDenied: + pass + + # 'rpc' scope does not really exist, we basically require a global key (scope NULL) + if self.env['res.users.apikeys']._check_credentials(scope='rpc', key=password) == self.env.uid: + return + + raise AccessDenied() + + @check_identity + def api_key_wizard(self): + return { + 'type': 'ir.actions.act_window', + 'res_model': 'res.users.apikeys.description', + 'name': 'New API Key', + 'target': 'new', + 'views': [(False, 'form')], + } + +class APIKeys(models.Model): + _name = _description = 'res.users.apikeys' + _auto = False # so we can have a secret column + + name = fields.Char("Description", required=True, readonly=True) + user_id = fields.Many2one('res.users', index=True, required=True, readonly=True, ondelete="cascade") + scope = fields.Char("Scope", readonly=True) + create_date = fields.Datetime("Creation Date", readonly=True) + + def init(self): + # pylint: disable=sql-injection + self.env.cr.execute(""" + CREATE TABLE IF NOT EXISTS {table} ( + id serial primary key, + name varchar not null, + user_id integer not null REFERENCES res_users(id), + scope varchar, + index varchar({index_size}) not null CHECK (char_length(index) = {index_size}), + key varchar not null, + create_date timestamp without time zone DEFAULT (now() at time zone 'utc') + ); + CREATE INDEX ON {table} (user_id, index); + """.format(table=self._table, index_size=INDEX_SIZE)) + + @check_identity + def remove(self): + if self.env.is_system() or self.mapped('user_id') == self.env.user: + ip = request.httprequest.environ['REMOTE_ADDR'] if request else 'n/a' + _logger.info("API key(s) removed: scope: <%s> for '%s' (#%s) from %s", + self.mapped('scope'), self.env.user.login, self.env.uid, ip) + self.sudo().unlink() + return {'type': 'ir.actions.act_window_close'} + raise AccessError(_("You can not remove API keys unless they're yours or you are a system user")) + + def _check_credentials(self, *, scope, key): + assert scope, "scope is required" + index = key[:INDEX_SIZE] + self.env.cr.execute('SELECT user_id, key FROM res_users_apikeys WHERE index = %s AND (scope IS NULL OR scope = %s)', [index, scope]) + for user_id, current_key in self.env.cr.fetchall(): + if KEY_CRYPT_CONTEXT.verify(key, current_key): + return user_id + + def _generate(self, scope, name): + """Generates an api key. + :param str scope: the scope of the key. If None, the key will give access to any rpc. + :param str name: the name of the key, mainly intended to be displayed in the UI. + :return: str: the key. + + """ + # no need to clear the LRU when *adding* a key, only when removing + k = binascii.hexlify(os.urandom(API_KEY_SIZE)).decode() + self.env.cr.execute(""" + INSERT INTO res_users_apikeys (name, user_id, scope, key, index) + VALUES (%s, %s, %s, %s, %s) + RETURNING id + """, + [name, self.env.user.id, scope, hash_api_key(k), k[:INDEX_SIZE]]) + + ip = request.httprequest.environ['REMOTE_ADDR'] if request else 'n/a' + _logger.info("API key generated: scope: <%s> for '%s' (#%s) from %s", + scope, self.env.user.login, self.env.uid, ip) + + return k + +class APIKeyDescription(models.TransientModel): + _name = _description = 'res.users.apikeys.description' + + name = fields.Char("Description", required=True) + + @check_identity + def make_key(self): + # only create keys for users who can delete their keys + if not self.user_has_groups('base.group_user'): + raise AccessError(_("Only internal users can create API keys")) + + description = self.sudo() + k = self.env['res.users.apikeys']._generate(None, self.sudo().name) + description.unlink() + + return { + 'type': 'ir.actions.act_window', + 'res_model': 'res.users.apikeys.show', + 'name': 'API Key Ready', + 'views': [(False, 'form')], + 'target': 'new', + 'context': { + 'default_key': k, + } + } + +class APIKeyShow(models.AbstractModel): + _name = _description = 'res.users.apikeys.show' + + key = fields.Char(readonly=True) diff --git a/odoo/addons/base/security/base_security.xml b/odoo/addons/base/security/base_security.xml index e09ee4c6f94..37f773d726d 100644 --- a/odoo/addons/base/security/base_security.xml +++ b/odoo/addons/base/security/base_security.xml @@ -184,5 +184,28 @@ ['|', ('share', '=', False), ('company_ids', 'in', company_ids)] + + + + Public users can't interact with keys at all + + [(0, '=', 1)] + + + + Users can read and delete their own keys + + [('user_id', '=', user.id)] + + + + Administrators can view user keys to revoke them + + [(1, '=', 1)] + + diff --git a/odoo/addons/base/security/ir.model.access.csv b/odoo/addons/base/security/ir.model.access.csv index 9c10ed68115..ae568207ce3 100644 --- a/odoo/addons/base/security/ir.model.access.csv +++ b/odoo/addons/base/security/ir.model.access.csv @@ -71,6 +71,11 @@ "access_res_users_log_all","res_users_log_all","model_res_users_log",,1,0,1,0 "access_res_users_identitycheck_employee","id check employees","model_res_users_identitycheck","group_user",1,1,1,0 "access_res_users_identitycheck_portal","id check portal","model_res_users_identitycheck","group_portal",1,1,1,0 +"access_res_users_apikeys_access_employee","API keys access employees","model_res_users_apikeys","group_user",1,0,0,0 +"access_res_users_apikeys_access_portal","API keys access portal","model_res_users_apikeys","group_portal",1,0,0,0 +"access_res_users_apikeys_description_employee","API key wizard employees","model_res_users_apikeys_description","group_user",1,0,1,0 +"access_res_users_apikeys_description_portal","API key wizard","model_res_users_apikeys_description","group_portal",1,0,1,0 +"access_res_users_apikeys_show_employee","API key result employees","model_res_users_apikeys_show","group_user",1,0,1,0 "access_ir_actions_all","ir_actions_all","model_ir_actions_actions",,1,0,0,0 "access_ir_actions_group_system","ir_actions_group_system","model_ir_actions_actions","group_system",1,1,1,1 "access_ir_actions_act_window_all","ir_actions_act_window_all","model_ir_actions_act_window",,1,0,0,0 diff --git a/odoo/addons/base/tests/test_xmlrpc.py b/odoo/addons/base/tests/test_xmlrpc.py index aa4d9028d10..755a0666066 100644 --- a/odoo/addons/base/tests/test_xmlrpc.py +++ b/odoo/addons/base/tests/test_xmlrpc.py @@ -1,8 +1,14 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import time + +from odoo.exceptions import AccessDenied, AccessError +from odoo.http import _request_stack import odoo.tools from odoo.tests import common +from odoo.service import common as auth, model +from odoo.tools import DotDict @common.tagged('post_install', '-at_install') @@ -64,3 +70,109 @@ class TestXMLRPC(common.HttpCase): 'args': args } }) + +# really just for the test cursor +@common.tagged('post_install', '-at_install') +class TestAPIKeys(common.HttpCase): + def setUp(self): + super().setUp() + self._user = self.env['res.users'].create({ + 'name': "Bylan", + 'login': 'byl', + 'password': 'ananananan', + 'tz': 'Australia/Eucla', + }) + + # needs a fake request in order to call methods protected with check_identity + fake_req = DotDict({ + # various things go and access request items + 'httprequest': DotDict({ + 'environ': {'REMOTE_ADDR': 'localhost'}, + 'cookies': {}, + }), + # bypass check_identity flow + 'session': {'identity-check-last': time.time()} + }) + _request_stack.push(fake_req) + self.addCleanup(_request_stack.pop) + + def test_trivial(self): + uid = auth.dispatch('authenticate', [self.env.cr.dbname, 'byl', 'ananananan', {}]) + self.assertEqual(uid, self._user.id) + + ctx = model.dispatch('execute_kw', [ + self.env.cr.dbname, uid, 'ananananan', + 'res.users', 'context_get', [] + ]) + self.assertEqual(ctx['tz'], 'Australia/Eucla') + + def test_wrongpw(self): + # User.authenticate raises but RPC.authenticate returns False + uid = auth.dispatch('authenticate', [self.env.cr.dbname, 'byl', 'aws', {}]) + self.assertFalse(uid) + with self.assertRaises(AccessDenied): + model.dispatch('execute_kw', [ + self.env.cr.dbname, self._user.id, 'aws', + 'res.users', 'context_get', [] + ]) + + def test_key(self): + env = self.env(user=self._user) + r = env['res.users.apikeys.description'].create({ + 'name': 'a', + }).make_key() + k = r['context']['default_key'] + + uid = auth.dispatch('authenticate', [self.env.cr.dbname, 'byl', 'ananananan', {}]) + self.assertEqual(uid, self._user.id) + + uid = auth.dispatch('authenticate', [self.env.cr.dbname, 'byl', k, {}]) + self.assertEqual(uid, self._user.id) + + ctx = model.dispatch('execute_kw', [ + self.env.cr.dbname, uid, k, + 'res.users', 'context_get', [] + ]) + self.assertEqual(ctx['tz'], 'Australia/Eucla') + + def test_delete(self): + env = self.env(user=self._user) + env['res.users.apikeys.description'].create({'name': 'b',}).make_key() + env['res.users.apikeys.description'].create({'name': 'b',}).make_key() + env['res.users.apikeys.description'].create({'name': 'b',}).make_key() + k0, k1, k2 = env['res.users.apikeys'].search([]) + + # user can remove their own keys + k0.remove() + self.assertFalse(k0.exists()) + + # admin can remove user keys + k1.with_user(self.env.ref('base.user_admin')).remove () + self.assertFalse(k1.exists()) + + # other user can't remove user keys + u = self.env['res.users'].create({ + 'name': 'a', + 'login': 'a', + 'groups_id': self.env.ref('base.group_user').ids, + }) + with self.assertRaises(AccessError): + k2.with_user(u).remove() + + def test_disabled(self): + env = self.env(user=self._user) + k = env['res.users.apikeys.description'].create({'name': 'b',}).make_key()['context']['default_key'] + + self._user.active = False + + with self.assertRaises(AccessDenied): + model.dispatch('execute_kw', [ + self.env.cr.dbname, self._user.id, 'ananananan', + 'res.users', 'context_get', [] + ]) + + with self.assertRaises(AccessDenied): + model.dispatch('execute_kw', [ + self.env.cr.dbname, self._user.id, k, + 'res.users', 'context_get', [] + ]) diff --git a/odoo/addons/base/views/res_users_views.xml b/odoo/addons/base/views/res_users_views.xml index 9b977a9832e..b560aa8ade9 100644 --- a/odoo/addons/base/views/res_users_views.xml +++ b/odoo/addons/base/views/res_users_views.xml @@ -38,7 +38,7 @@ res.users.identitycheck -
+
@@ -54,7 +54,9 @@ This is necessary for security-related changes. The authorization will last for a few minutes.

-
@@ -370,6 +372,65 @@ + + + API Key: description input wizard + res.users.apikeys.description + new + form + + + API Key: description input form + res.users.apikeys.description + + + +

+ Name your key +

+

Enter a description of and purpose for the key.

+ +

+ It is very important that this description be clear + and complete, it will be the only way to + identify the key once created. +

+
+
+ + + + + + + API Key: show + res.users.apikeys.show + +
+ +

+ Write down your key +

+

+ Here is your new API key, use it instead of a password for RPC access. + Your login is still necessary for interactive usage. +

+

+ +
+
+ +
+ + + res.users.preferences.form res.users @@ -409,6 +470,24 @@ groups="base.group_multi_company"/> + + + + + + + + + + +