From 6886b17b1ff3863053bb2150a8a83031d06c57c9 Mon Sep 17 00:00:00 2001 From: Oleg Bulkin Date: Mon, 18 Sep 2017 17:28:15 -0700 Subject: [PATCH 1/2] [FIX] password_security: auth_totp compatibility * Modify overloaded web_login action to not trigger a password expiration check if the login process is not complete yet (e.g. due to auth_totp) * Add unit test for new logic * Fix warning caused by unrelated unit test --- password_security/controllers/main.py | 3 ++- .../tests/test_password_security_home.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/password_security/controllers/main.py b/password_security/controllers/main.py index 5c9266b0c..e34bbe674 100644 --- a/password_security/controllers/main.py +++ b/password_security/controllers/main.py @@ -36,7 +36,8 @@ class PasswordSecurityHome(AuthSignupHome): def web_login(self, *args, **kw): ensure_db() response = super(PasswordSecurityHome, self).web_login(*args, **kw) - if not request.httprequest.method == 'POST': + login_success = request.params.get('login_success', True) + if not request.httprequest.method == 'POST' or not login_success: return response uid = request.session.authenticate( request.session.db, diff --git a/password_security/tests/test_password_security_home.py b/password_security/tests/test_password_security_home.py index f44230d97..ecfe9a077 100644 --- a/password_security/tests/test_password_security_home.py +++ b/password_security/tests/test_password_security_home.py @@ -114,6 +114,16 @@ class TestPasswordSecurityHome(TransactionCase): assets['web_login'](), res, ) + def test_web_login_login_success_flag(self): + """It should return result of super when login_success flag False""" + with self.mock_assets() as assets: + assets['request'].httprequest.method = 'POST' + assets['request'].params = {'login_success': False} + result = self.password_security_home.web_login() + + expected = assets['web_login']() + self.assertEqual(result, expected) + def test_web_login_authenticate(self): """ It should attempt authentication to obtain uid """ with self.mock_assets() as assets: @@ -217,6 +227,8 @@ class TestPasswordSecurityHome(TransactionCase): main.AuthSignupHome, 'get_auth_signup_qcontext', spec=dict ) as qcontext: assets['web_auth_signup'].side_effect = MockPassError + assets['request'].render.return_value = MockResponse() + res = self.password_security_home.web_auth_signup() assets['request'].render.assert_called_once_with( 'auth_signup.signup', qcontext(), From 38e28a5e85b42452baed4f977f9ae7fc5c1ea9a3 Mon Sep 17 00:00:00 2001 From: Oleg Bulkin Date: Wed, 20 Sep 2017 11:29:14 -0700 Subject: [PATCH 2/2] [ADD] auth_totp_password_security: Compatibility * Overload mfa_login_post action introduced by auth_totp so that password expiration check from password_security still happens during an MFA login * Add unit tests for new logic --- auth_totp_password_security/README.rst | 60 +++++++++++++++ auth_totp_password_security/__init__.py | 5 ++ auth_totp_password_security/__manifest__.py | 20 +++++ .../controllers/__init__.py | 5 ++ .../controllers/main.py | 26 +++++++ auth_totp_password_security/tests/__init__.py | 5 ++ .../tests/test_main.py | 77 +++++++++++++++++++ 7 files changed, 198 insertions(+) create mode 100644 auth_totp_password_security/README.rst create mode 100644 auth_totp_password_security/__init__.py create mode 100644 auth_totp_password_security/__manifest__.py create mode 100644 auth_totp_password_security/controllers/__init__.py create mode 100644 auth_totp_password_security/controllers/main.py create mode 100644 auth_totp_password_security/tests/__init__.py create mode 100644 auth_totp_password_security/tests/test_main.py diff --git a/auth_totp_password_security/README.rst b/auth_totp_password_security/README.rst new file mode 100644 index 000000000..31531b564 --- /dev/null +++ b/auth_totp_password_security/README.rst @@ -0,0 +1,60 @@ +.. image:: https://img.shields.io/badge/license-LGPL--3-blue.svg + :target: http://www.gnu.org/licenses/lgpl-3.0-standalone.html + :alt: License: LGPL-3 + +======================================= +MFA and Password Security Compatibility +======================================= + +This is a glue module with extra logic needed for full compatibility between +the ``auth_totp`` and ``password_security`` modules. + +Installation +============ + +There is no need to install this module directly as this should happen +automatically if both of the parent modules are installed. + +Usage +===== + +.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas + :alt: Try me on Runbot + :target: https://runbot.odoo-community.org/runbot/149/10.0 + +Bug Tracker +=========== + +Bugs are tracked on +`GitHub 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 welcome feedback. + +Credits +======= + +Images +------ + +* Odoo Community Association: + `Icon `_. + +Contributors +------------ + +* Oleg Bulkin + +Maintainer +---------- + +.. image:: https://odoo-community.org/logo.png + :alt: Odoo Community Association + :target: https://odoo-community.org + +This module is maintained by the OCA. + +OCA, or the Odoo Community Association, is a nonprofit organization whose +mission is to support the collaborative development of Odoo features and +promote its widespread use. + +To contribute to this module, please visit https://odoo-community.org. diff --git a/auth_totp_password_security/__init__.py b/auth_totp_password_security/__init__.py new file mode 100644 index 000000000..045d7601b --- /dev/null +++ b/auth_totp_password_security/__init__.py @@ -0,0 +1,5 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from . import controllers diff --git a/auth_totp_password_security/__manifest__.py b/auth_totp_password_security/__manifest__.py new file mode 100644 index 000000000..e148a5557 --- /dev/null +++ b/auth_totp_password_security/__manifest__.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +{ + 'name': 'MFA and Password Security Compatibility', + 'summary': 'auth_totp and password_security compatibility', + 'version': '10.0.1.0.0', + 'category': 'Hidden', + 'website': 'https://github.com/OCA/server-tools', + 'author': 'LasLabs, Odoo Community Association (OCA)', + 'license': 'LGPL-3', + 'application': False, + 'installable': True, + 'auto_install': True, + 'depends': [ + 'auth_totp', + 'password_security', + ], +} diff --git a/auth_totp_password_security/controllers/__init__.py b/auth_totp_password_security/controllers/__init__.py new file mode 100644 index 000000000..c1e77cdc3 --- /dev/null +++ b/auth_totp_password_security/controllers/__init__.py @@ -0,0 +1,5 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from . import main diff --git a/auth_totp_password_security/controllers/main.py b/auth_totp_password_security/controllers/main.py new file mode 100644 index 000000000..fd685f58c --- /dev/null +++ b/auth_totp_password_security/controllers/main.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from odoo.http import redirect_with_hash, request, route +from odoo.addons.auth_totp.controllers.main import AuthTotp + + +class AuthTotpPasswordSecurity(AuthTotp): + @route() + def mfa_login_post(self, *args, **kwargs): + """Overload to check password expiration after MFA login""" + super_object = super(AuthTotpPasswordSecurity, self) + response = super_object.mfa_login_post(*args, **kwargs) + + if not request.params.get('login_success'): + return response + + user = request.env['res.users'].sudo().browse(request.uid) + if user._password_has_expired(): + user.action_expire_password() + request.session.logout(keep_db=True) + request.params['login_success'] = False + return redirect_with_hash(user.partner_id.signup_url) + + return response diff --git a/auth_totp_password_security/tests/__init__.py b/auth_totp_password_security/tests/__init__.py new file mode 100644 index 000000000..f17e20dca --- /dev/null +++ b/auth_totp_password_security/tests/__init__.py @@ -0,0 +1,5 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from . import test_main diff --git a/auth_totp_password_security/tests/test_main.py b/auth_totp_password_security/tests/test_main.py new file mode 100644 index 000000000..e982b97ac --- /dev/null +++ b/auth_totp_password_security/tests/test_main.py @@ -0,0 +1,77 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 LasLabs Inc. +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) + +from datetime import datetime +from mock import patch +from odoo.fields import Datetime +from odoo.tests.common import TransactionCase +from ..controllers.main import AuthTotpPasswordSecurity + +CONTROLLER_PATH = 'odoo.addons.auth_totp_password_security.controllers.main' +MODEL_PATH = 'odoo.addons.password_security.models.res_users.ResUsers' + + +@patch(CONTROLLER_PATH + '.AuthTotp.mfa_login_post') +class TestAuthTotpPasswordSecurity(TransactionCase): + + def setUp(self): + super(TestAuthTotpPasswordSecurity, self).setUp() + + self.test_controller = AuthTotpPasswordSecurity() + + self.test_user = self.env.ref('base.user_root') + self.test_user.company_id.password_expiration = 1 + pass_date = datetime(year=2016, month=1, day=1) + self.test_user.password_write_date = Datetime.to_string(pass_date) + + request_patcher = patch(CONTROLLER_PATH + '.request') + self.addCleanup(request_patcher.stop) + self.request_mock = request_patcher.start() + self.request_mock.params = {'login_success': True} + self.request_mock.uid = self.test_user.id + self.request_mock.env = self.env + + # Needed when tests are run with no prior requests + base_request_patcher = patch('odoo.http.request') + self.addCleanup(base_request_patcher.stop) + base_request_patcher.start() + + def test_mfa_login_post_no_mfa_login(self, super_mock): + """Should return result of super if MFA login not complete""" + test_response = 'Test Response' + super_mock.return_value = test_response + self.request_mock.params = {} + result = self.test_controller.mfa_login_post().get_data() + + self.assertEqual(result, test_response) + + def test_mfa_login_post_pass_not_expired(self, super_mock): + """Should return result of super if user's password not expired""" + test_response = 'Test Response' + super_mock.return_value = test_response + self.test_user.password_write_date = Datetime.to_string(datetime.now()) + result = self.test_controller.mfa_login_post().get_data() + + self.assertEqual(result, test_response) + + @patch(MODEL_PATH + '.action_expire_password') + def test_mfa_login_post_expired_helper(self, helper_mock, super_mock): + """Should correctly call helper if user's password is expired""" + self.test_controller.mfa_login_post() + + helper_mock.assert_called_once_with() + + def test_mfa_login_post_expired_log_out(self, super_mock): + """Should log out user and update params if password is expired""" + self.test_controller.mfa_login_post() + + self.request_mock.session.logout.assert_called_once_with(keep_db=True) + self.assertFalse(self.request_mock.params['login_success']) + + def test_mfa_login_post_expired_redirect(self, super_mock): + """Should return correct redirect if password is expired""" + result = self.test_controller.mfa_login_post().get_data() + + expected = self.test_user.partner_id.signup_url + self.assertIn(expected, result)