[IMP] auth_totp: allow spaces in totp code input

TOTP programs generally group the code into two groups of 3 digits,
but we'd only allow a single group of 6 digits.

Allow spaces in the value for a bit of flexibility, and fix
placeholders to look like codes (also turns out @placeholder on a
field doesn't do anything, not sure where I got this idea).

Also improve the label slightly in the login flow:

* add information to the label itself
* properly link the label & input via an `id`

4 files changed, 20 insertions, 11 deletions

diff --git a/addons/auth_totp/controllers/home.py b/addons/auth_totp/controllers/home.py
index f68f9ed3896..10c2461ebf6 100644
--- a/addons/auth_totp/controllers/home.py
+++ b/addons/auth_totp/controllers/home.py
@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+import re
+
 import odoo.addons.web.controllers.main
 from odoo import http, _
 from odoo.exceptions import AccessDenied
@@ -23,7 +25,7 @@ class Home(odoo.addons.web.controllers.main.Home):
             user = request.env['res.users'].browse(request.session.pre_uid)
             try:
                 with user._assert_can_auth():
-                    user._totp_check(int(kwargs['totp_token']))
+                    user._totp_check(int(re.sub(r'\s', '', kwargs['totp_token'])))
             except AccessDenied:
                 error = _("Verification failed, please double-check the 6-digit code")
             except ValueError:
diff --git a/addons/auth_totp/models/res_users.py b/addons/auth_totp/models/res_users.py
index 06aad891550..a4de4bb89ba 100644
--- a/addons/auth_totp/models/res_users.py
+++ b/addons/auth_totp/models/res_users.py
@@ -1,9 +1,11 @@
 # -*- coding: utf-8 -*-
 import base64
+import functools
 import hmac
 import io
 import logging
 import os
+import re
 import struct
 import time
 
@@ -16,6 +18,7 @@ from odoo.http import request
 
 _logger = logging.getLogger(__name__)
 
+compress = functools.partial(re.sub, r'\s', '')
 class Users(models.Model):
     _inherit = 'res.users'
 
@@ -49,7 +52,7 @@ class Users(models.Model):
 
     def _totp_check(self, code):
         sudo = self.sudo()
-        key = base64.b32decode(sudo.totp_secret.upper())
+        key = base64.b32decode(sudo.totp_secret)
         match = TOTP(key).match(code)
         if match is None:
             _logger.info("2FA check: FAIL for '%s' (#%s)", self.login, self.id)
@@ -61,7 +64,8 @@ class Users(models.Model):
             _logger.info("2FA enable: REJECT for '%s' (#%s)", self.login, self.id)
             return False
 
-        match = TOTP(base64.b32decode(secret.upper())).match(code)
+        secret = compress(secret).upper()
+        match = TOTP(base64.b32decode(secret)).match(code)
         if match is None:
             _logger.info("2FA enable: REJECT CODE for '%s' (#%s)", self.login, self.id)
             return False
@@ -101,9 +105,12 @@ class Users(models.Model):
             raise UserError(_("Two-factor authentication already enabled"))
 
         secret_bytes_count = TOTP_SECRET_SIZE // 8
+        secret = base64.b32encode(os.urandom(secret_bytes_count)).decode()
+        # format secret in groups of 4 characters for readability
+        secret = ' '.join(map(''.join, zip(*[iter(secret)]*4)))
         w = self.env['auth_totp.wizard'].create({
             'user_id': self.id,
-            'secret': base64.b32encode(os.urandom(secret_bytes_count)).decode(),
+            'secret': secret,
         })
         return {
             'type': 'ir.actions.act_window',
@@ -125,7 +132,7 @@ class TOTPWizard(models.TransientModel):
         attachment=False, store=True, readonly=True,
         compute='_compute_qrcode',
     )
-    code = fields.Char(string="Verification Code", placeholder="6-digit code", size=6)
+    code = fields.Char(string="Verification Code", size=7)
 
     @api.depends('user_id.login', 'user_id.company_id.display_name', 'secret')
     def _compute_qrcode(self):
@@ -135,7 +142,7 @@ class TOTPWizard(models.TransientModel):
                 'otpauth', 'totp',
                 werkzeug.urls.url_quote(label, safe=''),
                 werkzeug.urls.url_encode({
-                    'secret': w.secret,
+                    'secret': compress(w.secret),
                     'issuer': w.user_id.company_id.display_name,
                     # apparently a lowercase hash name is anathema to google
                     # authenticator (error) and passlib (no token)
@@ -153,7 +160,7 @@ class TOTPWizard(models.TransientModel):
     @check_identity
     def enable(self):
         try:
-            c = int(self.code)
+            c = int(compress(self.code))
         except ValueError:
             raise UserError(_("The verification code should only contain numbers"))
         if self.user_id._totp_try_setting(self.secret, c):
diff --git a/addons/auth_totp/views/templates.xml b/addons/auth_totp/views/templates.xml
index 6ed3fdfac6f..e5065a1f8e1 100644
--- a/addons/auth_totp/views/templates.xml
+++ b/addons/auth_totp/views/templates.xml
@@ -13,9 +13,9 @@
                     <input type="hidden" name="csrf_token" t-att-value="request.csrf_token()"/>
                     <input type="hidden" name="redirect" t-att-value="redirect"/>
                     <div class="form-group">
-                        <label for="totp_token">Authentication Code</label>
-                        <input name="totp_token" class="form-control mb-2"
-                               autofocus="autofocus" required="required" placeholder="6-digit code"/>
+                        <label for="totp_token">Authentication Code (6 digits)</label>
+                        <input id="totp_token" name="totp_token" class="form-control mb-2"
+                               autofocus="autofocus" required="required"/>
                     </div>
                     <p class="alert alert-danger" t-if="error" role="alert">
                         <t t-esc="error"/>
diff --git a/addons/auth_totp/views/user_preferences.xml b/addons/auth_totp/views/user_preferences.xml
index b490efe2cbf..5bc86194158 100644
--- a/addons/auth_totp/views/user_preferences.xml
+++ b/addons/auth_totp/views/user_preferences.xml
@@ -48,7 +48,7 @@
                                 <div class="collapse" id="collapseTotpSecret">
                                   <div class="card card-body">
                                     <h3>Your two-factor secret:</h3>
-                                    <code><field name="secret"/></code>
+                                    <code class="text-center"><field name="secret"/></code>
                                   </div>
                                 </div>
                             </div>