Merge pull request #1065 from LasLabs/bugfix/10.0/LABS-493-auth_totp-MFA-ignored-by-RPC-and-other-fixes

[10.0][FIX] auth_totp: Various issues

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 
-<https://github.com/OCA/server-tools/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 <https://github.com/OCA/server-tools/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 <https://github.com/OCA/maintainer-tools/blob/master/template/module/static/description/icon.svg>`_.
+* Odoo Community Association:
+  `Icon <https://github.com/OCA/maintainer-tools/blob/master/template/module/static/description/icon.svg>`_.
 
 Contributors
 ------------

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,

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,54 +16,20 @@ 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
+        if request.session.get('mfa_login_needed'):
+            request.session['mfa_login_needed'] = False
             return http.local_redirect(
                 '/auth_totp/login',
-            query={
-                'mfa_login_token': user.mfa_login_token,
-                'redirect': request.params.get('redirect'),
-            },
+                query={'redirect': request.params.get('redirect')},
                 keep_hash=True,
             )
 
+        return response
+
     @http.route(
         '/auth_totp/login',
         type='http',
@@ -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,20 +86,14 @@ 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)
+        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')
@@ -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)

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

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],
+    )

auth_totp/models/__init__.py

@@ -4,4 +4,3 @@
 
 from . import res_users
 from . import res_users_authenticator
-from . import res_users_device

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.'
                 ))
 
-    @api.model
-    def check_credentials(self, password):
-        try:
-            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)
-            )
+    @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)
 
-            expiration = datetime.now() + timedelta(minutes=lifetime_mins)
-            record.mfa_login_token_exp = fields.Datetime.to_string(expiration)
+        return super(ResUsers, cls).check(db, uid, 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.'
-            ))
+    def check_credentials(self, password):
+        """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)
 
-        user = self.search([('mfa_login_token', '=', token)])
-        user._user_from_mfa_login_token_validate()
+        self._mfa_uid_cache[self.env.cr.dbname].add(self.env.uid)
 
-        return user
+        if request:
+            if request.session.get('mfa_login_active') == self.env.uid:
+                return super(ResUsers, self).check_credentials(password)
 
-    @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.'
-            ))
+            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)
 
-        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):

auth_totp/models/res_users_device.py

@@ -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,
-    )

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

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

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()

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())

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)
+            self.env['res.users'].check_credentials('admin')
+        except AccessDenied:
+            self.fail('An exception was raised with a correct password.')
 
-    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.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'

auth_totp/views/auth_totp.xml

@@ -11,7 +11,6 @@
             <form class="oe_login_form" role="form" t-attf-action="/auth_totp/login" method="post" onsubmit="this.action = this.action + location.hash">
                 <input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
                 <input type="hidden" name="redirect" t-att-value="redirect"/>
-                <input type="hidden" name="mfa_login_token" t-att-value="mfa_login_token"/>
                 <div class="form-group field-login">
                     <label for="confirmation_code" class="control-label">MFA Confirmation Code</label>
                     <input type="text" name="confirmation_code" id="confirmation_code" class="form-control" required="required" autofocus="autofocus" autocapitalize="off"/>