Allow authentication methods to be combined

issue9303
review291621002

CHANGELOG

@@ -1,3 +1,4 @@
+* Allow combining authentication methods together
 * Add context to export CSV route
 
 Version 5.6.0 - 2020-05-04

doc/topics/configuration.rst

@@ -283,11 +283,19 @@ session
 authentications
 ~~~~~~~~~~~~~~~
 
-A comma separated list of login methods to use to authenticate the user.
-By default, Tryton supports only the `password` method which compare the
-password entered by the user against a stored hash. But other modules can
-define new methods (please refers to their documentation).
-The methods are tested following the order of the list.
+A comma separated list of the authentication methods to try when attempting to
+verify a user's identity. Each method is tried in turn, following the order of
+the list, until one succeeds. In order to allow `multi-factor authentication`_,
+individual methods can be combined together using a plus (`+`) symbol.
+
+Example::
+
+    authentications = password+sms,ldap
+
+By default, Tryton only supports the `password` method.  This method compares
+the password entered by the user against a stored hash of the user's password.
+Other modules can define additional authentication methods, please refer to
+their documentation for more information.
 
 Default: `password`
 
@@ -496,3 +504,4 @@ Example::
 .. _SSL-CERT: https://docs.python.org/library/ssl.html#ssl.wrap_socket
 .. _STARTTLS: http://en.wikipedia.org/wiki/STARTTLS
 .. _WSGI middleware: https://en.wikipedia.org/wiki/Web_Server_Gateway_Interface#Specification_overview
+.. _`multi-factor authentication`: https://en.wikipedia.org/wiki/Multi-factor_authentication

trytond/res/user.py

@@ -11,6 +11,7 @@ import datetime
 import logging
 import uuid
 import mmap
+import re
 try:
     import secrets
 except ImportError:
@@ -59,8 +60,8 @@ __all__ = [
     'UserConfigStart', 'UserConfig',
     ]
 logger = logging.getLogger(__name__)
-_has_password = 'password' in config.get(
-    'session', 'authentications', default='password').split(',')
+_has_password = 'password' in re.split('[,+]', config.get(
+    'session', 'authentications', default='password'))
 
 passlib_path = config.get('password', 'passlib')
 if passlib_path:
@@ -625,17 +626,21 @@ class User(DeactivableMixin, ModelSQL, ModelView):
             LoginAttempt.add(login)
             raise RateLimitException()
         Transaction().atexit(time.sleep, random.randint(0, 2 ** count - 1))
-        for method in config.get(
+        for methods in config.get(
                 'session', 'authentications', default='password').split(','):
-            try:
-                func = getattr(cls, '_login_%s' % method)
-            except AttributeError:
-                logger.info('Missing login method: %s', method)
-                continue
-            user_id = func(login, parameters)
-            if user_id:
+            user_ids = set()
+            for method in methods.split('+'):
+                try:
+                    func = getattr(cls, '_login_%s' % method)
+                except AttributeError:
+                    logger.info('Missing login method: %s', method)
+                    break
+                user_ids.add(func(login, parameters))
+                if len(user_ids) != 1 or not all(user_ids):
+                    break
+            if len(user_ids) == 1 and all(user_ids):
                 LoginAttempt.remove(login)
-                return user_id
+                return user_ids.pop()
         LoginAttempt.add(login)
 
     @classmethod

trytond/tests/test_user.py

@@ -3,7 +3,8 @@
 import datetime
 import os
 import unittest
-from unittest.mock import patch, ANY
+from contextlib import contextmanager
+from unittest.mock import patch, ANY, Mock
 
 from trytond.tests.test_tryton import activate_module, with_transaction
 from trytond.pool import Pool
@@ -15,6 +16,16 @@ from trytond.transaction import Transaction
 FROM = 'tryton@example.com'
 
 
+@contextmanager
+def set_authentications(methods):
+    saved_methods = config.get('session', 'authentications')
+    config.set('session', 'authentications', methods)
+    try:
+        yield
+    finally:
+        config.set('session', 'authentications', saved_methods)
+
+
 class UserTestCase(unittest.TestCase):
     'Test User'
 
@@ -205,6 +216,34 @@ class UserTestCase(unittest.TestCase):
                     'password': user.password_reset,
                     }))
 
+    @with_transaction()
+    def test_authentications(self):
+        "Test authentications"
+        pool = Pool()
+        User = pool.get('res.user')
+
+        user = User(login='user')
+        user.save()
+
+        User._login_always = Mock(return_value=user.id)
+        User._login_different = Mock(return_value=user.id + 1)
+        User._login_never = Mock(return_value=None)
+
+        for methods, result in (
+                ('never,never', None),
+                ('never,always', user.id),
+                ('always,never', user.id),
+                ('always,always', user.id),
+                ('never+never', None),
+                ('never+always', None),
+                ('always+never', None),
+                ('always+always', user.id),
+                ('always+different', None),
+                ):
+            with self.subTest(methods=methods, result=result):
+                with set_authentications(methods):
+                    self.assertEqual(User.get_login('user', {}), result)
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromTestCase(UserTestCase)