[CHG] Change login/auth internal protocol

* Make Users._login and session.authenticate always raise AccessDenied
  on authentication failure instead of only sometimes (cf
  Session.authenticate calling security.check() which raises and not
  catching the exception)
* Alter AccessDenied such that it's possible to add a custom access
  message, for use with login rate limiting instead of smuggling the
  information via the session
* Alter the RPC endpoints to catch and convert AccessDenied back to
  a boolean sentinel

7 files changed, 53 insertions, 47 deletions

diff --git a/addons/auth_ldap/models/res_users.py b/addons/auth_ldap/models/res_users.py
index 9e783064557..f0c8bb79e2e 100644
--- a/addons/auth_ldap/models/res_users.py
+++ b/addons/auth_ldap/models/res_users.py
@@ -11,23 +11,22 @@ class Users(models.Model):
 
     @classmethod
     def _login(cls, db, login, password):
-        user_id = super(Users, cls)._login(db, login, password)
-        if user_id:
-            return user_id
-        with registry(db).cursor() as cr:
-            cr.execute("SELECT id FROM res_users WHERE lower(login)=%s", (login,))
-            res = cr.fetchone()
-            if res:
-                return False
-            env = api.Environment(cr, SUPERUSER_ID, {})
-            Ldap = env['res.company.ldap']
-            for conf in Ldap.get_ldap_dicts():
-                entry = Ldap.authenticate(conf, login, password)
-                if entry:
-                    user_id = Ldap.get_or_create_user(conf, login, entry)
-                    if user_id:
-                        break
-            return user_id
+        try:
+            return super(Users, cls)._login(db, login, password)
+        except AccessDenied as e:
+            with registry(db).cursor() as cr:
+                cr.execute("SELECT id FROM res_users WHERE lower(login)=%s", (login,))
+                res = cr.fetchone()
+                if res:
+                    raise e
+
+                env = api.Environment(cr, SUPERUSER_ID, {})
+                Ldap = env['res.company.ldap']
+                for conf in Ldap.get_ldap_dicts():
+                    entry = Ldap.authenticate(conf, login, password)
+                    if entry:
+                        return Ldap.get_or_create_user(conf, login, entry)
+                raise e
 
     def _check_credentials(self, password):
         try:
diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py
index 1269c175cd0..ff4807af25c 100644
--- a/addons/web/controllers/main.py
+++ b/addons/web/controllers/main.py
@@ -42,7 +42,7 @@ from odoo.tools.safe_eval import safe_eval
 from odoo import http
 from odoo.http import content_disposition, dispatch_rpc, request, \
     serialize_exception as _serialize_exception, Response
-from odoo.exceptions import AccessError, UserError
+from odoo.exceptions import AccessError, UserError, AccessDenied
 from odoo.models import check_method_name
 from odoo.service import db
 
@@ -482,18 +482,17 @@ class Home(http.Controller):
             values['databases'] = None
 
         if request.httprequest.method == 'POST':
-            # ensure there's no leftover in the session from non-web auth
-            request.session.pop('login_error', None)
             old_uid = request.uid
-            uid = request.session.authenticate(request.session.db, request.params['login'], request.params['password'])
-            if uid is not False:
+            try:
+                uid = request.session.authenticate(request.session.db, request.params['login'], request.params['password'])
                 request.params['login_success'] = True
                 return http.redirect_with_hash(self._login_redirect(uid, redirect=redirect))
-            request.uid = old_uid
-            values['error'] = request.session.pop(
-                'login_error',
-                _("Wrong login/password")
-            )
+            except odoo.exceptions.AccessDenied as e:
+                request.uid = old_uid
+                if e.args == odoo.exceptions.AccessDenied().args:
+                    values['error'] = _("Wrong login/password")
+                else:
+                    values['error'] = e.args[0]
         else:
             if 'error' in request.params and request.params.get('error') == 'access':
                 values['error'] = _('Only employee can access this database. Please contact the administrator.')
diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py
index 43b0ebba856..8526f9e7bd5 100644
--- a/odoo/addons/base/models/res_users.py
+++ b/odoo/addons/base/models/res_users.py
@@ -545,25 +545,26 @@ class Users(models.Model):
     @classmethod
     def _login(cls, db, login, password):
         if not password:
-            return False
-        user_id = False
+            raise AccessDenied()
+        ip = request.httprequest.environ['REMOTE_ADDR'] if request else 'n/a'
         try:
             with cls.pool.cursor() as cr:
                 self = api.Environment(cr, SUPERUSER_ID, {})[cls._name]
                 with self._assert_can_auth():
                     user = self.search([('login', '=', login)])
