From 29db699e9bd062d11d2f0749c26da6d2acae1b96 Mon Sep 17 00:00:00 2001 From: David Beguin Date: Mon, 17 May 2021 09:59:10 +0000 Subject: [PATCH] [IMP] auth_top, *: revamp Two-factor authentication flow Purpose ======= Review the UX of the 2-factor authentication flow in order to make it more clear and easy to use. Specifications ============== This commit applies multiple rewording of instructions, button, etc. Tests have been adapted accordingly. It also adds an 'invite to use two-factor authentication' flow that will send an email to the selected used to redirect them their account security settings. - If portal is not installed yet, the user is redirected to his account security settings in backend. - If portal is installed, the user is redirected to /my/profile if them are portal user. Otherwise, the redirection is still done at backend side. As the backend view of auth_totp wizard is used at frontend side, copyclipboard widget has to be rebuilt at frontend side (click event, style etc..). As API key section is now displayed only on debug mode, test urls have been adapted accordingly. Task-2487630 Part-of: odoo/odoo#71142 --- addons/auth_oauth/views/res_users_views.xml | 2 +- addons/auth_totp/__manifest__.py | 8 +- addons/auth_totp/data/mail_templates_data.xml | 29 ++ addons/auth_totp/models/res_users.py | 46 +++- .../static/src/scss/res_users_view_form.scss | 3 + addons/auth_totp/static/tests/totp_flow.js | 50 ++-- addons/auth_totp/views/templates.xml | 41 +-- addons/auth_totp/views/user_preferences.xml | 250 ++++++++++++------ addons/auth_totp_portal/__init__.py | 3 + addons/auth_totp_portal/models/__init__.py | 4 + addons/auth_totp_portal/models/res_users.py | 14 + .../static/src/js/totp_frontend.js | 53 +++- .../static/src/scss/auth_totp_portal.scss | 35 +++ .../static/tests/totp_portal.js | 14 +- .../static/src/img/logo_apple_store.png | Bin 0 -> 2982 bytes .../static/src/img/logo_google_play.png | Bin 0 -> 7478 bytes addons/portal/static/src/js/portal.js | 3 - .../portal/static/src/xml/portal_security.xml | 6 +- addons/web/static/src/legacy/xml/base.xml | 2 +- odoo/addons/base/models/res_users.py | 6 +- odoo/addons/base/views/res_users_views.xml | 26 +- .../test_apikeys/static/tests/apikey_flow.js | 6 +- 22 files changed, 430 insertions(+), 171 deletions(-) create mode 100644 addons/auth_totp/data/mail_templates_data.xml create mode 100644 addons/auth_totp/static/src/scss/res_users_view_form.scss create mode 100644 addons/auth_totp_portal/models/__init__.py create mode 100644 addons/auth_totp_portal/models/res_users.py create mode 100644 addons/auth_totp_portal/static/src/scss/auth_totp_portal.scss create mode 100644 addons/base_setup/static/src/img/logo_apple_store.png create mode 100644 addons/base_setup/static/src/img/logo_google_play.png diff --git a/addons/auth_oauth/views/res_users_views.xml b/addons/auth_oauth/views/res_users_views.xml index 369dcb331a9..a56b57dad5f 100644 --- a/addons/auth_oauth/views/res_users_views.xml +++ b/addons/auth_oauth/views/res_users_views.xml @@ -6,7 +6,7 @@ form - + diff --git a/addons/auth_totp/__manifest__.py b/addons/auth_totp/__manifest__.py index 208ae9600bd..7005abfb8b6 100644 --- a/addons/auth_totp/__manifest__.py +++ b/addons/auth_totp/__manifest__.py @@ -14,17 +14,21 @@ Note: logically, two-factor prevents password-based RPC access for users where it is enabled. In order to be able to execute RPC scripts, the user can setup API keys to replace their main password. """, - 'depends': ['web'], + 'depends': ['web', 'mail'], 'category': 'Extra Tools', 'auto_install': True, 'data': [ + 'data/mail_templates_data.xml', 'security/security.xml', 'views/user_preferences.xml', 'views/templates.xml', ], 'assets': { 'web.assets_tests': [ - 'auth_totp/static/**/*', + 'auth_totp/static/tests/**/*', + ], + 'web.assets_backend': [ + 'auth_totp/static/src/**/*', ], }, 'license': 'LGPL-3', diff --git a/addons/auth_totp/data/mail_templates_data.xml b/addons/auth_totp/data/mail_templates_data.xml new file mode 100644 index 00000000000..a17c9b105e1 --- /dev/null +++ b/addons/auth_totp/data/mail_templates_data.xml @@ -0,0 +1,29 @@ + + + + + TOTP on User: Invite by email + + Invitation to activate two-factor authentication on your Odoo account + ${object.partner_id.id} + ${object.partner_id.lang} + + +
+

