diff --git a/auth_totp/README.rst b/auth_totp/README.rst index 1fcce6ea2..abc036507 100644 --- a/auth_totp/README.rst +++ b/auth_totp/README.rst @@ -45,6 +45,18 @@ re-enabling MFA for that user. Known Issues / Roadmap ====================== +Known Issues +------------ + +* External calls to the Odoo XML-RPC API are blocked for users who enable MFA + since there is currently no way to perform MFA authentication as part of this + process. However, due to the way that Odoo handles authentication caching, + multi-threaded or multi-process servers will need to be restarted before the + block can take effect for users who have just enabled MFA. + +Roadmap +------- + * Make the lifetime of the trusted device cookie configurable rather than fixed at 30 days * Add device fingerprinting to the trusted device cookie @@ -54,10 +66,10 @@ Known Issues / Roadmap Bug Tracker =========== -Bugs are tracked on `GitHub Issues -`_. In case of trouble, please -check there if your issue has already been reported. If you spotted it first, -help us smash it by providing detailed and welcomed feedback. +Bugs are tracked on +`GitHub Issues `_. In case of +trouble, please check there if your issue has already been reported. If you +spotted it first, help us smash it by providing detailed and welcomed feedback. Credits ======= @@ -65,7 +77,8 @@ Credits Images ------ -* Odoo Community Association: `Icon `_. +* Odoo Community Association: + `Icon `_. Contributors ------------ diff --git a/auth_totp/models/res_users.py b/auth_totp/models/res_users.py index c04a539f5..5406cc6a6 100644 --- a/auth_totp/models/res_users.py +++ b/auth_totp/models/res_users.py @@ -2,6 +2,7 @@ # Copyright 2016-2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). +from collections import defaultdict from uuid import uuid4 from odoo import _, api, fields, models from odoo.exceptions import ValidationError @@ -12,6 +13,7 @@ from ..exceptions import MfaLoginNeeded class ResUsers(models.Model): _inherit = 'res.users' + _mfa_uid_cache = defaultdict(set) @classmethod def _build_model(cls, pool, cr): @@ -54,6 +56,15 @@ class ResUsers(models.Model): ' please add one before you activate this feature.' )) + @classmethod + def check(cls, db, uid, password): + """Prevent auth caching for MFA users without active MFA session""" + if uid in cls._mfa_uid_cache[db]: + if not request or request.session.get('mfa_login_active') != uid: + cls._Users__uid_cache[db].pop(uid, None) + + return super(ResUsers, cls).check(db, uid, password) + @api.model def check_credentials(self, password): """Add MFA logic to core authentication process. @@ -69,6 +80,8 @@ class ResUsers(models.Model): if not self.env.user.mfa_enabled: return super(ResUsers, self).check_credentials(password) + self._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid) + if request: if request.session.get('mfa_login_active') == self.env.uid: return super(ResUsers, self).check_credentials(password) diff --git a/auth_totp/tests/test_res_users.py b/auth_totp/tests/test_res_users.py index 08264aa2f..caf145d11 100644 --- a/auth_totp/tests/test_res_users.py +++ b/auth_totp/tests/test_res_users.py @@ -18,6 +18,8 @@ class TestResUsers(TransactionCase): def setUp(self): super(TestResUsers, self).setUp() + self.test_model = self.env['res.users'] + self.test_user = self.env.ref('base.user_root') self.test_user.mfa_enabled = False self.test_user.authenticator_ids = False @@ -62,6 +64,67 @@ class TestResUsers(TransactionCase): except ValidationError: self.fail('A ValidationError was raised and should not have been.') + @patch(REQUEST_PATH, new=None) + def test_check_mfa_without_request(self): + """It should remove UID from cache if in MFA cache and no request""" + test_cache = self.test_model._Users__uid_cache[self.env.cr.dbname] + test_cache[self.env.uid] = 'test' + self.test_model._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid) + try: + self.test_model.check(self.env.cr.dbname, self.env.uid, 'test') + except AccessDenied: + pass + + self.assertFalse(test_cache.get(self.env.uid)) + + @patch(REQUEST_PATH) + def test_check_mfa_no_mfa_session(self, request_mock): + """It should remove UID from cache if MFA cache but no MFA session""" + request_mock.session = {} + test_cache = self.test_model._Users__uid_cache[self.env.cr.dbname] + test_cache[self.env.uid] = 'test' + self.test_model._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid) + try: + self.test_model.check(self.env.cr.dbname, self.env.uid, 'test') + except AccessDenied: + pass + + self.assertFalse(test_cache.get(self.env.uid)) + + @patch(REQUEST_PATH) + def test_check_mfa_invalid_mfa_session(self, request_mock): + """It should remove UID if in MFA cache but invalid MFA session""" + request_mock.session = {'mfa_login_active': self.env.uid + 1} + test_cache = self.test_model._Users__uid_cache[self.env.cr.dbname] + test_cache[self.env.uid] = 'test' + self.test_model._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid) + try: + self.test_model.check(self.env.cr.dbname, self.env.uid, 'test') + except AccessDenied: + pass + + self.assertFalse(test_cache.get(self.env.uid)) + + def test_check_no_mfa(self): + """It should not remove UID from cache if not in MFA cache""" + test_cache = self.test_model._Users__uid_cache[self.env.cr.dbname] + test_cache[self.env.uid] = 'test' + self.test_model._mfa_uid_cache[self.env.cr.dbname].clear() + self.test_model.check(self.env.cr.dbname, self.env.uid, 'test') + + self.assertEqual(test_cache.get(self.env.uid), 'test') + + @patch(REQUEST_PATH) + def test_check_mfa_valid_session(self, request_mock): + """It should not remove UID if in MFA cache and valid session""" + request_mock.session = {'mfa_login_active': self.env.uid} + test_cache = self.test_model._Users__uid_cache[self.env.cr.dbname] + test_cache[self.env.uid] = 'test' + self.test_model._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid) + self.test_model.check(self.env.cr.dbname, self.env.uid, 'test') + + self.assertEqual(test_cache.get(self.env.uid), 'test') + def test_check_credentials_mfa_not_enabled(self): '''Should check password if user does not have MFA enabled''' self.test_user.mfa_enabled = False @@ -73,6 +136,17 @@ class TestResUsers(TransactionCase): except AccessDenied: self.fail('An exception was raised with a correct password.') + def test_check_credentials_mfa_uid_cache(self): + """It should add user's ID to MFA UID cache if MFA enabled""" + self.test_model._mfa_uid_cache[self.env.cr.dbname].clear() + try: + self.test_model.check_credentials('invalid') + except AccessDenied: + pass + + result_cache = self.test_model._mfa_uid_cache[self.env.cr.dbname] + self.assertEqual(result_cache, {self.test_user.id}) + @patch(REQUEST_PATH, new=None) def test_check_credentials_mfa_and_no_request(self): '''Should raise correct exception if MFA enabled and no request'''