-                    if user:
-                        user_id = user.id
-                        user.sudo(user_id)._check_credentials(password)
-                        user.sudo(user_id)._update_last_login()
+                    if not user:
+                        raise AccessDenied()
+
+                    user = user.sudo(user.id)
+                    user._check_credentials(password)
+                    user._update_last_login()
         except AccessDenied:
-            user_id = False
+            _logger.info("Login failed for db:%s login:%s from %s", db, login, ip)
+            raise
 
-        status = "successful" if user_id else "failed"
-        ip = request.httprequest.environ['REMOTE_ADDR'] if request else 'n/a'
-        _logger.info("Login %s for db:%s login:%s from %s", status, db, login, ip)
+        _logger.info("Login successful for db:%s login:%s from %s", db, login, ip)
 
-        return user_id
+        return user.id
 
     @classmethod
     def authenticate(cls, db, login, password, user_agent_env):
@@ -874,8 +875,6 @@ class Users(models.Model):
         source = request.httprequest.remote_addr
         (failures, previous) = failures_map[source]
         if self._on_login_cooldown(failures, previous):
-            request.session['login_error'] = _("Too many login failures, please wait a bit before trying again.")
-
             _logger.warn(
                 "Login attempt ignored for %s on %s: "
                 "%d failures since last success, last failure at %s. "
@@ -893,7 +892,7 @@ class Users(models.Model):
                     "https://www.odoo.com/documentation/11.0/setup/deploy.html#https for details.",
                     source
                 )
-            raise AccessDenied()
+            raise AccessDenied(_("Too many login failures, please wait a bit before trying again."))
 
         try:
             yield
diff --git a/odoo/exceptions.py b/odoo/exceptions.py
index e1689ffd27f..c969d077a86 100644
--- a/odoo/exceptions.py
+++ b/odoo/exceptions.py
@@ -49,10 +49,12 @@ class RedirectWarning(Exception):
 
 
 class AccessDenied(Exception):
-    """ Login/password error. No message, no traceback.
+    """ Login/password error. no traceback.
     Example: When you try to log with a wrong password."""
-    def __init__(self):
-        super(AccessDenied, self).__init__('Access denied')
+    def __init__(self, message='Access denied'):
+        super(AccessDenied, self).__init__(message)
+        self.with_traceback(None)
+        self.__cause__ = None
         self.traceback = ('', '', '')
 
 
diff --git a/odoo/http.py b/odoo/http.py
index c8604a87b20..d32b4caa5e3 100644
--- a/odoo/http.py
+++ b/odoo/http.py
@@ -1032,7 +1032,7 @@ class OpenERPSession(werkzeug.contrib.sessions.Session):
                 HTTP_HOST=wsgienv['HTTP_HOST'],
                 REMOTE_ADDR=wsgienv['REMOTE_ADDR'],
             )
-            uid = dispatch_rpc('common', 'authenticate', [db, login, password, env])
+            uid = odoo.registry(db)['res.users'].authenticate(db, login, password, env)
         else:
             security.check(db, uid, password)
         self.db = db
diff --git a/odoo/service/common.py b/odoo/service/common.py
index 36f61b0cab5..42ec4ca3738 100644
--- a/odoo/service/common.py
+++ b/odoo/service/common.py
@@ -4,6 +4,7 @@ import logging
 
 import odoo.release
 import odoo.tools
+from odoo.exceptions import AccessDenied
 from odoo.tools.translate import _
 
 from . import security
@@ -27,7 +28,10 @@ def exp_login(db, login, password):
 
 def exp_authenticate(db, login, password, user_agent_env):
     res_users = odoo.registry(db)['res.users']
-    return res_users.authenticate(db, login, password, user_agent_env)
+    try:
+        return res_users.authenticate(db, login, password, user_agent_env)
+    except AccessDenied:
+        return False
 
 def exp_version():
     return RPC_VERSION_1
diff --git a/odoo/service/security.py b/odoo/service/security.py
index 7fdffe2d647..703fd3687f3 100644
--- a/odoo/service/security.py
+++ b/odoo/service/security.py
@@ -6,7 +6,10 @@ import odoo.exceptions
 
 def login(db, login, password):
     res_users = odoo.registry(db)['res.users']
-    return res_users._login(db, login, password)
+    try:
+        return res_users._login(db, login, password)
+    except odoo.exceptions.AccessDenied:
+        return False
 
 def check(db, uid, passwd):
     res_users = odoo.registry(db)['res.users']