+ Dear ${object.partner_id.name}

+ ${user.name} requested you activate two-factor authentication to protect your account.

+ Two-factor Authentication ("2FA") is a system of double authentication. The first one is done with your password and the second one with a code you get from a dedicated mobile app. Popular ones include Authy, Google Authenticator or the Microsoft Authenticator. + +

+ + Activate my two-factor Authentication + +

+

+
+ + + + diff --git a/addons/auth_totp/models/res_users.py b/addons/auth_totp/models/res_users.py index ac03ea05745..33b8593d567 100644 --- a/addons/auth_totp/models/res_users.py +++ b/addons/auth_totp/models/res_users.py @@ -41,6 +41,16 @@ class Users(models.Model): for r, v in zip(self, self.sudo()): r.totp_enabled = bool(v.totp_secret) + def action_open_my_account_settings(self): + action = { + "name": _("Account Security"), + "type": "ir.actions.act_window", + "res_model": "res.users", + "views": [[self.env.ref('auth_totp.res_users_view_form_security').id, "form"]], + "res_id": self.id, + } + return action + def _rpc_api_keys_only(self): # 2FA enabled means we can't allow password-based RPC self.ensure_one() @@ -79,8 +89,34 @@ class Users(models.Model): _logger.info("2FA enable: SUCCESS for %s %r", self, self.login) return True + def get_totp_invite_url(self): + return '/web#action=auth_totp.action_activate_two_factor_authentication' + + def action_totp_invite(self): + invite_template = self.env.ref('auth_totp.mail_template_totp_invite') + users_to_invite = self.sudo().filtered(lambda user: not user.totp_secret) + for user in users_to_invite: + email_values = { + 'email_from': self.env.user.email_formatted, + 'author_id': self.env.user.partner_id.id, + } + invite_template.send_mail(user.id, force_send=True, email_values=email_values, + notif_layout='mail.mail_notification_light') + + # Display a confirmation toaster + return { + 'type': 'ir.actions.client', + 'tag': 'display_notification', + 'params': { + 'type': 'info', + 'sticky': False, + 'message': _("Invitation to use two-factor authentication sent for the following user(s): %s", + ', '.join(users_to_invite.mapped('name'))), + } + } + @check_identity - def totp_disable(self): + def action_totp_disable(self): logins = ', '.join(map(repr, self.mapped('login'))) if not (self == self.env.user or self.env.user._is_admin() or self.env.su): _logger.info("2FA disable: REJECT for %s (%s) by uid #%s", self, logins, self.env.user.id) @@ -99,13 +135,13 @@ class Users(models.Model): 'tag': 'display_notification', 'params': { 'type': 'warning', - 'message': _("Two-factor authentication disabled for user(s) %s", logins), + 'message': _("Two-factor authentication disabled for the following user(s): %s", ', '.join(self.mapped('name'))), 'next': {'type': 'ir.actions.act_window_close'}, } } @check_identity - def totp_enable_wizard(self): + def action_totp_enable_wizard(self): if self.env.user != self: raise UserError(_("Two-factor authentication can only be enabled for yourself")) @@ -124,11 +160,13 @@ class Users(models.Model): 'type': 'ir.actions.act_window', 'target': 'new', 'res_model': 'auth_totp.wizard', - 'name': _("Enable Two-Factor Authentication"), + 'name': _("Two-Factor Authentication Activation"), 'res_id': w.id, 'views': [(False, 'form')], + 'context': self.env.context, } + class TOTPWizard(models.TransientModel): _name = 'auth_totp.wizard' _description = "Two-Factor Setup Wizard" diff --git a/addons/auth_totp/static/src/scss/res_users_view_form.scss b/addons/auth_totp/static/src/scss/res_users_view_form.scss new file mode 100644 index 00000000000..d5be28420d4 --- /dev/null +++ b/addons/auth_totp/static/src/scss/res_users_view_form.scss @@ -0,0 +1,3 @@ +.o_auth_2fa_btn { + font-size: 1.5em; +} diff --git a/addons/auth_totp/static/tests/totp_flow.js b/addons/auth_totp/static/tests/totp_flow.js index 6555b7c2d34..22a21af4e7c 100644 --- a/addons/auth_totp/static/tests/totp_flow.js +++ b/addons/auth_totp/static/tests/totp_flow.js @@ -39,10 +39,10 @@ tour.register('totp_tour_setup', { url: '/web' }, [...openUserProfileAtSecurityTab(), { content: "Open totp wizard", - trigger: 'button[name=totp_enable_wizard]', + trigger: 'button[name=action_totp_enable_wizard]', }, { content: "Check that we have to enter enhanced security mode", - trigger: 'div:contains("confirm your password")', + trigger: 'div:contains("enter your password")', run: () => {}, }, { content: "Input password", @@ -53,18 +53,21 @@ tour.register('totp_tour_setup', { trigger: "button:contains(Confirm Password)", }, { content: "Check the wizard has opened", - trigger: 'div:contains("Scan the image below")', + trigger: 'li:contains("When requested to do so")', run: () => {} }, { content: "Get secret from collapsed div", - trigger: 'a:contains("show the code")', + trigger: 'a:contains("Cannot scan it?")', run(helpers) { - const secret = this.$anchor.closest('div').find('code').text(); + const $secret = this.$anchor.closest('div').find('span[name=secret]'); + const $copyBtn = $secret.find('button'); + $copyBtn.remove(); + const secret = $secret.text(); ajax.jsonRpc('/totphook', 'call', { secret }).then((token) => { helpers._text(helpers._get_action_values('input[name=code]'), token); - helpers._click(helpers._get_action_values('button.btn-primary:contains(Enable)')); + helpers._click(helpers._get_action_values('button.btn-primary:contains(Activate)')); $('body').addClass('got-token') }); } @@ -77,7 +80,7 @@ tour.register('totp_tour_setup', { ...openUserProfileAtSecurityTab(), { content: "Check that the button has changed", - trigger: 'button:contains(Disable two-factor authentication)', + trigger: 'button[name=action_totp_disable]', run: () => {} }]); @@ -109,7 +112,7 @@ tour.register('totp_login_enabled', { helpers._text(helpers._get_action_values(), token); // FIXME: is there a way to put the button as its own step trigger without // the tour straight blowing through and not waiting for this? - helpers._click(helpers._get_action_values('button:contains("Verify")')); + helpers._click(helpers._get_action_values('button:contains("Login")')); }); } }, { @@ -117,16 +120,16 @@ tour.register('totp_login_enabled', { trigger: ".o_user_menu .oe_topbar_name", run: () => {} }, -// now go and disable totp would be annoying to do in a separate tour +// now go and disable two-factor authentication would be annoying to do in a separate tour // because we'd need to login & totp again as HttpCase.authenticate can't // succeed w/ totp enabled ...openUserProfileAtSecurityTab(), { content: "Open totp wizard", - trigger: 'button[name=totp_disable]', + trigger: 'button[name=action_totp_disable]', }, { content: "Check that we have to enter enhanced security mode", - trigger: 'div:contains("confirm your password")', + trigger: 'div:contains("enter your password")', run: () => {}, }, { content: "Input password", @@ -140,7 +143,7 @@ tour.register('totp_login_enabled', { ...openUserProfileAtSecurityTab(), { content: "Check that the button has changed", - trigger: 'button:contains(Enable two-factor authentication)', + trigger: 'button[name=action_totp_enable_wizard]', run: () => {} }]); @@ -212,20 +215,17 @@ tour.register('totp_admin_disables', { } const $row = this.$anchor.closest('tr'); const sel = $row.find('.o_list_record_selector input[type=checkbox]'); - const totp = $row[0].children[columns['totp_enabled']].querySelector('input'); - if (totp.checked) { - helpers.click(sel); - } + helpers.click(sel); } }, { content: "Open Actions menu", trigger: 'button.o_dropdown_toggler:contains("Action")' }, { content: "Select totp remover", - trigger: 'a.dropdown-item:contains(Disable TOTP on users)' + trigger: 'a.dropdown-item:contains(Disable two-factor authentication)' }, { // enhanced security yo content: "Check that we have to enter enhanced security mode", -trigger: 'div:contains("confirm your password")', +trigger: 'div:contains("enter your password")', run: () => {}, }, { content: "Input password", @@ -235,13 +235,13 @@ trigger: 'div:contains("confirm your password")', content: "Confirm", trigger: "button:contains(Confirm Password)", }, { - content: "check that demo user has been de-totp'd", + content: "open the user's form", trigger: "td.o_data_cell:contains(demo)", - run: function () { - const totpcell = this.$anchor.closest('tr')[0].children[columns['totp_enabled']]; - if (totpcell.querySelector('input').checked) { - throw new Error("totp should have been disabled on demo user"); - } - } +}, { + content: "go to Account security Tab", + trigger: "a.nav-link:contains(Account Security)", +}, { + content: "check that demo user has been de-totp'd", + trigger: "button:contains(Invite to use 2FA)", }]) }); diff --git a/addons/auth_totp/views/templates.xml b/addons/auth_totp/views/templates.xml index cd6c90cad38..af73f829b7c 100644 --- a/addons/auth_totp/views/templates.xml +++ b/addons/auth_totp/views/templates.xml @@ -5,34 +5,37 @@