diff --git a/auth_totp/README.rst b/auth_totp/README.rst index 6e0200525..abc036507 100644 --- a/auth_totp/README.rst +++ b/auth_totp/README.rst @@ -26,15 +26,17 @@ Configuration ============= By default, the trusted device cookies introduced by this module have a -``Secure`` flag and can only be sent via HTTPS. You can disable this by going -to ``Settings > Parameters > System Parameters`` and changing the -``auth_totp.secure_cookie`` key to ``0``, but this is not recommended in -production as it increases the likelihood of cookie theft via eavesdropping. +``Secure`` flag. This decreases the likelihood of cookie theft via +eavesdropping but may result in cookies not being set by certain browsers +unless your Odoo instance uses HTTPS. If necessary, you can disable this flag +by going to ``Settings > Parameters > System Parameters`` and changing the +``auth_totp.secure_cookie`` key to ``0``. Usage ===== -Install and enjoy. +If necessary, a user's trusted devices can be revoked by disabling and +re-enabling MFA for that user. .. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas :alt: Try me on Runbot @@ -46,35 +48,28 @@ Known Issues / Roadmap Known Issues ------------ -* The module does not uninstall cleanly due to an Odoo bug, leaving the - ``res.users.authenticator`` and ``res.users.device`` models partially in - place. This may be addressed at a later time via an Odoo fix or by adding - custom uninstall logic via an uninstall hook. +* 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 various durations associated with the module configurable. They are - currently hard-coded as follows: - - * 15 minutes to enter an MFA confirmation code after a password log in - * 30 days before the MFA session expires and the user has to log in again - * 30 days before the trusted device cookie expires - -* Add logic to extend an MFA user's session each time it's validated, - effectively keeping it alive indefinitely as long as the user remains active -* Add device fingerprinting to the trusted device cookie and provide a way to - revoke trusted devices +* Make the lifetime of the trusted device cookie configurable rather than fixed + at 30 days +* Add device fingerprinting to the trusted device cookie * Add company-level settings for forcing all users to enable MFA and disabling the trusted device option 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 ======= @@ -82,7 +77,8 @@ Credits Images ------ -* Odoo Community Association: `Icon `_. +* Odoo Community Association: + `Icon `_. Contributors ------------ diff --git a/auth_totp/__manifest__.py b/auth_totp/__manifest__.py index 8764cc404..9d7b3f4c6 100644 --- a/auth_totp/__manifest__.py +++ b/auth_totp/__manifest__.py @@ -5,9 +5,9 @@ { 'name': 'MFA Support', 'summary': 'Allows users to enable MFA and add optional trusted devices', - 'version': '10.0.1.0.1', + 'version': '10.0.2.0.0', 'category': 'Extra Tools', - 'website': 'https://laslabs.com/', + 'website': 'https://github.com/OCA/server-tools', 'author': 'LasLabs, Odoo Community Association (OCA)', 'license': 'LGPL-3', 'application': False, diff --git a/auth_totp/controllers/main.py b/auth_totp/controllers/main.py index 029474528..384437f9a 100644 --- a/auth_totp/controllers/main.py +++ b/auth_totp/controllers/main.py @@ -6,11 +6,9 @@ from datetime import datetime, timedelta import json from werkzeug.contrib.securecookie import SecureCookie from werkzeug.wrappers import Response as WerkzeugResponse -from odoo import _, http, registry, SUPERUSER_ID -from odoo.api import Environment +from odoo import _, http from odoo.http import Response, request from odoo.addons.web.controllers.main import Home -from ..exceptions import MfaTokenInvalidError, MfaTokenExpiredError class JsonSecureCookie(SecureCookie): @@ -18,53 +16,19 @@ class JsonSecureCookie(SecureCookie): class AuthTotp(Home): - @http.route() def web_login(self, *args, **kwargs): - """Add MFA logic to the web_login action in Home - - Overview: - * Call web_login in Home - * Return the result of that call if the user has not logged in yet - using a password, does not have MFA enabled, or has a valid - trusted device cookie - * If none of these is true, generate a new MFA login token for the - user, log the user out, and redirect to the MFA login form - """ - - # sudo() is required because there may be no request.env.uid (likely - # since there may be no user logged in at the start of the request) - user_model_sudo = request.env['res.users'].sudo() - config_model_sudo = user_model_sudo.env['ir.config_parameter'] - response = super(AuthTotp, self).web_login(*args, **kwargs) - if not request.params.get('login_success'): - return response - - user = user_model_sudo.browse(request.uid) - if not user.mfa_enabled: - return response - - cookie_key = 'trusted_devices_%d' % user.id - device_cookie = request.httprequest.cookies.get(cookie_key) - if device_cookie: - secret = config_model_sudo.get_param('database.secret') - device_cookie = JsonSecureCookie.unserialize(device_cookie, secret) - if device_cookie.get('device_id') in user.trusted_device_ids.ids: - return response - - user.generate_mfa_login_token() - request.session.logout(keep_db=True) - request.params['login_success'] = False - return http.local_redirect( - '/auth_totp/login', - query={ - 'mfa_login_token': user.mfa_login_token, - 'redirect': request.params.get('redirect'), - }, - keep_hash=True, - ) + if request.session.get('mfa_login_needed'): + request.session['mfa_login_needed'] = False + return http.local_redirect( + '/auth_totp/login', + query={'redirect': request.params.get('redirect')}, + keep_hash=True, + ) + + return response @http.route( '/auth_totp/login', @@ -78,35 +42,36 @@ class AuthTotp(Home): @http.route('/auth_totp/login', type='http', auth='none', methods=['POST']) def mfa_login_post(self, *args, **kwargs): - """Process MFA login attempt + """Process MFA login attempt. Overview: - * Try to find a user based on the MFA login token. If this doesn't - work, redirect to the password login page with an error message + * Identify current user based on login in session. If this doesn't + work, redirect to the password login page with an error message. * Validate the confirmation code provided by the user. If it's not - valid, redirect to the previous login step with an error message - * Generate a long-term MFA login token for the user and log the - user in using the token + valid, redirect to the previous login step with an error message. + * Update the session to indicate that the MFA login process for + this user is complete and attempt password authentication again. * Build a trusted device cookie and add it to the response if the - trusted device option was checked - * Redirect to the provided URL or to '/web' if one was not given + trusted device option was checked. + * Redirect to the provided URL or to '/web' if one was not given. """ # sudo() is required because there is no request.env.uid (likely since # there is no user logged in at the start of the request) user_model_sudo = request.env['res.users'].sudo() - device_model_sudo = user_model_sudo.env['res.users.device'] config_model_sudo = user_model_sudo.env['ir.config_parameter'] - token = request.params.get('mfa_login_token') - try: - user = user_model_sudo.user_from_mfa_login_token(token) - except (MfaTokenInvalidError, MfaTokenExpiredError) as exception: + user_login = request.session.get('login') + user = user_model_sudo.search([('login', '=', user_login)]) + if not user: return http.local_redirect( '/web/login', query={ 'redirect': request.params.get('redirect'), - 'error': exception.message, + 'error': _( + 'You must log in with a password before starting the' + ' MFA login process.' + ), }, keep_hash=True, ) @@ -121,21 +86,15 @@ class AuthTotp(Home): 'Your confirmation code is not correct. Please try' ' again.' ), - 'mfa_login_token': token, }, keep_hash=True, ) + request.session['mfa_login_active'] = user.id - # These context managers trigger a safe commit, which persists the - # changes right away and is needed for the auth call - with Environment.manage(): - with registry(request.db).cursor() as temp_cr: - temp_env = Environment(temp_cr, SUPERUSER_ID, request.context) - temp_user = temp_env['res.users'].browse(user.id) - temp_user.generate_mfa_login_token(60 * 24 * 30) - token = temp_user.mfa_login_token - request.session.authenticate(request.db, user.login, token, user.id) - request.params['login_success'] = True + user_pass = request.session.get('password') + uid = request.session.authenticate(request.db, user.login, user_pass) + if uid: + request.params['login_success'] = True redirect = request.params.get('redirect') if not redirect: @@ -145,9 +104,8 @@ class AuthTotp(Home): response = Response(response) if request.params.get('remember_device'): - device = device_model_sudo.create({'user_id': user.id}) - secret = config_model_sudo.get_param('database.secret') - device_cookie = JsonSecureCookie({'device_id': device.id}, secret) + secret = user.trusted_device_cookie_key + device_cookie = JsonSecureCookie({'user_id': user.id}, secret) cookie_lifetime = timedelta(days=30) cookie_exp = datetime.utcnow() + cookie_lifetime device_cookie = device_cookie.serialize(cookie_exp) diff --git a/auth_totp/exceptions.py b/auth_totp/exceptions.py index 40cd36dd6..c2b227e58 100644 --- a/auth_totp/exceptions.py +++ b/auth_totp/exceptions.py @@ -5,16 +5,5 @@ from odoo.exceptions import AccessDenied -class MfaTokenError(AccessDenied): - - def __init__(self, message): - super(MfaTokenError, self).__init__() - self.message = message - - -class MfaTokenInvalidError(MfaTokenError): - pass - - -class MfaTokenExpiredError(MfaTokenError): +class MfaLoginNeeded(AccessDenied): pass diff --git a/auth_totp/migrations/10.0.2.0.0/post-migrate.py b/auth_totp/migrations/10.0.2.0.0/post-migrate.py new file mode 100644 index 000000000..7838c6a40 --- /dev/null +++ b/auth_totp/migrations/10.0.2.0.0/post-migrate.py @@ -0,0 +1,36 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). + +from uuid import uuid4 +from odoo import api, SUPERUSER_ID + + +def migrate(cr, version): + """Generate cookie keys for all users with MFA enabled and clean up.""" + env = api.Environment(cr, SUPERUSER_ID, {}) + user_model = env['res.users'].with_context(active_test=False) + mfa_users = user_model.search([('mfa_enabled', '=', True)]) + + for mfa_user in mfa_users: + mfa_user.trusted_device_cookie_key = uuid4() + + # Clean up ir records for device model to prevent warnings + removed_model = 'res.users.device' + removed_model_record = env['ir.model'].search([ + ('model', '=', removed_model), + ]) + removed_model_fields = removed_model_record.field_id + + removed_model_fields._prepare_update() + env['ir.model.constraint'].search([ + ('model', '=', removed_model_record.id), + ]).unlink() + env['ir.model.data'].search([ + ('model', '=', 'ir.model'), + ('res_id', '=', removed_model_record.id), + ]).unlink() + cr.execute( + 'DELETE FROM ir_model WHERE model = %s', + [removed_model], + ) diff --git a/auth_totp/models/__init__.py b/auth_totp/models/__init__.py index af6af42d9..926221bf2 100644 --- a/auth_totp/models/__init__.py +++ b/auth_totp/models/__init__.py @@ -4,4 +4,3 @@ from . import res_users from . import res_users_authenticator -from . import res_users_device diff --git a/auth_totp/models/res_users.py b/auth_totp/models/res_users.py index 72693b3cf..5406cc6a6 100644 --- a/auth_totp/models/res_users.py +++ b/auth_totp/models/res_users.py @@ -2,16 +2,18 @@ # Copyright 2016-2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). -from datetime import datetime, timedelta -import random -import string +from collections import defaultdict +from uuid import uuid4 from odoo import _, api, fields, models -from odoo.exceptions import AccessDenied, ValidationError -from ..exceptions import MfaTokenInvalidError, MfaTokenExpiredError +from odoo.exceptions import ValidationError +from odoo.http import request +from ..controllers.main import JsonSecureCookie +from ..exceptions import MfaLoginNeeded class ResUsers(models.Model): _inherit = 'res.users' + _mfa_uid_cache = defaultdict(set) @classmethod def _build_model(cls, pool, cr): @@ -29,14 +31,20 @@ class ResUsers(models.Model): ' right. If the button is not present, you do not have the' ' permissions to do this.', ) - mfa_login_token = fields.Char() - mfa_login_token_exp = fields.Datetime() - trusted_device_ids = fields.One2many( - comodel_name='res.users.device', - inverse_name='user_id', - string='Trusted Devices', + trusted_device_cookie_key = fields.Char( + compute='_compute_trusted_device_cookie_key', + store=True, ) + @api.multi + @api.depends('mfa_enabled') + def _compute_trusted_device_cookie_key(self): + for record in self: + if record.mfa_enabled: + record.trusted_device_cookie_key = uuid4() + else: + record.trusted_device_cookie_key = False + @api.multi @api.constrains('mfa_enabled', 'authenticator_ids') def _check_enabled_with_authenticator(self): @@ -48,55 +56,48 @@ 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): - try: + """Add MFA logic to core authentication process. + + Overview: + * If user does not have MFA enabled, defer to parent logic. + * If user has MFA enabled and has gone through MFA login process + this session or has correct device cookie, defer to parent logic. + * If neither of these is true, call parent logic. If successful, + prevent auth while updating session to indicate that MFA login + process can now commence. + """ + if not self.env.user.mfa_enabled: return super(ResUsers, self).check_credentials(password) - except AccessDenied: - user = self.sudo().search([ - ('id', '=', self.env.uid), - ('mfa_login_token', '=', password), - ]) - user._user_from_mfa_login_token_validate() - @api.multi - def generate_mfa_login_token(self, lifetime_mins=15): - char_set = string.ascii_letters + string.digits - - for record in self: - record.mfa_login_token = ''.join( - random.SystemRandom().choice(char_set) for __ in range(20) - ) + self._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid) - expiration = datetime.now() + timedelta(minutes=lifetime_mins) - record.mfa_login_token_exp = fields.Datetime.to_string(expiration) + if request: + if request.session.get('mfa_login_active') == self.env.uid: + return super(ResUsers, self).check_credentials(password) - @api.model - def user_from_mfa_login_token(self, token): - if not token: - raise MfaTokenInvalidError(_( - 'Your MFA login token is not valid. Please try again.' - )) + cookie_key = 'trusted_devices_%d' % self.env.uid + device_cook = request.httprequest.cookies.get(cookie_key) + if device_cook: + secret = self.env.user.trusted_device_cookie_key + device_cook = JsonSecureCookie.unserialize(device_cook, secret) + if device_cook: + return super(ResUsers, self).check_credentials(password) - user = self.search([('mfa_login_token', '=', token)]) - user._user_from_mfa_login_token_validate() - - return user - - @api.multi - def _user_from_mfa_login_token_validate(self): - try: - self.ensure_one() - except ValueError: - raise MfaTokenInvalidError(_( - 'Your MFA login token is not valid. Please try again.' - )) - - token_exp = fields.Datetime.from_string(self.mfa_login_token_exp) - if token_exp < datetime.now(): - raise MfaTokenExpiredError(_( - 'Your MFA login token has expired. Please try again.' - )) + super(ResUsers, self).check_credentials(password) + if request: + request.session['mfa_login_needed'] = True + raise MfaLoginNeeded @api.multi def validate_mfa_confirmation_code(self, confirmation_code): diff --git a/auth_totp/models/res_users_device.py b/auth_totp/models/res_users_device.py deleted file mode 100644 index f2f4d0f17..000000000 --- a/auth_totp/models/res_users_device.py +++ /dev/null @@ -1,16 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2016-2017 LasLabs Inc. -# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). - -from odoo import fields, models - - -class ResUsersDevice(models.Model): - _name = 'res.users.device' - _description = 'Trusted Device for MFA Auth' - - user_id = fields.Many2one( - comodel_name='res.users', - ondelete='cascade', - required=True, - ) diff --git a/auth_totp/security/ir.model.access.csv b/auth_totp/security/ir.model.access.csv index 54c0835ab..a7b1d74bc 100644 --- a/auth_totp/security/ir.model.access.csv +++ b/auth_totp/security/ir.model.access.csv @@ -1,3 +1,2 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink authenticator_access,MFA Authenticator - User Access,model_res_users_authenticator,base.group_user,1,1,1,1 -device_access,MFA Device - Manager Access,model_res_users_device,,0,0,0,0 diff --git a/auth_totp/tests/__init__.py b/auth_totp/tests/__init__.py index 572c43147..bf7a63db8 100644 --- a/auth_totp/tests/__init__.py +++ b/auth_totp/tests/__init__.py @@ -3,6 +3,7 @@ # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). from . import test_main +from . import test_post_migrate from . import test_res_users from . import test_res_users_authenticator from . import test_res_users_authenticator_create diff --git a/auth_totp/tests/test_main.py b/auth_totp/tests/test_main.py index a3810fd3f..62188ac41 100644 --- a/auth_totp/tests/test_main.py +++ b/auth_totp/tests/test_main.py @@ -3,7 +3,7 @@ # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). from datetime import datetime -import mock +from mock import MagicMock, patch from odoo.http import Response from odoo.tests.common import TransactionCase from ..controllers.main import AuthTotp @@ -12,18 +12,19 @@ CONTROLLER_PATH = 'odoo.addons.auth_totp.controllers.main' REQUEST_PATH = CONTROLLER_PATH + '.request' SUPER_PATH = CONTROLLER_PATH + '.Home.web_login' JSON_PATH = CONTROLLER_PATH + '.JsonSecureCookie' -ENVIRONMENT_PATH = CONTROLLER_PATH + '.Environment' RESPONSE_PATH = CONTROLLER_PATH + '.Response' DATETIME_PATH = CONTROLLER_PATH + '.datetime' REDIRECT_PATH = CONTROLLER_PATH + '.http.redirect_with_hash' TRANSLATE_PATH_CONT = CONTROLLER_PATH + '._' MODEL_PATH = 'odoo.addons.auth_totp.models.res_users' -GENERATE_PATH = MODEL_PATH + '.ResUsers.generate_mfa_login_token' VALIDATE_PATH = MODEL_PATH + '.ResUsers.validate_mfa_confirmation_code' -TRANSLATE_PATH_MOD = MODEL_PATH + '._' -@mock.patch(REQUEST_PATH) +class AssignableDict(dict): + pass + + +@patch(REQUEST_PATH) class TestAuthTotp(TransactionCase): def setUp(self): @@ -32,104 +33,40 @@ class TestAuthTotp(TransactionCase): self.test_controller = AuthTotp() self.test_user = self.env.ref('base.user_root') + self.test_user.mfa_enabled = False + self.test_user.authenticator_ids = False self.env['res.users.authenticator'].create({ 'name': 'Test Authenticator', 'secret_key': 'iamatestsecretyo', 'user_id': self.test_user.id, }) self.test_user.mfa_enabled = True - self.test_user.generate_mfa_login_token() - self.test_user.trusted_device_ids = None # Needed when tests are run with no prior requests (e.g. on a new DB) - patcher = mock.patch('odoo.http.request') + patcher = patch('odoo.http.request') self.addCleanup(patcher.stop) patcher.start() - @mock.patch(SUPER_PATH) - def test_web_login_no_password_login(self, super_mock, request_mock): - '''Should return wrapped result of super if no password log in''' - test_response = 'Test Response' - super_mock.return_value = test_response - request_mock.params = {} - - self.assertEqual(self.test_controller.web_login().data, test_response) - - @mock.patch(SUPER_PATH) - def test_web_login_user_no_mfa(self, super_mock, request_mock): - '''Should return wrapped result of super if user did not enable MFA''' - test_response = 'Test Response' - super_mock.return_value = test_response - request_mock.params = {'login_success': True} - request_mock.env = self.env - request_mock.uid = self.test_user.id - self.test_user.mfa_enabled = False + @patch(SUPER_PATH) + def test_web_login_mfa_needed(self, super_mock, request_mock): + '''Should update session and redirect correctly if MFA login needed''' + request_mock.session = {'mfa_login_needed': True} + request_mock.params = {'redirect': 'Test Redir'} - self.assertEqual(self.test_controller.web_login().data, test_response) + test_result = self.test_controller.web_login() + super_mock.assert_called_once() + self.assertIn('/auth_totp/login?redirect=Test+Redir', test_result.data) + self.assertFalse(request_mock.session['mfa_login_needed']) - @mock.patch(JSON_PATH) - @mock.patch(SUPER_PATH) - def test_web_login_valid_cookie(self, super_mock, json_mock, request_mock): - '''Should return wrapped result of super if valid device cookie''' + @patch(SUPER_PATH) + def test_web_login_mfa_not_needed(self, super_mock, request_mock): + '''Should return result of calling super if MFA login not needed''' test_response = 'Test Response' super_mock.return_value = test_response - request_mock.params = {'login_success': True} - request_mock.env = self.env - request_mock.uid = self.test_user.id - - device_model = self.env['res.users.device'] - test_device = device_model.create({'user_id': self.test_user.id}) - json_mock.unserialize().get.return_value = test_device.id + request_mock.session = {} self.assertEqual(self.test_controller.web_login().data, test_response) - @mock.patch(SUPER_PATH) - @mock.patch(GENERATE_PATH) - def test_web_login_no_cookie(self, gen_mock, super_mock, request_mock): - '''Should respond correctly if no device cookie with expected key''' - request_mock.env = self.env - request_mock.uid = self.test_user.id - request_mock.params = { - 'login_success': True, - 'redirect': 'Test Redir', - } - self.test_user.mfa_login_token = 'Test Token' - request_mock.httprequest.cookies = {} - request_mock.reset_mock() - - test_result = self.test_controller.web_login() - gen_mock.assert_called_once_with() - request_mock.session.logout.assert_called_once_with(keep_db=True) - self.assertIn( - '/auth_totp/login?redirect=Test+Redir&mfa_login_token=Test+Token', - test_result.data, - ) - - @mock.patch(SUPER_PATH) - @mock.patch(JSON_PATH) - @mock.patch(GENERATE_PATH) - def test_web_login_bad_device_id( - self, gen_mock, json_mock, super_mock, request_mock - ): - '''Should respond correctly if invalid device_id in device cookie''' - request_mock.env = self.env - request_mock.uid = self.test_user.id - request_mock.params = { - 'login_success': True, - 'redirect': 'Test Redir', - } - self.test_user.mfa_login_token = 'Test Token' - json_mock.unserialize.return_value = {'device_id': 1} - request_mock.reset_mock() - - test_result = self.test_controller.web_login() - gen_mock.assert_called_once_with() - request_mock.session.logout.assert_called_once_with(keep_db=True) - self.assertIn( - '/auth_totp/login?redirect=Test+Redir&mfa_login_token=Test+Token', - test_result.data, - ) - def test_mfa_login_get(self, request_mock): '''Should render mfa_login template with correct context''' request_mock.render.return_value = 'Test Value' @@ -141,51 +78,40 @@ class TestAuthTotp(TransactionCase): qcontext=request_mock.params, ) - @mock.patch(TRANSLATE_PATH_MOD) - def test_mfa_login_post_invalid_token(self, tl_mock, request_mock): - '''Should return correct redirect if login token invalid''' + @patch(TRANSLATE_PATH_CONT) + def test_mfa_login_post_no_login(self, tl_mock, request_mock): + '''Should redirect correctly if login missing from session''' request_mock.env = self.env - request_mock.params = { - 'mfa_login_token': 'Invalid Token', - 'redirect': 'Test Redir', - } + request_mock.session = {} + request_mock.params = {'redirect': 'Test Redir'} tl_mock.side_effect = lambda arg: arg tl_mock.reset_mock() test_result = self.test_controller.mfa_login_post() tl_mock.assert_called_once() self.assertIn('/web/login?redirect=Test+Redir', test_result.data) - self.assertIn( - '&error=Your+MFA+login+token+is+not+valid.', - test_result.data, - ) + self.assertIn('&error=You+must+log+in', test_result.data) - @mock.patch(TRANSLATE_PATH_MOD) - def test_mfa_login_post_expired_token(self, tl_mock, request_mock): - '''Should return correct redirect if login token expired''' + @patch(TRANSLATE_PATH_CONT) + def test_mfa_login_post_invalid_login(self, tl_mock, request_mock): + '''Should redirect correctly if invalid login in session''' request_mock.env = self.env - self.test_user.generate_mfa_login_token(-1) - request_mock.params = { - 'mfa_login_token': self.test_user.mfa_login_token, - 'redirect': 'Test Redir', - } + request_mock.session = {'login': 'Invalid Login'} + request_mock.params = {'redirect': 'Test Redir'} tl_mock.side_effect = lambda arg: arg tl_mock.reset_mock() test_result = self.test_controller.mfa_login_post() tl_mock.assert_called_once() self.assertIn('/web/login?redirect=Test+Redir', test_result.data) - self.assertIn( - '&error=Your+MFA+login+token+has+expired.', - test_result.data, - ) + self.assertIn('&error=You+must+log+in', test_result.data) - @mock.patch(TRANSLATE_PATH_CONT) + @patch(TRANSLATE_PATH_CONT) def test_mfa_login_post_invalid_conf_code(self, tl_mock, request_mock): '''Should return correct redirect if confirmation code is invalid''' request_mock.env = self.env + request_mock.session = {'login': self.test_user.login} request_mock.params = { - 'mfa_login_token': self.test_user.mfa_login_token, 'redirect': 'Test Redir', 'confirmation_code': 'Invalid Code', } @@ -199,135 +125,119 @@ class TestAuthTotp(TransactionCase): '&error=Your+confirmation+code+is+not+correct.', test_result.data, ) - self.assertIn( - '&mfa_login_token=%s' % self.test_user.mfa_login_token, - test_result.data, - ) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) - def test_mfa_login_post_new_token(self, val_mock, gen_mock, request_mock): - '''Should refresh user's login token w/right lifetime if info valid''' + @patch(VALIDATE_PATH) + def test_mfa_login_post_valid_conf_code(self, val_mock, request_mock): + '''Should correctly update session if confirmation code is valid''' request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = {'mfa_login_token': test_token} + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock() + test_conf_code = 'Test Code' + request_mock.params = {'confirmation_code': test_conf_code} val_mock.return_value = True - gen_mock.reset_mock() self.test_controller.mfa_login_post() - gen_mock.assert_called_once_with(60 * 24 * 30) + val_mock.assert_called_once_with(test_conf_code) + resulting_flag = request_mock.session['mfa_login_active'] + self.assertEqual(resulting_flag, self.test_user.id) - @mock.patch(ENVIRONMENT_PATH) - @mock.patch(VALIDATE_PATH) - def test_mfa_login_post_session(self, val_mock, env_mock, request_mock): - '''Should log user in with new token as password if info valid''' + @patch(VALIDATE_PATH) + def test_mfa_login_post_pass_auth_fail(self, val_mock, request_mock): + '''Should not set success param if password auth fails''' request_mock.env = self.env - request_mock.db = self.registry.db_name - old_test_token = self.test_user.mfa_login_token - request_mock.params = {'mfa_login_token': old_test_token} + request_mock.db = test_db = 'Test DB' + test_password = 'Test Password' + request_mock.session = AssignableDict( + login=self.test_user.login, password=test_password, + ) + request_mock.session.authenticate = MagicMock(return_value=False) + request_mock.params = {} val_mock.return_value = True - env_mock.return_value = self.env - request_mock.reset_mock() self.test_controller.mfa_login_post() - new_test_token = self.test_user.mfa_login_token request_mock.session.authenticate.assert_called_once_with( - request_mock.db, - self.test_user.login, - new_test_token, - self.test_user.id, + test_db, self.test_user.login, test_password, ) + self.assertFalse(request_mock.params.get('login_success')) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) - def test_mfa_login_post_redirect(self, val_mock, gen_mock, request_mock): + @patch(VALIDATE_PATH) + def test_mfa_login_post_pass_auth_success(self, val_mock, request_mock): + '''Should set success param if password auth succeeds''' + request_mock.env = self.env + request_mock.db = test_db = 'Test DB' + test_password = 'Test Password' + request_mock.session = AssignableDict( + login=self.test_user.login, password=test_password, + ) + request_mock.session.authenticate = MagicMock(return_value=True) + request_mock.params = {} + val_mock.return_value = True + self.test_controller.mfa_login_post() + + request_mock.session.authenticate.assert_called_once_with( + test_db, self.test_user.login, test_password, + ) + self.assertTrue(request_mock.params.get('login_success')) + + @patch(VALIDATE_PATH) + def test_mfa_login_post_redirect(self, val_mock, request_mock): '''Should return correct redirect if info valid and redirect present''' request_mock.env = self.env - request_mock.db = self.registry.db_name + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) test_redir = 'Test Redir' - request_mock.params = { - 'mfa_login_token': self.test_user.mfa_login_token, - 'redirect': test_redir, - } + request_mock.params = {'redirect': test_redir} val_mock.return_value = True test_result = self.test_controller.mfa_login_post() self.assertIn("window.location = '%s'" % test_redir, test_result.data) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) - def test_mfa_login_post_redir_def(self, val_mock, gen_mock, request_mock): + @patch(VALIDATE_PATH) + def test_mfa_login_post_redir_def(self, val_mock, request_mock): '''Should return redirect to /web if info valid and no redirect''' request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = {'mfa_login_token': test_token} + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) + request_mock.params = {} val_mock.return_value = True test_result = self.test_controller.mfa_login_post() self.assertIn("window.location = '/web'", test_result.data) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) - def test_mfa_login_post_device(self, val_mock, gen_mock, request_mock): - '''Should add trusted device to user if remember flag set''' - request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = { - 'mfa_login_token': test_token, - 'remember_device': True, - } - val_mock.return_value = True - self.test_controller.mfa_login_post() - - self.assertEqual(len(self.test_user.trusted_device_ids), 1) - - @mock.patch(RESPONSE_PATH) - @mock.patch(JSON_PATH) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) + @patch(RESPONSE_PATH) + @patch(JSON_PATH) + @patch(VALIDATE_PATH) def test_mfa_login_post_cookie_werkzeug_cookie( - self, val_mock, gen_mock, json_mock, resp_mock, request_mock + self, val_mock, json_mock, resp_mock, request_mock ): '''Should create Werkzeug cookie w/right info if remember flag set''' request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = { - 'mfa_login_token': test_token, - 'remember_device': True, - } + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) + request_mock.params = {'remember_device': True} val_mock.return_value = True resp_mock().__class__ = Response json_mock.reset_mock() self.test_controller.mfa_login_post() - test_device = self.test_user.trusted_device_ids - config_model = self.env['ir.config_parameter'] - test_secret = config_model.get_param('database.secret') + test_secret = self.test_user.trusted_device_cookie_key json_mock.assert_called_once_with( - {'device_id': test_device.id}, + {'user_id': self.test_user.id}, test_secret, ) - @mock.patch(DATETIME_PATH) - @mock.patch(RESPONSE_PATH) - @mock.patch(JSON_PATH) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) + @patch(DATETIME_PATH) + @patch(RESPONSE_PATH) + @patch(JSON_PATH) + @patch(VALIDATE_PATH) def test_mfa_login_post_cookie_werkzeug_cookie_exp( - self, val_mock, gen_mock, json_mock, resp_mock, dt_mock, request_mock + self, val_mock, json_mock, resp_mock, dt_mock, request_mock ): '''Should serialize Werkzeug cookie w/right exp if remember flag set''' request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = { - 'mfa_login_token': test_token, - 'remember_device': True, - } + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) + request_mock.params = {'remember_device': True} val_mock.return_value = True dt_mock.utcnow.return_value = datetime(2016, 12, 1) resp_mock().__class__ = Response @@ -336,22 +246,18 @@ class TestAuthTotp(TransactionCase): json_mock().serialize.assert_called_once_with(datetime(2016, 12, 31)) - @mock.patch(DATETIME_PATH) - @mock.patch(RESPONSE_PATH) - @mock.patch(JSON_PATH) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) + @patch(DATETIME_PATH) + @patch(RESPONSE_PATH) + @patch(JSON_PATH) + @patch(VALIDATE_PATH) def test_mfa_login_post_cookie_final_cookie( - self, val_mock, gen_mock, json_mock, resp_mock, dt_mock, request_mock + self, val_mock, json_mock, resp_mock, dt_mock, request_mock ): '''Should add correct cookie to response if remember flag set''' request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = { - 'mfa_login_token': test_token, - 'remember_device': True, - } + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) + request_mock.params = {'remember_device': True} val_mock.return_value = True dt_mock.utcnow.return_value = datetime(2016, 12, 1) config_model = self.env['ir.config_parameter'] @@ -369,20 +275,16 @@ class TestAuthTotp(TransactionCase): secure=False, ) - @mock.patch(RESPONSE_PATH) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) + @patch(RESPONSE_PATH) + @patch(VALIDATE_PATH) def test_mfa_login_post_cookie_final_cookie_secure( - self, val_mock, gen_mock, resp_mock, request_mock + self, val_mock, resp_mock, request_mock ): '''Should set secure cookie if config parameter set accordingly''' request_mock.env = self.env - request_mock.db = self.registry.db_name - test_token = self.test_user.mfa_login_token - request_mock.params = { - 'mfa_login_token': test_token, - 'remember_device': True, - } + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) + request_mock.params = {'remember_device': True} val_mock.return_value = True config_model = self.env['ir.config_parameter'] config_model.set_param('auth_totp.secure_cookie', '1') @@ -393,18 +295,16 @@ class TestAuthTotp(TransactionCase): new_test_security = resp_mock().set_cookie.mock_calls[0][2]['secure'] self.assertIs(new_test_security, True) - @mock.patch(REDIRECT_PATH) - @mock.patch(GENERATE_PATH) - @mock.patch(VALIDATE_PATH) + @patch(REDIRECT_PATH) + @patch(VALIDATE_PATH) def test_mfa_login_post_firefox_response_returned( - self, val_mock, gen_mock, redirect_mock, request_mock + self, val_mock, redirect_mock, request_mock ): '''Should behave well if redirect returns Response (Firefox case)''' request_mock.env = self.env - request_mock.db = self.registry.db_name + request_mock.session = AssignableDict(login=self.test_user.login) + request_mock.session.authenticate = MagicMock(return_value=True) redirect_mock.return_value = Response('Test Response') - test_token = self.test_user.mfa_login_token - request_mock.params = {'mfa_login_token': test_token} val_mock.return_value = True test_result = self.test_controller.mfa_login_post() diff --git a/auth_totp/tests/test_post_migrate.py b/auth_totp/tests/test_post_migrate.py new file mode 100644 index 000000000..7f1c4784a --- /dev/null +++ b/auth_totp/tests/test_post_migrate.py @@ -0,0 +1,124 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from mock import patch +import os +import sys +from odoo.modules.module import get_module_path +from odoo.tests.common import TransactionCase + +module_path = get_module_path('auth_totp') +migration_path = os.path.join(module_path, 'migrations', '10.0.2.0.0') +sys.path.insert(0, migration_path) +sys.modules.pop('auth_totp', None) +post_migrate = __import__('post-migrate') +migrate = post_migrate.migrate + +HELPER_PATH = 'odoo.addons.base.ir.ir_model.IrModelFields._prepare_update' + + +@patch(HELPER_PATH, autospec=True) +class TestPostMigrate(TransactionCase): + def setUp(self): + super(TestPostMigrate, self).setUp() + + self.test_user = self.env['res.users'].create({ + 'name': 'Test User', + 'login': 'test_user', + }) + self.env['res.users.authenticator'].create({ + 'name': 'Test Name', + 'secret_key': 'Test Key', + 'user_id': self.test_user.id, + }) + self.test_user.mfa_enabled = True + self.test_user.active = False + + self.test_user_2 = self.env['res.users'].create({ + 'name': 'Test User 2', + 'login': 'test_user_2', + }) + + self.test_device_model = self.env['ir.model'].create({ + 'name': 'Test Device Model', + 'model': 'res.users.device', + 'state': 'base', + }) + + def test_migrate_mfa_enabled(self, helper_mock): + """It should give users with MFA enabled a new key""" + old_key = self.test_user.trusted_device_cookie_key + migrate(self.env.cr, None) + + self.assertTrue(self.test_user.trusted_device_cookie_key) + self.assertNotEqual(self.test_user.trusted_device_cookie_key, old_key) + + def test_migrate_mfa_disabled(self, helper_mock): + """It should leave users with MFA disabled without a key""" + migrate(self.env.cr, None) + + self.assertFalse(self.test_user_2.trusted_device_cookie_key) + + def test_migrate_call_field_helper(self, helper_mock): + """It should call update helper on all device model field records""" + test_field = self.test_device_model.field_id + test_field_2 = self.env['ir.model.fields'].create({ + 'name': 'test_field_2', + 'model': self.test_device_model.model, + 'model_id': self.test_device_model.id, + 'ttype': 'char', + 'state': 'base', + }) + test_field_set = test_field + test_field_2 + migrate(self.env.cr, None) + + helper_mock.assert_called_once_with(test_field_set) + + def test_migrate_clean_up_constraints(self, helper_mock): + """It should clean up all constraints associated with device model""" + test_module_record = self.env['ir.module.module'].search([], limit=1) + self.env['ir.model.constraint'].create({ + 'name': 'Test Constraint', + 'model': self.test_device_model.id, + 'module': test_module_record.id, + 'type': 'u', + }) + self.env['ir.model.constraint'].create({ + 'name': 'Test Constraint 2', + 'model': self.test_device_model.id, + 'module': test_module_record.id, + 'type': 'u', + }) + migrate(self.env.cr, None) + + resulting_constraints = self.env['ir.model.constraint'].search([ + ('model', '=', self.test_device_model.id), + ]) + self.assertFalse(resulting_constraints) + + def test_migrate_clean_up_xml_ids(self, helper_mock): + """It should clean up XML IDs tied to device model""" + self.env['ir.model.data'].create({ + 'name': 'Test XML ID', + 'model': 'ir.model', + 'res_id': self.test_device_model.id, + }) + self.env['ir.model.data'].create({ + 'name': 'Test XML ID 2', + 'model': 'ir.model', + 'res_id': self.test_device_model.id, + }) + migrate(self.env.cr, None) + + resulting_xml_ids = self.env['ir.model.data'].search([ + ('model', '=', 'ir.model'), + ('res_id', '=', self.test_device_model.id), + ]) + self.assertFalse(resulting_xml_ids) + + def test_migrate_clean_up_ir_record(self, helper_mock): + """It should clean up device model ir.model record""" + migrate(self.env.cr, None) + + self.assertFalse(self.test_device_model.exists()) diff --git a/auth_totp/tests/test_res_users.py b/auth_totp/tests/test_res_users.py index 60c38cc37..caf145d11 100644 --- a/auth_totp/tests/test_res_users.py +++ b/auth_totp/tests/test_res_users.py @@ -2,19 +2,15 @@ # Copyright 2016-2017 LasLabs Inc. # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). -from datetime import datetime -import mock -import string -from odoo.exceptions import ValidationError +from mock import patch +from odoo.exceptions import AccessDenied, ValidationError from odoo.tests.common import TransactionCase -from ..exceptions import ( - MfaTokenError, - MfaTokenInvalidError, - MfaTokenExpiredError, -) +from ..exceptions import MfaLoginNeeded +from ..models.res_users import JsonSecureCookie from ..models.res_users_authenticator import ResUsersAuthenticator -DATETIME_PATH = 'odoo.addons.auth_totp.models.res_users.datetime' +MODEL_PATH = 'odoo.addons.auth_totp.models.res_users' +REQUEST_PATH = MODEL_PATH + '.request' class TestResUsers(TransactionCase): @@ -22,11 +18,34 @@ 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 + self.env['res.users.authenticator'].create({ + 'name': 'Test Name', + 'secret_key': 'Test Key', + 'user_id': self.test_user.id, + }) + self.test_user.mfa_enabled = True + self.env.uid = self.test_user.id + def test_compute_trusted_device_cookie_key_disable_mfa(self): + """It should clear out existing key when MFA is disabled""" + self.test_user.mfa_enabled = False + + self.assertFalse(self.test_user.trusted_device_cookie_key) + + def test_compute_trusted_device_cookie_key_enable_mfa(self): + """It should generate a new key when MFA is enabled""" + old_key = self.test_user.trusted_device_cookie_key + self.test_user.mfa_enabled = False + self.test_user.mfa_enabled = True + + self.assertNotEqual(self.test_user.trusted_device_cookie_key, old_key) + def test_build_model_mfa_fields_in_self_writeable_list(self): '''Should add MFA fields to list of fields users can modify for self''' ResUsersClass = type(self.test_user) @@ -36,155 +55,205 @@ class TestResUsers(TransactionCase): def test_check_enabled_with_authenticator_mfa_no_auth(self): '''Should raise correct error if MFA enabled without authenticators''' with self.assertRaisesRegexp(ValidationError, 'locked out'): - self.test_user.mfa_enabled = True + self.test_user.authenticator_ids = False def test_check_enabled_with_authenticator_no_mfa_auth(self): '''Should not raise error if MFA not enabled with authenticators''' try: - self.env['res.users.authenticator'].create({ - 'name': 'Test Name', - 'secret_key': 'Test Key', - 'user_id': self.test_user.id, - }) + self.test_user.mfa_enabled = False except ValidationError: self.fail('A ValidationError was raised and should not have been.') - def test_check_enabled_with_authenticator_mfa_auth(self): - '''Should not raise error if MFA enabled with authenticators''' + @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.env['res.users.authenticator'].create({ - 'name': 'Test Name', - 'secret_key': 'Test Key', - 'user_id': self.test_user.id, - }) - self.test_user.mfa_enabled = True - except ValidationError: - self.fail('A ValidationError was raised and should not have been.') + 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 - def test_check_credentials_no_match(self): - '''Should raise appropriate error if there is no match''' - with self.assertRaises(MfaTokenInvalidError): + with self.assertRaises(AccessDenied): self.env['res.users'].check_credentials('invalid') - - @mock.patch(DATETIME_PATH) - def test_check_credentials_expired(self, datetime_mock): - '''Should raise appropriate error if match based on expired token''' - datetime_mock.now.return_value = datetime(2016, 12, 1) - self.test_user.generate_mfa_login_token() - test_token = self.test_user.mfa_login_token - datetime_mock.now.return_value = datetime(2017, 12, 1) - - with self.assertRaises(MfaTokenExpiredError): - self.env['res.users'].check_credentials(test_token) - - def test_check_credentials_current(self): - '''Should not raise error if match based on active token''' - self.test_user.generate_mfa_login_token() - test_token = self.test_user.mfa_login_token - try: - self.env['res.users'].check_credentials(test_token) - except MfaTokenError: - self.fail('An MfaTokenError was raised and should not have been.') - - def test_generate_mfa_login_token_token_field_content(self): - '''Should set token field to 20 char string of ASCII letters/digits''' - self.test_user.generate_mfa_login_token() - test_chars = set(string.ascii_letters + string.digits) - - self.assertEqual(len(self.test_user.mfa_login_token), 20) - self.assertTrue(set(self.test_user.mfa_login_token) <= test_chars) - - def test_generate_mfa_login_token_token_field_random(self): - '''Should set token field to new value each time''' - test_tokens = set([]) - for __ in xrange(3): - self.test_user.generate_mfa_login_token() - test_tokens.add(self.test_user.mfa_login_token) - - self.assertEqual(len(test_tokens), 3) - - @mock.patch(DATETIME_PATH) - def test_generate_mfa_login_token_exp_field_default(self, datetime_mock): - '''Should set token lifetime to 15 minutes if no argument provided''' - datetime_mock.now.return_value = datetime(2016, 12, 1) - self.test_user.generate_mfa_login_token() + self.env['res.users'].check_credentials('admin') + except AccessDenied: + self.fail('An exception was raised with a correct password.') - self.assertEqual( - self.test_user.mfa_login_token_exp, - '2016-12-01 00:15:00' - ) - - @mock.patch(DATETIME_PATH) - def test_generate_mfa_login_token_exp_field_custom(self, datetime_mock): - '''Should set token lifetime to value provided''' - datetime_mock.now.return_value = datetime(2016, 12, 1) - self.test_user.generate_mfa_login_token(45) - - self.assertEqual( - self.test_user.mfa_login_token_exp, - '2016-12-01 00:45:00' - ) - - def test_user_from_mfa_login_token_validate_not_singleton(self): - '''Should raise correct error when recordset is not a singleton''' - self.test_user.copy() - test_set = self.env['res.users'].search([('id', '>', 0)], limit=2) - - with self.assertRaises(MfaTokenInvalidError): - self.env['res.users']._user_from_mfa_login_token_validate() - with self.assertRaises(MfaTokenInvalidError): - test_set._user_from_mfa_login_token_validate() + 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 - @mock.patch(DATETIME_PATH) - def test_user_from_mfa_login_token_validate_expired(self, datetime_mock): - '''Should raise correct error when record has expired token''' - datetime_mock.now.return_value = datetime(2016, 12, 1) - self.test_user.generate_mfa_login_token() - datetime_mock.now.return_value = datetime(2017, 12, 1) + result_cache = self.test_model._mfa_uid_cache[self.env.cr.dbname] + self.assertEqual(result_cache, {self.test_user.id}) - with self.assertRaises(MfaTokenExpiredError): - self.test_user._user_from_mfa_login_token_validate() + @patch(REQUEST_PATH, new=None) + def test_check_credentials_mfa_and_no_request(self): + '''Should raise correct exception if MFA enabled and no request''' + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') + with self.assertRaises(MfaLoginNeeded): + self.env['res.users'].check_credentials('admin') - def test_user_from_mfa_login_token_validate_current_singleton(self): - '''Should not raise error when one record with active token''' - self.test_user.generate_mfa_login_token() + @patch(REQUEST_PATH) + def test_check_credentials_mfa_login_active(self, request_mock): + '''Should check password if user has finished MFA auth this session''' + request_mock.session = {'mfa_login_active': self.test_user.id} + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') try: - self.test_user._user_from_mfa_login_token_validate() - except MfaTokenError: - self.fail('An MfaTokenError was raised and should not have been.') + self.env['res.users'].check_credentials('admin') + except AccessDenied: + self.fail('An exception was raised with a correct password.') - def test_user_from_mfa_login_token_match(self): - '''Should retreive correct user when there is a current match''' - self.test_user.generate_mfa_login_token() - test_token = self.test_user.mfa_login_token + @patch(REQUEST_PATH) + def test_check_credentials_mfa_different_login_active(self, request_mock): + '''Should correctly raise/update if other user finished MFA auth''' + request_mock.session = {'mfa_login_active': self.test_user.id + 1} + request_mock.httprequest.cookies = {} - self.assertEqual( - self.env['res.users'].user_from_mfa_login_token(test_token), - self.test_user, + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') + self.assertFalse(request_mock.session.get('mfa_login_needed')) + with self.assertRaises(MfaLoginNeeded): + self.env['res.users'].check_credentials('admin') + self.assertTrue(request_mock.session.get('mfa_login_needed')) + + @patch(REQUEST_PATH) + def test_check_credentials_mfa_no_device_cookie(self, request_mock): + '''Should correctly raise/update session if MFA and no device cookie''' + request_mock.session = {'mfa_login_active': False} + request_mock.httprequest.cookies = {} + + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') + self.assertFalse(request_mock.session.get('mfa_login_needed')) + with self.assertRaises(MfaLoginNeeded): + self.env['res.users'].check_credentials('admin') + self.assertTrue(request_mock.session.get('mfa_login_needed')) + + @patch(REQUEST_PATH) + def test_check_credentials_mfa_corrupted_device_cookie(self, request_mock): + '''Should correctly raise/update session if MFA and corrupted cookie''' + request_mock.session = {'mfa_login_active': False} + test_key = 'trusted_devices_%d' % self.test_user.id + request_mock.httprequest.cookies = {test_key: 'invalid'} + + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') + self.assertFalse(request_mock.session.get('mfa_login_needed')) + with self.assertRaises(MfaLoginNeeded): + self.env['res.users'].check_credentials('admin') + self.assertTrue(request_mock.session.get('mfa_login_needed')) + + @patch(REQUEST_PATH) + def test_check_credentials_mfa_cookie_from_wrong_user(self, request_mock): + '''Should raise and update session if MFA and wrong user's cookie''' + request_mock.session = {'mfa_login_active': False} + test_user_2 = self.env['res.users'].create({ + 'name': 'Test User', + 'login': 'test_user', + }) + test_id_2 = test_user_2.id + self.env['res.users.authenticator'].create({ + 'name': 'Test Name', + 'secret_key': 'Test Key', + 'user_id': test_id_2, + }) + test_user_2.mfa_enabled = True + secret = test_user_2.trusted_device_cookie_key + test_device_cookie = JsonSecureCookie({'user_id': test_id_2}, secret) + test_device_cookie = test_device_cookie.serialize() + test_key = 'trusted_devices_%d' % self.test_user.id + request_mock.httprequest.cookies = {test_key: test_device_cookie} + + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') + self.assertFalse(request_mock.session.get('mfa_login_needed')) + with self.assertRaises(MfaLoginNeeded): + self.env['res.users'].check_credentials('admin') + self.assertTrue(request_mock.session.get('mfa_login_needed')) + + @patch(REQUEST_PATH) + def test_check_credentials_mfa_correct_device_cookie(self, request_mock): + '''Should check password if MFA and correct device cookie''' + request_mock.session = {'mfa_login_active': False} + secret = self.test_user.trusted_device_cookie_key + test_device_cookie = JsonSecureCookie( + {'user_id': self.test_user.id}, + secret, ) + test_device_cookie = test_device_cookie.serialize() + test_key = 'trusted_devices_%d' % self.test_user.id + request_mock.httprequest.cookies = {test_key: test_device_cookie} - def test_user_from_mfa_login_token_falsy(self): - '''Should raise correct error when token is falsy''' - with self.assertRaises(MfaTokenInvalidError): - self.env['res.users'].user_from_mfa_login_token(None) - - def test_user_from_mfa_login_token_no_match(self): - '''Should raise correct error when there is no match''' - with self.assertRaises(MfaTokenInvalidError): - self.env['res.users'].user_from_mfa_login_token('Test Token') - - @mock.patch(DATETIME_PATH) - def test_user_from_mfa_login_token_match_expired(self, datetime_mock): - '''Should raise correct error when the match is expired''' - datetime_mock.now.return_value = datetime(2016, 12, 1) - self.test_user.generate_mfa_login_token() - test_token = self.test_user.mfa_login_token - datetime_mock.now.return_value = datetime(2017, 12, 1) - - with self.assertRaises(MfaTokenExpiredError): - self.env['res.users'].user_from_mfa_login_token(test_token) + with self.assertRaises(AccessDenied): + self.env['res.users'].check_credentials('invalid') + try: + self.env['res.users'].check_credentials('admin') + except AccessDenied: + self.fail('An exception was raised with a correct password.') def test_validate_mfa_confirmation_code_not_singleton(self): '''Should raise correct error when recordset is not singleton''' @@ -197,7 +266,7 @@ class TestResUsers(TransactionCase): with self.assertRaisesRegexp(ValueError, 'Expected singleton'): test_set.validate_mfa_confirmation_code('Test Code') - @mock.patch.object(ResUsersAuthenticator, 'validate_conf_code') + @patch.object(ResUsersAuthenticator, 'validate_conf_code') def test_validate_mfa_confirmation_code_singleton_return(self, mock_func): '''Should return validate_conf_code() value if singleton recordset''' mock_func.return_value = 'Test Result' diff --git a/auth_totp/views/auth_totp.xml b/auth_totp/views/auth_totp.xml index 33c2a21d0..0042b4e89 100644 --- a/auth_totp/views/auth_totp.xml +++ b/auth_totp/views/auth_totp.xml @@ -11,7 +11,6 @@