From 08af92d17b5fde94e2d22b473c16083e8b66cb45 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Tue, 28 Aug 2018 05:19:49 +0000 Subject: [PATCH 01/13] Add Notify MFA --- homeassistant/auth/mfa_modules/__init__.py | 9 +- .../auth/mfa_modules/insecure_example.py | 2 +- homeassistant/auth/mfa_modules/notify.py | 347 +++++++++++++++++ homeassistant/auth/mfa_modules/totp.py | 2 +- homeassistant/auth/providers/__init__.py | 11 +- .../components/auth/.translations/en.json | 19 + homeassistant/components/auth/strings.json | 19 + homeassistant/config.py | 3 +- requirements_all.txt | 1 + requirements_test_all.txt | 1 + .../auth/mfa_modules/test_insecure_example.py | 8 +- tests/auth/mfa_modules/test_notify.py | 354 ++++++++++++++++++ tests/auth/mfa_modules/test_totp.py | 6 +- 13 files changed, 768 insertions(+), 14 deletions(-) create mode 100644 homeassistant/auth/mfa_modules/notify.py create mode 100644 tests/auth/mfa_modules/test_notify.py diff --git a/homeassistant/auth/mfa_modules/__init__.py b/homeassistant/auth/mfa_modules/__init__.py index 603ca6ff3b16d7..98f31203f37f90 100644 --- a/homeassistant/auth/mfa_modules/__init__.py +++ b/homeassistant/auth/mfa_modules/__init__.py @@ -84,11 +84,18 @@ async def async_is_user_setup(self, user_id: str) -> bool: """Return whether user is setup.""" raise NotImplementedError - async def async_validation( + async def async_validate( self, user_id: str, user_input: Dict[str, Any]) -> bool: """Return True if validation passed.""" raise NotImplementedError + async def async_generate(self, user_id: str) -> Optional[str]: + """Generate init code. + + Optional + """ + return None + class SetupFlow(data_entry_flow.FlowHandler): """Handler for the setup flow.""" diff --git a/homeassistant/auth/mfa_modules/insecure_example.py b/homeassistant/auth/mfa_modules/insecure_example.py index 9c72111ef9697f..9804cbcf635883 100644 --- a/homeassistant/auth/mfa_modules/insecure_example.py +++ b/homeassistant/auth/mfa_modules/insecure_example.py @@ -77,7 +77,7 @@ async def async_is_user_setup(self, user_id: str) -> bool: return True return False - async def async_validation( + async def async_validate( self, user_id: str, user_input: Dict[str, Any]) -> bool: """Return True if validation passed.""" for data in self._data: diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py new file mode 100644 index 00000000000000..c0ff6e023830b3 --- /dev/null +++ b/homeassistant/auth/mfa_modules/notify.py @@ -0,0 +1,347 @@ +"""HMAC-based One-time Password auth module. + +Sending HOTP through notify service +""" +import logging +from collections import OrderedDict +from random import SystemRandom +from typing import Any, Dict, Optional, Tuple, List # noqa: F401 + +import voluptuous as vol + +from homeassistant.const import CONF_EXCLUDE, CONF_INCLUDE +from homeassistant.core import HomeAssistant, callback +from homeassistant.helpers import config_validation as cv + +from . import MultiFactorAuthModule, MULTI_FACTOR_AUTH_MODULES, \ + MULTI_FACTOR_AUTH_MODULE_SCHEMA, SetupFlow + +REQUIREMENTS = ['pyotp==2.2.6'] + +CONF_MESSAGE = 'message' + +CONFIG_SCHEMA = MULTI_FACTOR_AUTH_MODULE_SCHEMA.extend({ + vol.Optional(CONF_INCLUDE): vol.All(cv.ensure_list, [cv.string]), + vol.Optional(CONF_EXCLUDE): vol.All(cv.ensure_list, [cv.string]), + vol.Optional(CONF_MESSAGE, default='Your Home Assistant One-time Password' + ' is "{}"'): str +}, extra=vol.PREVENT_EXTRA) + +STORAGE_VERSION = 1 +STORAGE_KEY = 'auth_module.notify' +STORAGE_USERS = 'users' +STORAGE_USER_ID = 'user_id' +STORAGE_OTA_SECRET = 'ota_secret' +STORAGE_COUNTER = 'counter' + +INPUT_FIELD_CODE = 'code' + +DUMMY_SECRET = '7Z5EFWI4RFLVV67G' + +_LOGGER = logging.getLogger(__name__) + +_UsersDict = Dict[str, Tuple[str, int, Optional[str], Optional[str]]] + + +def _generate_secret_and_init_counter() -> Tuple[str, int]: + """Generate a secret and a random initial counter.""" + import pyotp + + ota_secret = pyotp.random_base32() + counter = SystemRandom().randint(0, 2 << 31) + return ota_secret, counter + + +def _generate_otp(secret: str, count: int) -> str: + """Generate one time password.""" + import pyotp + + return str(pyotp.HOTP(secret).at(count)) + + +def _verify_otp(secret: str, otp: str, counter: int) -> bool: + """Verify one time password.""" + import pyotp + + return bool(pyotp.HOTP(secret).verify(otp, counter)) + + +@MULTI_FACTOR_AUTH_MODULES.register('notify') +class NotifyAuthModule(MultiFactorAuthModule): + """Auth module send hmac-based one time password by notify service.""" + + DEFAULT_TITLE = 'Notify One-Time Password' + + def __init__(self, hass: HomeAssistant, config: Dict[str, Any]) -> None: + """Initialize the user data store.""" + super().__init__(hass, config) + self._users = None # type: Optional[_UsersDict] + self._user_store = hass.helpers.storage.Store( + STORAGE_VERSION, STORAGE_KEY) + self._include = config.get(CONF_INCLUDE, []) + self._exclude = config.get(CONF_EXCLUDE, []) + self._message_template = config[CONF_MESSAGE] + + @property + def input_schema(self) -> vol.Schema: + """Validate login flow input data.""" + return vol.Schema({INPUT_FIELD_CODE: str}) + + async def _async_load(self) -> None: + """Load stored data.""" + data = await self._user_store.async_load() + + if data is None: + data = {STORAGE_USERS: {}} + + self._users = data.get(STORAGE_USERS, {}) + + async def _async_save(self) -> None: + """Save data.""" + await self._user_store.async_save({STORAGE_USERS: self._users}) + + def _add_user_setup_data(self, user_id: str, + secret: Optional[str] = None, + counter: int = 0, + notify_service: Optional[str] = None, + target: Optional[str] = None) -> None: + """Create a ota_secret for user.""" + import pyotp + + ota_secret = secret or pyotp.random_base32() # type: str + init_counter = counter + + self._users[user_id] = (ota_secret, init_counter, # type: ignore + notify_service, target) + + @callback + def aync_get_aviliable_notify_services(self) -> List[str]: + """Return list of notify services.""" + unordered_services = [ + service_id for service_id in + self.hass.services.async_services().get( + 'notify', {}).keys()] + + for exclude_service in self._exclude: + if exclude_service in unordered_services: + unordered_services.remove(exclude_service) + + if self._include: + unordered_services = [s for s in self._include + if s in unordered_services] + + return sorted(unordered_services) + + async def async_setup_flow(self, user_id: str) -> SetupFlow: + """Return a data entry flow handler for setup module. + + Mfa module should extend SetupFlow + """ + return NotifySetupFlow( + self, self.input_schema, user_id, + self.aync_get_aviliable_notify_services()) + + async def async_setup_user(self, user_id: str, setup_data: Any) -> Any: + """Set up auth module for user.""" + if self._users is None: + await self._async_load() + + await self.hass.async_add_executor_job( + self._add_user_setup_data, user_id, + setup_data.get('secret'), + int(setup_data.get('counter', 0)), + setup_data.get('notify_service'), + setup_data.get('target'), + ) + + await self._async_save() + + async def async_depose_user(self, user_id: str) -> None: + """Depose auth module for user.""" + if self._users is None: + await self._async_load() + + if self._users.pop(user_id, None): # type: ignore + await self._async_save() + + async def async_is_user_setup(self, user_id: str) -> bool: + """Return whether user is setup.""" + if self._users is None: + await self._async_load() + + return user_id in self._users # type: ignore + + async def async_validate( + self, user_id: str, user_input: Dict[str, Any]) -> bool: + """Return True if validation passed.""" + if self._users is None: + await self._async_load() + + # user_input has been validate in caller + result = await self.hass.async_add_executor_job( + self._validate_one_time_password, user_id, + user_input.get(INPUT_FIELD_CODE, '')) + + # save user data no matter if passed validation to update counter + await self._async_save() + + return result + + def _validate_one_time_password(self, user_id: str, code: str) -> bool: + """Validate one time password.""" + ota_secret, counter, notify_service, target = \ + self._users.get(user_id, (None, 0, None, None)) # type: ignore + if ota_secret is None: + # even we cannot find user, we still do verify + # to make timing the same as if user was found. + _verify_otp(DUMMY_SECRET, code, 0) + return False + + result = _verify_otp(ota_secret, code, counter) + + # move counter no matter if passed validation + self._users[user_id] = (ota_secret, counter + 1, # type: ignore + notify_service, target) + return result + + async def async_generate(self, user_id: str) -> Optional[str]: + """Generate code and notify user.""" + if self._users is None: + await self._async_load() + + code = await self.hass.async_add_executor_job( + self._generate_and_send_one_time_password, user_id) + + await self.async_notify_user(user_id, code) + + # Do not return code, code has delivered by notify service + return None + + def _generate_and_send_one_time_password(self, user_id: str) -> str: + """Generate and send one time password.""" + ota_secret, counter, _, _ = \ + self._users.get(user_id, (None, 0, None, None)) # type: ignore + if ota_secret is None: + raise ValueError('Cannot find user_id') + + return _generate_otp(ota_secret, counter) + + async def async_notify_user(self, user_id: str, code: str) -> None: + """Send code by user's notify service.""" + if self._users is None: + await self._async_load() + + _, _, notify_service, target = \ + self._users.get(user_id, (None, 0, None, None)) # type: ignore + + if notify_service is None: + _LOGGER.error('Cannot find user %s', user_id) + return + + await self.async_notify(code, notify_service, target) + + async def async_notify(self, code: str, notify_service: str, + target: Optional[str] = None) -> None: + """Send code by notify service.""" + data = {'message': self._message_template.format(code)} + if target: + data['target'] = [target] + + await self.hass.services.async_call('notify', notify_service, data) + + +class NotifySetupFlow(SetupFlow): + """Handler for the setup flow.""" + + def __init__(self, auth_module: NotifyAuthModule, + setup_schema: vol.Schema, + user_id: str, + available_notify_services: List[str]) -> None: + """Initialize the setup flow.""" + super().__init__(auth_module, setup_schema, user_id) + # to fix typing complaint + self._auth_module = auth_module # type: NotifyAuthModule + self._available_notify_services = available_notify_services + self._ota_secret = None # type: Optional[str] + self._counter = None # type Optional[int] + self._notify_service = None # type: Optional[str] + self._target = None # type: Optional[str] + + async def async_step_init( + self, user_input: Optional[Dict[str, str]] = None) \ + -> Dict[str, Any]: + """Handle the first step of setup flow. + + Return self.async_show_form(step_id='init') if user_input == None. + Return self.async_create_entry(data={'result': result}) if finish. + """ + errors = {} # type: Dict[str, str] + + hass = self._auth_module.hass + if user_input: + self._notify_service = user_input['notify_service'] + self._target = user_input.get('target') + + return await self.async_step_setup() + + if not self._available_notify_services: + return self.async_abort(reason='no_available_service') + + self._ota_secret, self._counter = \ + await hass.async_add_executor_job( # type: ignore + _generate_secret_and_init_counter) + + schema = OrderedDict() # type: Dict[str, Any] + schema['notify_service'] = vol.In(self._available_notify_services) + schema['target'] = vol.Optional(str) + + return self.async_show_form( + step_id='init', + data_schema=vol.Schema(schema), + errors=errors + ) + + async def async_step_setup( + self, user_input: Optional[Dict[str, str]] = None) \ + -> Dict[str, Any]: + """Handle the setup step of setup flow. + + Return self.async_show_form(step_id='init') if user_input == None. + Return self.async_create_entry(data={'result': result}) if finish. + """ + import pyotp + + errors = {} # type: Dict[str, str] + + if user_input: + hass = self._auth_module.hass + verified = await hass.async_add_executor_job( + pyotp.HOTP(self._ota_secret).verify, + user_input['code'], self._counter) + self._counter += 1 # type: ignore + if verified: + result = await self._auth_module.async_setup_user( + self._user_id, { + 'secret': self._ota_secret, + 'counter': self._counter, # counter has increased + 'notify_service': self._notify_service, + 'target': self._target, + }) + return self.async_create_entry( + title=self._auth_module.name, + data={'result': result} + ) + + errors['base'] = 'invalid_code' + + code = _generate_otp(self._ota_secret, self._counter) # type: ignore + + await self._auth_module.async_notify( # type: ignore + code, self._notify_service, self._target) + + return self.async_show_form( + step_id='setup', + data_schema=self._setup_schema, + description_placeholders={'notify_service': self._notify_service}, + errors=errors + ) diff --git a/homeassistant/auth/mfa_modules/totp.py b/homeassistant/auth/mfa_modules/totp.py index 50cd9d334660b0..b00bd24cc622e3 100644 --- a/homeassistant/auth/mfa_modules/totp.py +++ b/homeassistant/auth/mfa_modules/totp.py @@ -130,7 +130,7 @@ async def async_is_user_setup(self, user_id: str) -> bool: return user_id in self._users # type: ignore - async def async_validation( + async def async_validate( self, user_id: str, user_input: Dict[str, Any]) -> bool: """Return True if validation passed.""" if self._users is None: diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index 3cb1c6b121e4cc..cf17ab50d967c6 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/homeassistant/auth/providers/__init__.py @@ -228,7 +228,7 @@ async def async_step_mfa( reason='login_expired' ) - result = await auth_module.async_validation( + result = await auth_module.async_validate( self.user.id, user_input) # type: ignore if not result: errors['base'] = 'invalid_code' @@ -236,10 +236,15 @@ async def async_step_mfa( if not errors: return await self.async_finish(self.user) + # MFA module may have init code need generate + mfa_init_code = await auth_module.async_generate( + self.user.id) # type: ignore + description_placeholders = { 'mfa_module_name': auth_module.name, - 'mfa_module_id': auth_module.id - } # type: Dict[str, str] + 'mfa_module_id': auth_module.id, + 'mfa_init_code': mfa_init_code, + } # type: Dict[str, Optional[str]] return self.async_show_form( step_id='mfa', diff --git a/homeassistant/components/auth/.translations/en.json b/homeassistant/components/auth/.translations/en.json index a0fd20e9d083b1..21cb45e3050a22 100644 --- a/homeassistant/components/auth/.translations/en.json +++ b/homeassistant/components/auth/.translations/en.json @@ -1,5 +1,24 @@ { "mfa_setup": { + "notify": { + "abort": { + "no_available_service": "No available notify services." + }, + "error": { + "invalid_code": "Invalid code, please try again." + }, + "step": { + "init": { + "description": "Please select one of notify service:", + "title": "Set up one-time password delivered by notify component" + }, + "setup": { + "description": "A one-time password have sent by **notify.{notify_service}**. Please input it in below:", + "title": "Verify setup" + } + }, + "title": "Notify One-Time Password" + }, "totp": { "error": { "invalid_code": "Invalid code, please try again. If you get this error consistently, please make sure the clock of your Home Assistant system is accurate." diff --git a/homeassistant/components/auth/strings.json b/homeassistant/components/auth/strings.json index b0083ab577b4c0..2b1fc0c94f6f74 100644 --- a/homeassistant/components/auth/strings.json +++ b/homeassistant/components/auth/strings.json @@ -11,6 +11,25 @@ "error": { "invalid_code": "Invalid code, please try again. If you get this error consistently, please make sure the clock of your Home Assistant system is accurate." } + }, + "notify": { + "title": "Notify One-Time Password", + "step": { + "init": { + "title": "Set up one-time password delivered by notify component", + "description": "Please select one of notify service:" + }, + "setup": { + "title": "Verify setup", + "description": "A one-time password have sent by **notify.{notify_service}**. Please input it in below:" + } + }, + "abort": { + "no_available_service": "No available notify services." + }, + "error": { + "invalid_code": "Invalid code, please try again." + } } } } diff --git a/homeassistant/config.py b/homeassistant/config.py index 5474b283494e39..617aaedd19f98e 100644 --- a/homeassistant/config.py +++ b/homeassistant/config.py @@ -475,7 +475,8 @@ async def async_process_ha_core_config( auth_conf.append({'type': 'trusted_networks'}) mfa_conf = config.get(CONF_AUTH_MFA_MODULES, [ - {'type': 'totp', 'id': 'totp', 'name': 'Authenticator app'} + {'type': 'totp', 'id': 'totp', 'name': 'Authenticator app'}, + {'type': 'notify', 'id': 'notify'} ]) setattr(hass, 'auth', await auth.auth_manager_from_config( diff --git a/requirements_all.txt b/requirements_all.txt index bb41c1a9d29bb1..7533fbd14e388d 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1017,6 +1017,7 @@ pyota==2.0.5 # homeassistant.components.climate.opentherm_gw pyotgw==0.1b0 +# homeassistant.auth.mfa_modules.notify # homeassistant.auth.mfa_modules.totp # homeassistant.components.sensor.otp pyotp==2.2.6 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index dc263fd4e9c726..faba75414e4db4 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -160,6 +160,7 @@ pynx584==0.4 # homeassistant.components.openuv pyopenuv==1.0.4 +# homeassistant.auth.mfa_modules.notify # homeassistant.auth.mfa_modules.totp # homeassistant.components.sensor.otp pyotp==2.2.6 diff --git a/tests/auth/mfa_modules/test_insecure_example.py b/tests/auth/mfa_modules/test_insecure_example.py index 80109627140d58..d9330d5f6e8c7a 100644 --- a/tests/auth/mfa_modules/test_insecure_example.py +++ b/tests/auth/mfa_modules/test_insecure_example.py @@ -12,15 +12,15 @@ async def test_validate(hass): 'data': [{'user_id': 'test-user', 'pin': '123456'}] }) - result = await auth_module.async_validation( + result = await auth_module.async_validate( 'test-user', {'pin': '123456'}) assert result is True - result = await auth_module.async_validation( + result = await auth_module.async_validate( 'test-user', {'pin': 'invalid'}) assert result is False - result = await auth_module.async_validation( + result = await auth_module.async_validate( 'invalid-user', {'pin': '123456'}) assert result is False @@ -36,7 +36,7 @@ async def test_setup_user(hass): 'test-user', {'pin': '123456'}) assert len(auth_module._data) == 1 - result = await auth_module.async_validation( + result = await auth_module.async_validate( 'test-user', {'pin': '123456'}) assert result is True diff --git a/tests/auth/mfa_modules/test_notify.py b/tests/auth/mfa_modules/test_notify.py new file mode 100644 index 00000000000000..d9c7e645daa052 --- /dev/null +++ b/tests/auth/mfa_modules/test_notify.py @@ -0,0 +1,354 @@ +"""Test the HMAC-based One Time Password (MFA) auth module.""" +from unittest.mock import patch + +from homeassistant import data_entry_flow +from homeassistant.auth import models as auth_models, auth_manager_from_config +from homeassistant.auth.mfa_modules import auth_mfa_module_from_config +from homeassistant.components.notify import NOTIFY_SERVICE_SCHEMA +from tests.common import MockUser, async_mock_service + +MOCK_CODE = '123456' +MOCK_CODE_2 = '654321' + + +async def test_validating_mfa(hass): + """Test validating mfa code.""" + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify' + }) + await notify_auth_module.async_setup_user('test-user', { + 'notify_service': 'dummy' + }) + + with patch('pyotp.HOTP.verify', return_value=True): + assert await notify_auth_module.async_validate( + 'test-user', {'code': MOCK_CODE}) + + +async def test_validating_mfa_invalid_code(hass): + """Test validating an invalid mfa code.""" + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify' + }) + await notify_auth_module.async_setup_user('test-user', { + 'notify_service': 'dummy' + }) + + with patch('pyotp.HOTP.verify', return_value=False): + assert await notify_auth_module.async_validate( + 'test-user', {'code': MOCK_CODE}) is False + + +async def test_validating_mfa_invalid_user(hass): + """Test validating an mfa code with invalid user.""" + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify' + }) + await notify_auth_module.async_setup_user('test-user', { + 'notify_service': 'dummy' + }) + + assert await notify_auth_module.async_validate( + 'invalid-user', {'code': MOCK_CODE}) is False + + +async def test_validating_mfa_counter(hass): + """Test counter will move no matter code validation result.""" + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify' + }) + await notify_auth_module.async_setup_user('test-user', { + 'counter': 0, + 'notify_service': 'dummy', + }) + + assert notify_auth_module._users + user = list(notify_auth_module._users.values())[0] + init_count = user[1] + assert init_count is not None + + with patch('pyotp.HOTP.verify', return_value=True): + assert await notify_auth_module.async_validate( + 'test-user', {'code': MOCK_CODE}) + + user = list(notify_auth_module._users.values())[0] + after_pass_count = user[1] + assert after_pass_count != init_count + + with patch('pyotp.HOTP.verify', return_value=False): + assert await notify_auth_module.async_validate( + 'test-user', {'code': MOCK_CODE}) is False + + user = list(notify_auth_module._users.values())[0] + after_fail_count = user[1] + assert after_fail_count != init_count + assert after_fail_count != after_pass_count + + +async def test_setup_depose_user(hass): + """Test set up and despose user.""" + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify' + }) + await notify_auth_module.async_setup_user('test-user', {}) + assert len(notify_auth_module._users) == 1 + await notify_auth_module.async_setup_user('test-user', {}) + assert len(notify_auth_module._users) == 1 + + await notify_auth_module.async_depose_user('test-user') + assert len(notify_auth_module._users) == 0 + + await notify_auth_module.async_setup_user( + 'test-user2', {'secret': 'secret-code'}) + assert len(notify_auth_module._users) == 1 + + +async def test_login_flow_validates_mfa(hass): + """Test login flow with mfa enabled.""" + hass.auth = await auth_manager_from_config(hass, [{ + 'type': 'insecure_example', + 'users': [{'username': 'test-user', 'password': 'test-pass'}], + }], [{ + 'type': 'notify', + }]) + user = MockUser( + id='mock-user', + is_owner=False, + is_active=False, + name='Paulus', + ).add_to_auth_manager(hass.auth) + await hass.auth.async_link_user(user, auth_models.Credentials( + id='mock-id', + auth_provider_type='insecure_example', + auth_provider_id=None, + data={'username': 'test-user'}, + is_new=False, + )) + + notify_calls = async_mock_service(hass, 'notify', 'test-notify', + NOTIFY_SERVICE_SCHEMA) + + await hass.auth.async_enable_user_mfa(user, 'notify', { + 'notify_service': 'test-notify', + }) + + provider = hass.auth.auth_providers[0] + + result = await hass.auth.login_flow.async_init( + (provider.type, provider.id)) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + + result = await hass.auth.login_flow.async_configure(result['flow_id'], { + 'username': 'incorrect-user', + 'password': 'test-pass', + }) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['errors']['base'] == 'invalid_auth' + + result = await hass.auth.login_flow.async_configure(result['flow_id'], { + 'username': 'test-user', + 'password': 'incorrect-pass', + }) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['errors']['base'] == 'invalid_auth' + + with patch('pyotp.HOTP.at', return_value=MOCK_CODE): + result = await hass.auth.login_flow.async_configure( + result['flow_id'], + { + 'username': 'test-user', + 'password': 'test-pass', + }) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'mfa' + assert result['data_schema'].schema.get('code') == str + + # wait service call finished + await hass.async_block_till_done() + + assert len(notify_calls) == 1 + notify_call = notify_calls[0] + assert notify_call.domain == 'notify' + assert notify_call.service == 'test-notify' + message = notify_call.data['message'] + message.hass = hass + assert MOCK_CODE in message.async_render() + + with patch('pyotp.HOTP.verify', return_value=False), \ + patch('pyotp.HOTP.at', return_value=MOCK_CODE_2): + result = await hass.auth.login_flow.async_configure( + result['flow_id'], {'code': 'invalid-code'}) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'mfa' + assert result['errors']['base'] == 'invalid_code' + + # wait service call finished + await hass.async_block_till_done() + + assert len(notify_calls) == 2 + notify_call = notify_calls[1] + assert notify_call.domain == 'notify' + assert notify_call.service == 'test-notify' + message = notify_call.data['message'] + message.hass = hass + assert MOCK_CODE_2 in message.async_render() + + with patch('pyotp.HOTP.verify', return_value=True): + result = await hass.auth.login_flow.async_configure( + result['flow_id'], {'code': MOCK_CODE}) + assert result['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result['data'].id == 'mock-user' + + +async def test_setup_user_notify_service(hass): + """Test allow select notify service during mfa setup.""" + notify_calls = async_mock_service( + hass, 'notify', 'test1', NOTIFY_SERVICE_SCHEMA) + async_mock_service(hass, 'notify', 'test2', NOTIFY_SERVICE_SCHEMA) + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify', + }) + + services = notify_auth_module.aync_get_aviliable_notify_services() + assert services == ['test1', 'test2'] + + flow = await notify_auth_module.async_setup_flow('test-user') + step = await flow.async_step_init() + assert step['type'] == data_entry_flow.RESULT_TYPE_FORM + assert step['step_id'] == 'init' + schema = step['data_schema'] + schema({'notify_service': 'test2'}) + + with patch('pyotp.HOTP.at', return_value=MOCK_CODE): + step = await flow.async_step_init({'notify_service': 'test1'}) + assert step['type'] == data_entry_flow.RESULT_TYPE_FORM + assert step['step_id'] == 'setup' + + # wait service call finished + await hass.async_block_till_done() + + assert len(notify_calls) == 1 + notify_call = notify_calls[0] + assert notify_call.domain == 'notify' + assert notify_call.service == 'test1' + message = notify_call.data['message'] + message.hass = hass + assert MOCK_CODE in message.async_render() + + with patch('pyotp.HOTP.at', return_value=MOCK_CODE_2): + step = await flow.async_step_setup({'code': 'invalid'}) + assert step['type'] == data_entry_flow.RESULT_TYPE_FORM + assert step['step_id'] == 'setup' + assert step['errors']['base'] == 'invalid_code' + + # wait service call finished + await hass.async_block_till_done() + + assert len(notify_calls) == 2 + notify_call = notify_calls[1] + assert notify_call.domain == 'notify' + assert notify_call.service == 'test1' + message = notify_call.data['message'] + message.hass = hass + assert MOCK_CODE_2 in message.async_render() + + with patch('pyotp.HOTP.verify', return_value=True): + step = await flow.async_step_setup({'code': MOCK_CODE_2}) + assert step['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + + +async def test_include_exclude_config(hass): + """Test allow include exclude config.""" + async_mock_service(hass, 'notify', 'include1', NOTIFY_SERVICE_SCHEMA) + async_mock_service(hass, 'notify', 'include2', NOTIFY_SERVICE_SCHEMA) + async_mock_service(hass, 'notify', 'exclude1', NOTIFY_SERVICE_SCHEMA) + async_mock_service(hass, 'notify', 'exclude2', NOTIFY_SERVICE_SCHEMA) + async_mock_service(hass, 'other', 'include3', NOTIFY_SERVICE_SCHEMA) + async_mock_service(hass, 'other', 'exclude3', NOTIFY_SERVICE_SCHEMA) + + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify', + 'exclude': ['exclude1', 'exclude2', 'exclude3'], + }) + services = notify_auth_module.aync_get_aviliable_notify_services() + assert services == ['include1', 'include2'] + + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify', + 'include': ['include1', 'include2', 'include3'], + }) + services = notify_auth_module.aync_get_aviliable_notify_services() + assert services == ['include1', 'include2'] + + # exclude has high priority than include + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify', + 'include': ['include1', 'include2', 'include3'], + 'exclude': ['exclude1', 'exclude2', 'include2'], + }) + services = notify_auth_module.aync_get_aviliable_notify_services() + assert services == ['include1'] + + +async def test_setup_user_no_notify_service(hass): + """Test setup flow abort if there is no avilable notify service.""" + async_mock_service(hass, 'notify', 'test1', NOTIFY_SERVICE_SCHEMA) + notify_auth_module = await auth_mfa_module_from_config(hass, { + 'type': 'notify', + 'exclude': 'test1', + }) + + services = notify_auth_module.aync_get_aviliable_notify_services() + assert services == [] + + flow = await notify_auth_module.async_setup_flow('test-user') + step = await flow.async_step_init() + assert step['type'] == data_entry_flow.RESULT_TYPE_ABORT + assert step['reason'] == 'no_available_service' + + +async def test_not_raise_exception_when_service_not_exist(hass): + """Test login flow will not raise exception when notify service error.""" + hass.auth = await auth_manager_from_config(hass, [{ + 'type': 'insecure_example', + 'users': [{'username': 'test-user', 'password': 'test-pass'}], + }], [{ + 'type': 'notify', + }]) + user = MockUser( + id='mock-user', + is_owner=False, + is_active=False, + name='Paulus', + ).add_to_auth_manager(hass.auth) + await hass.auth.async_link_user(user, auth_models.Credentials( + id='mock-id', + auth_provider_type='insecure_example', + auth_provider_id=None, + data={'username': 'test-user'}, + is_new=False, + )) + + await hass.auth.async_enable_user_mfa(user, 'notify', { + 'notify_service': 'invalid-notify', + }) + + provider = hass.auth.auth_providers[0] + + result = await hass.auth.login_flow.async_init( + (provider.type, provider.id)) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + + with patch('pyotp.HOTP.at', return_value=MOCK_CODE): + result = await hass.auth.login_flow.async_configure( + result['flow_id'], + { + 'username': 'test-user', + 'password': 'test-pass', + }) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'mfa' + assert result['data_schema'].schema.get('code') == str + + # wait service call finished + await hass.async_block_till_done() diff --git a/tests/auth/mfa_modules/test_totp.py b/tests/auth/mfa_modules/test_totp.py index 6e3558ec5496e3..d400fe80672d89 100644 --- a/tests/auth/mfa_modules/test_totp.py +++ b/tests/auth/mfa_modules/test_totp.py @@ -17,7 +17,7 @@ async def test_validating_mfa(hass): await totp_auth_module.async_setup_user('test-user', {}) with patch('pyotp.TOTP.verify', return_value=True): - assert await totp_auth_module.async_validation( + assert await totp_auth_module.async_validate( 'test-user', {'code': MOCK_CODE}) @@ -29,7 +29,7 @@ async def test_validating_mfa_invalid_code(hass): await totp_auth_module.async_setup_user('test-user', {}) with patch('pyotp.TOTP.verify', return_value=False): - assert await totp_auth_module.async_validation( + assert await totp_auth_module.async_validate( 'test-user', {'code': MOCK_CODE}) is False @@ -40,7 +40,7 @@ async def test_validating_mfa_invalid_user(hass): }) await totp_auth_module.async_setup_user('test-user', {}) - assert await totp_auth_module.async_validation( + assert await totp_auth_module.async_validate( 'invalid-user', {'code': MOCK_CODE}) is False From 516671e533bfdc49bab2ee0de52c93c59e284f09 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Sat, 1 Sep 2018 07:33:37 -0700 Subject: [PATCH 02/13] Fix unit test --- tests/test_config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index e4a6798093ffbb..8076fcc6b63c75 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -841,8 +841,9 @@ async def test_auth_provider_config_default(hass): assert len(hass.auth.auth_providers) == 1 assert hass.auth.auth_providers[0].type == 'homeassistant' assert hass.auth.active is True - assert len(hass.auth.auth_mfa_modules) == 1 + assert len(hass.auth.auth_mfa_modules) == 2 assert hass.auth.auth_mfa_modules[0].id == 'totp' + assert hass.auth.auth_mfa_modules[1].id == 'notify' async def test_auth_provider_config_default_api_password(hass): From 6c5536b3ca2530a7e1c17ace4b3ca14bc8d3f7ac Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 02:31:07 +0000 Subject: [PATCH 03/13] Address review comment, change storage implementation --- homeassistant/auth/mfa_modules/__init__.py | 7 -- homeassistant/auth/mfa_modules/notify.py | 83 +++++++++++++--------- homeassistant/auth/providers/__init__.py | 5 +- homeassistant/config.py | 1 - tests/auth/mfa_modules/test_notify.py | 32 ++++----- tests/test_config.py | 3 +- 6 files changed, 67 insertions(+), 64 deletions(-) diff --git a/homeassistant/auth/mfa_modules/__init__.py b/homeassistant/auth/mfa_modules/__init__.py index 98f31203f37f90..4ed83efc040a45 100644 --- a/homeassistant/auth/mfa_modules/__init__.py +++ b/homeassistant/auth/mfa_modules/__init__.py @@ -89,13 +89,6 @@ async def async_validate( """Return True if validation passed.""" raise NotImplementedError - async def async_generate(self, user_id: str) -> Optional[str]: - """Generate init code. - - Optional - """ - return None - class SetupFlow(data_entry_flow.FlowHandler): """Handler for the setup flow.""" diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index c0ff6e023830b3..1b69271af5a514 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -7,6 +7,7 @@ from random import SystemRandom from typing import Any, Dict, Optional, Tuple, List # noqa: F401 +import attr import voluptuous as vol from homeassistant.const import CONF_EXCLUDE, CONF_INCLUDE @@ -40,7 +41,18 @@ _LOGGER = logging.getLogger(__name__) -_UsersDict = Dict[str, Tuple[str, int, Optional[str], Optional[str]]] + +@attr.s(slots=True) +class NotifySetting: + """Store notify setting for one user.""" + + secret = attr.ib(type=str) + counter = attr.ib(type=int) + notify_service = attr.ib(type=Optional[str], default=None) + target = attr.ib(type=Optional[str], default=None) + + +_UsersDict = Dict[str, NotifySetting] def _generate_secret_and_init_counter() -> Tuple[str, int]: @@ -75,7 +87,7 @@ class NotifyAuthModule(MultiFactorAuthModule): def __init__(self, hass: HomeAssistant, config: Dict[str, Any]) -> None: """Initialize the user data store.""" super().__init__(hass, config) - self._users = None # type: Optional[_UsersDict] + self._user_settings = None # type: Optional[_UsersDict] self._user_store = hass.helpers.storage.Store( STORAGE_VERSION, STORAGE_KEY) self._include = config.get(CONF_INCLUDE, []) @@ -94,11 +106,14 @@ async def _async_load(self) -> None: if data is None: data = {STORAGE_USERS: {}} - self._users = data.get(STORAGE_USERS, {}) + self._user_settings = data.get(STORAGE_USERS, {}) async def _async_save(self) -> None: """Save data.""" - await self._user_store.async_save({STORAGE_USERS: self._users}) + await self._user_store.async_save({STORAGE_USERS: { + user_id: attr.asdict(notify_setting) + for user_id, notify_setting in self._user_settings.items() + }}) def _add_user_setup_data(self, user_id: str, secret: Optional[str] = None, @@ -111,11 +126,15 @@ def _add_user_setup_data(self, user_id: str, ota_secret = secret or pyotp.random_base32() # type: str init_counter = counter - self._users[user_id] = (ota_secret, init_counter, # type: ignore - notify_service, target) + self._user_settings[user_id] = NotifySetting( + secret=ota_secret, + counter=init_counter, + notify_service=notify_service, + target=target, + ) @callback - def aync_get_aviliable_notify_services(self) -> List[str]: + def aync_get_available_notify_services(self) -> List[str]: """Return list of notify services.""" unordered_services = [ service_id for service_id in @@ -139,11 +158,11 @@ async def async_setup_flow(self, user_id: str) -> SetupFlow: """ return NotifySetupFlow( self, self.input_schema, user_id, - self.aync_get_aviliable_notify_services()) + self.aync_get_available_notify_services()) async def async_setup_user(self, user_id: str, setup_data: Any) -> Any: """Set up auth module for user.""" - if self._users is None: + if self._user_settings is None: await self._async_load() await self.hass.async_add_executor_job( @@ -158,23 +177,23 @@ async def async_setup_user(self, user_id: str, setup_data: Any) -> Any: async def async_depose_user(self, user_id: str) -> None: """Depose auth module for user.""" - if self._users is None: + if self._user_settings is None: await self._async_load() - if self._users.pop(user_id, None): # type: ignore + if self._user_settings.pop(user_id, None): # type: ignore await self._async_save() async def async_is_user_setup(self, user_id: str) -> bool: """Return whether user is setup.""" - if self._users is None: + if self._user_settings is None: await self._async_load() - return user_id in self._users # type: ignore + return user_id in self._user_settings # type: ignore async def async_validate( self, user_id: str, user_input: Dict[str, Any]) -> bool: """Return True if validation passed.""" - if self._users is None: + if self._user_settings is None: await self._async_load() # user_input has been validate in caller @@ -189,24 +208,23 @@ async def async_validate( def _validate_one_time_password(self, user_id: str, code: str) -> bool: """Validate one time password.""" - ota_secret, counter, notify_service, target = \ - self._users.get(user_id, (None, 0, None, None)) # type: ignore - if ota_secret is None: + notify_setting = self._user_settings.get(user_id, None) + if notify_setting is None: # even we cannot find user, we still do verify # to make timing the same as if user was found. _verify_otp(DUMMY_SECRET, code, 0) return False - result = _verify_otp(ota_secret, code, counter) + result = _verify_otp( + notify_setting.secret, code, notify_setting.counter) # move counter no matter if passed validation - self._users[user_id] = (ota_secret, counter + 1, # type: ignore - notify_service, target) + notify_setting.counter += 1 return result - async def async_generate(self, user_id: str) -> Optional[str]: + async def async_generate(self, user_id: str) -> None: """Generate code and notify user.""" - if self._users is None: + if self._user_settings is None: await self._async_load() code = await self.hass.async_add_executor_job( @@ -214,31 +232,26 @@ async def async_generate(self, user_id: str) -> Optional[str]: await self.async_notify_user(user_id, code) - # Do not return code, code has delivered by notify service - return None - def _generate_and_send_one_time_password(self, user_id: str) -> str: """Generate and send one time password.""" - ota_secret, counter, _, _ = \ - self._users.get(user_id, (None, 0, None, None)) # type: ignore - if ota_secret is None: + notify_setting = self._user_settings.get(user_id, None) + if notify_setting is None: raise ValueError('Cannot find user_id') - return _generate_otp(ota_secret, counter) + return _generate_otp(notify_setting.secret, notify_setting.counter) async def async_notify_user(self, user_id: str, code: str) -> None: """Send code by user's notify service.""" - if self._users is None: + if self._user_settings is None: await self._async_load() - _, _, notify_service, target = \ - self._users.get(user_id, (None, 0, None, None)) # type: ignore - - if notify_service is None: + notify_setting = self._user_settings.get(user_id, None) + if notify_setting is None: _LOGGER.error('Cannot find user %s', user_id) return - await self.async_notify(code, notify_service, target) + await self.async_notify( + code, notify_setting.notify_service, notify_setting.target) async def async_notify(self, code: str, notify_service: str, target: Optional[str] = None) -> None: diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index cf17ab50d967c6..e0888851d4677a 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/homeassistant/auth/providers/__init__.py @@ -237,13 +237,12 @@ async def async_step_mfa( return await self.async_finish(self.user) # MFA module may have init code need generate - mfa_init_code = await auth_module.async_generate( - self.user.id) # type: ignore + if hasattr(auth_module, 'async_generate'): + await auth_module.async_generate(self.user.id) # type: ignore description_placeholders = { 'mfa_module_name': auth_module.name, 'mfa_module_id': auth_module.id, - 'mfa_init_code': mfa_init_code, } # type: Dict[str, Optional[str]] return self.async_show_form( diff --git a/homeassistant/config.py b/homeassistant/config.py index 617aaedd19f98e..d3b666934397a4 100644 --- a/homeassistant/config.py +++ b/homeassistant/config.py @@ -476,7 +476,6 @@ async def async_process_ha_core_config( mfa_conf = config.get(CONF_AUTH_MFA_MODULES, [ {'type': 'totp', 'id': 'totp', 'name': 'Authenticator app'}, - {'type': 'notify', 'id': 'notify'} ]) setattr(hass, 'auth', await auth.auth_manager_from_config( diff --git a/tests/auth/mfa_modules/test_notify.py b/tests/auth/mfa_modules/test_notify.py index d9c7e645daa052..3b275ef5ee35da 100644 --- a/tests/auth/mfa_modules/test_notify.py +++ b/tests/auth/mfa_modules/test_notify.py @@ -62,25 +62,25 @@ async def test_validating_mfa_counter(hass): 'notify_service': 'dummy', }) - assert notify_auth_module._users - user = list(notify_auth_module._users.values())[0] - init_count = user[1] + assert notify_auth_module._user_settings + notify_setting = list(notify_auth_module._user_settings.values())[0] + init_count = notify_setting.counter assert init_count is not None with patch('pyotp.HOTP.verify', return_value=True): assert await notify_auth_module.async_validate( 'test-user', {'code': MOCK_CODE}) - user = list(notify_auth_module._users.values())[0] - after_pass_count = user[1] + notify_setting = list(notify_auth_module._user_settings.values())[0] + after_pass_count = notify_setting.counter assert after_pass_count != init_count with patch('pyotp.HOTP.verify', return_value=False): assert await notify_auth_module.async_validate( 'test-user', {'code': MOCK_CODE}) is False - user = list(notify_auth_module._users.values())[0] - after_fail_count = user[1] + notify_setting = list(notify_auth_module._user_settings.values())[0] + after_fail_count = notify_setting.counter assert after_fail_count != init_count assert after_fail_count != after_pass_count @@ -91,16 +91,16 @@ async def test_setup_depose_user(hass): 'type': 'notify' }) await notify_auth_module.async_setup_user('test-user', {}) - assert len(notify_auth_module._users) == 1 + assert len(notify_auth_module._user_settings) == 1 await notify_auth_module.async_setup_user('test-user', {}) - assert len(notify_auth_module._users) == 1 + assert len(notify_auth_module._user_settings) == 1 await notify_auth_module.async_depose_user('test-user') - assert len(notify_auth_module._users) == 0 + assert len(notify_auth_module._user_settings) == 0 await notify_auth_module.async_setup_user( 'test-user2', {'secret': 'secret-code'}) - assert len(notify_auth_module._users) == 1 + assert len(notify_auth_module._user_settings) == 1 async def test_login_flow_validates_mfa(hass): @@ -209,7 +209,7 @@ async def test_setup_user_notify_service(hass): 'type': 'notify', }) - services = notify_auth_module.aync_get_aviliable_notify_services() + services = notify_auth_module.aync_get_available_notify_services() assert services == ['test1', 'test2'] flow = await notify_auth_module.async_setup_flow('test-user') @@ -270,14 +270,14 @@ async def test_include_exclude_config(hass): 'type': 'notify', 'exclude': ['exclude1', 'exclude2', 'exclude3'], }) - services = notify_auth_module.aync_get_aviliable_notify_services() + services = notify_auth_module.aync_get_available_notify_services() assert services == ['include1', 'include2'] notify_auth_module = await auth_mfa_module_from_config(hass, { 'type': 'notify', 'include': ['include1', 'include2', 'include3'], }) - services = notify_auth_module.aync_get_aviliable_notify_services() + services = notify_auth_module.aync_get_available_notify_services() assert services == ['include1', 'include2'] # exclude has high priority than include @@ -286,7 +286,7 @@ async def test_include_exclude_config(hass): 'include': ['include1', 'include2', 'include3'], 'exclude': ['exclude1', 'exclude2', 'include2'], }) - services = notify_auth_module.aync_get_aviliable_notify_services() + services = notify_auth_module.aync_get_available_notify_services() assert services == ['include1'] @@ -298,7 +298,7 @@ async def test_setup_user_no_notify_service(hass): 'exclude': 'test1', }) - services = notify_auth_module.aync_get_aviliable_notify_services() + services = notify_auth_module.aync_get_available_notify_services() assert services == [] flow = await notify_auth_module.async_setup_flow('test-user') diff --git a/tests/test_config.py b/tests/test_config.py index 8076fcc6b63c75..e4a6798093ffbb 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -841,9 +841,8 @@ async def test_auth_provider_config_default(hass): assert len(hass.auth.auth_providers) == 1 assert hass.auth.auth_providers[0].type == 'homeassistant' assert hass.auth.active is True - assert len(hass.auth.auth_mfa_modules) == 2 + assert len(hass.auth.auth_mfa_modules) == 1 assert hass.auth.auth_mfa_modules[0].id == 'totp' - assert hass.auth.auth_mfa_modules[1].id == 'notify' async def test_auth_provider_config_default_api_password(hass): From 9984fa548de02a977f68f506548eeb46ca47520a Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 03:43:08 +0000 Subject: [PATCH 04/13] Add retry limit to mfa module --- homeassistant/auth/mfa_modules/__init__.py | 1 + homeassistant/auth/mfa_modules/notify.py | 17 +++---- homeassistant/auth/mfa_modules/totp.py | 1 + homeassistant/auth/providers/__init__.py | 10 +++- tests/auth/mfa_modules/test_notify.py | 57 +++++++++++++++++++--- 5 files changed, 68 insertions(+), 18 deletions(-) diff --git a/homeassistant/auth/mfa_modules/__init__.py b/homeassistant/auth/mfa_modules/__init__.py index 4ed83efc040a45..5401f08584cdf8 100644 --- a/homeassistant/auth/mfa_modules/__init__.py +++ b/homeassistant/auth/mfa_modules/__init__.py @@ -34,6 +34,7 @@ class MultiFactorAuthModule: """Multi-factor Auth Module of validation function.""" DEFAULT_TITLE = 'Unnamed auth module' + MAX_RETRY_TIME = 3 def __init__(self, hass: HomeAssistant, config: Dict[str, Any]) -> None: """Initialize an auth module.""" diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index 1b69271af5a514..6d48d2ccdfc66c 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -201,9 +201,6 @@ async def async_validate( self._validate_one_time_password, user_id, user_input.get(INPUT_FIELD_CODE, '')) - # save user data no matter if passed validation to update counter - await self._async_save() - return result def _validate_one_time_password(self, user_id: str, code: str) -> bool: @@ -215,13 +212,9 @@ def _validate_one_time_password(self, user_id: str, code: str) -> bool: _verify_otp(DUMMY_SECRET, code, 0) return False - result = _verify_otp( + return _verify_otp( notify_setting.secret, code, notify_setting.counter) - # move counter no matter if passed validation - notify_setting.counter += 1 - return result - async def async_generate(self, user_id: str) -> None: """Generate code and notify user.""" if self._user_settings is None: @@ -230,6 +223,9 @@ async def async_generate(self, user_id: str) -> None: code = await self.hass.async_add_executor_job( self._generate_and_send_one_time_password, user_id) + # update counter in storage + await self._async_save() + await self.async_notify_user(user_id, code) def _generate_and_send_one_time_password(self, user_id: str) -> str: @@ -238,6 +234,8 @@ def _generate_and_send_one_time_password(self, user_id: str) -> str: if notify_setting is None: raise ValueError('Cannot find user_id') + # always move counter before generate new code + notify_setting.counter += 1 return _generate_otp(notify_setting.secret, notify_setting.counter) async def async_notify_user(self, user_id: str, code: str) -> None: @@ -331,12 +329,11 @@ async def async_step_setup( verified = await hass.async_add_executor_job( pyotp.HOTP(self._ota_secret).verify, user_input['code'], self._counter) - self._counter += 1 # type: ignore if verified: result = await self._auth_module.async_setup_user( self._user_id, { 'secret': self._ota_secret, - 'counter': self._counter, # counter has increased + 'counter': self._counter + 1, # increase counter 'notify_service': self._notify_service, 'target': self._target, }) diff --git a/homeassistant/auth/mfa_modules/totp.py b/homeassistant/auth/mfa_modules/totp.py index b00bd24cc622e3..625cc0302e1a26 100644 --- a/homeassistant/auth/mfa_modules/totp.py +++ b/homeassistant/auth/mfa_modules/totp.py @@ -60,6 +60,7 @@ class TotpAuthModule(MultiFactorAuthModule): """Auth module validate time-based one time password.""" DEFAULT_TITLE = 'Time-based One Time Password' + MAX_RETRY_TIME = 5 def __init__(self, hass: HomeAssistant, config: Dict[str, Any]) -> None: """Initialize the user data store.""" diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index e0888851d4677a..f2dce6c2ebd8de 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/homeassistant/auth/providers/__init__.py @@ -171,6 +171,7 @@ def __init__(self, auth_provider: AuthProvider) -> None: self._auth_manager = auth_provider.hass.auth # type: ignore self.available_mfa_modules = {} # type: Dict[str, str] self.created_at = dt_util.utcnow() + self.invalid_mfa_times = 0 self.user = None # type: Optional[User] async def async_step_init( @@ -232,12 +233,19 @@ async def async_step_mfa( self.user.id, user_input) # type: ignore if not result: errors['base'] = 'invalid_code' + self.invalid_mfa_times += 1 + if (auth_module.MAX_RETRY_TIME > 0 and + self.invalid_mfa_times >= auth_module.MAX_RETRY_TIME): + return self.async_abort( + reason='too_many_retry' + ) if not errors: return await self.async_finish(self.user) # MFA module may have init code need generate - if hasattr(auth_module, 'async_generate'): + if (self.invalid_mfa_times == 0 and + hasattr(auth_module, 'async_generate')): await auth_module.async_generate(self.user.id) # type: ignore description_placeholders = { diff --git a/tests/auth/mfa_modules/test_notify.py b/tests/auth/mfa_modules/test_notify.py index 3b275ef5ee35da..fe5be14dc19ef1 100644 --- a/tests/auth/mfa_modules/test_notify.py +++ b/tests/auth/mfa_modules/test_notify.py @@ -53,7 +53,7 @@ async def test_validating_mfa_invalid_user(hass): async def test_validating_mfa_counter(hass): - """Test counter will move no matter code validation result.""" + """Test counter will move only after generate code.""" notify_auth_module = await auth_mfa_module_from_config(hass, { 'type': 'notify' }) @@ -67,22 +67,26 @@ async def test_validating_mfa_counter(hass): init_count = notify_setting.counter assert init_count is not None + with patch('pyotp.HOTP.at', return_value=MOCK_CODE): + await notify_auth_module.async_generate('test-user') + + notify_setting = list(notify_auth_module._user_settings.values())[0] + after_generate_count = notify_setting.counter + assert after_generate_count != init_count + with patch('pyotp.HOTP.verify', return_value=True): assert await notify_auth_module.async_validate( 'test-user', {'code': MOCK_CODE}) notify_setting = list(notify_auth_module._user_settings.values())[0] - after_pass_count = notify_setting.counter - assert after_pass_count != init_count + assert after_generate_count == notify_setting.counter with patch('pyotp.HOTP.verify', return_value=False): assert await notify_auth_module.async_validate( 'test-user', {'code': MOCK_CODE}) is False notify_setting = list(notify_auth_module._user_settings.values())[0] - after_fail_count = notify_setting.counter - assert after_fail_count != init_count - assert after_fail_count != after_pass_count + assert after_generate_count == notify_setting.counter async def test_setup_depose_user(hass): @@ -174,6 +178,20 @@ async def test_login_flow_validates_mfa(hass): message.hass = hass assert MOCK_CODE in message.async_render() + with patch('pyotp.HOTP.verify', return_value=False): + result = await hass.auth.login_flow.async_configure( + result['flow_id'], {'code': 'invalid-code'}) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'mfa' + assert result['errors']['base'] == 'invalid_code' + + # wait service call finished + await hass.async_block_till_done() + + # would not send new code, allow user retry + assert len(notify_calls) == 1 + + # retry twice with patch('pyotp.HOTP.verify', return_value=False), \ patch('pyotp.HOTP.at', return_value=MOCK_CODE_2): result = await hass.auth.login_flow.async_configure( @@ -182,6 +200,31 @@ async def test_login_flow_validates_mfa(hass): assert result['step_id'] == 'mfa' assert result['errors']['base'] == 'invalid_code' + # after the 3rd failure, flow abort + result = await hass.auth.login_flow.async_configure( + result['flow_id'], {'code': 'invalid-code'}) + assert result['type'] == data_entry_flow.RESULT_TYPE_ABORT + assert result['reason'] == 'too_many_retry' + + # wait service call finished + await hass.async_block_till_done() + + # restart login + result = await hass.auth.login_flow.async_init( + (provider.type, provider.id)) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + + with patch('pyotp.HOTP.at', return_value=MOCK_CODE): + result = await hass.auth.login_flow.async_configure( + result['flow_id'], + { + 'username': 'test-user', + 'password': 'test-pass', + }) + assert result['type'] == data_entry_flow.RESULT_TYPE_FORM + assert result['step_id'] == 'mfa' + assert result['data_schema'].schema.get('code') == str + # wait service call finished await hass.async_block_till_done() @@ -191,7 +234,7 @@ async def test_login_flow_validates_mfa(hass): assert notify_call.service == 'test-notify' message = notify_call.data['message'] message.hass = hass - assert MOCK_CODE_2 in message.async_render() + assert MOCK_CODE in message.async_render() with patch('pyotp.HOTP.verify', return_value=True): result = await hass.auth.login_flow.async_configure( From 2d8e6daf01cb0bf8f311e3af60e73ba3eb5cc2e0 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 04:00:38 +0000 Subject: [PATCH 05/13] Fix loading --- homeassistant/auth/mfa_modules/notify.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index 6d48d2ccdfc66c..98924a7192c31a 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -106,7 +106,10 @@ async def _async_load(self) -> None: if data is None: data = {STORAGE_USERS: {}} - self._user_settings = data.get(STORAGE_USERS, {}) + self._user_settings = { + user_id: NotifySetting(**setting) + for user_id, setting in data.get(STORAGE_USERS, {}).items() + } async def _async_save(self) -> None: """Save data.""" From 0bf7bc68b3c347e99052fe17549f7640d8501cf1 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 04:09:12 +0000 Subject: [PATCH 06/13] Fix invalaid login log processing --- homeassistant/components/auth/login_flow.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/auth/login_flow.py b/homeassistant/components/auth/login_flow.py index 73a739c2960187..3a51cf8066f783 100644 --- a/homeassistant/components/auth/login_flow.py +++ b/homeassistant/components/auth/login_flow.py @@ -226,8 +226,9 @@ async def post(self, request, flow_id, data): if result['type'] != data_entry_flow.RESULT_TYPE_CREATE_ENTRY: # @log_invalid_auth does not work here since it returns HTTP 200 # need manually log failed login attempts - if result['errors'] is not None and \ - result['errors'].get('base') == 'invalid_auth': + if (result.get('errors') is not None and + result['errors'].get('base') in ['invalid_auth', + 'invalid_code']): await process_wrong_login(request) return self.json(_prepare_result_json(result)) From e2da768c25886a3613d25769dee48abd97ff0625 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 04:18:47 +0000 Subject: [PATCH 07/13] Typing --- homeassistant/auth/mfa_modules/notify.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index 98924a7192c31a..ec5755e0651bfa 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -115,7 +115,8 @@ async def _async_save(self) -> None: """Save data.""" await self._user_store.async_save({STORAGE_USERS: { user_id: attr.asdict(notify_setting) - for user_id, notify_setting in self._user_settings.items() + for user_id, notify_setting + in self._user_settings.items() # type: ignore }}) def _add_user_setup_data(self, user_id: str, @@ -129,7 +130,7 @@ def _add_user_setup_data(self, user_id: str, ota_secret = secret or pyotp.random_base32() # type: str init_counter = counter - self._user_settings[user_id] = NotifySetting( + self._user_settings[user_id] = NotifySetting( # type: ignore secret=ota_secret, counter=init_counter, notify_service=notify_service, @@ -208,7 +209,8 @@ async def async_validate( def _validate_one_time_password(self, user_id: str, code: str) -> bool: """Validate one time password.""" - notify_setting = self._user_settings.get(user_id, None) + notify_setting = self._user_settings.get( # type: ignore + user_id, None) if notify_setting is None: # even we cannot find user, we still do verify # to make timing the same as if user was found. @@ -233,7 +235,8 @@ async def async_generate(self, user_id: str) -> None: def _generate_and_send_one_time_password(self, user_id: str) -> str: """Generate and send one time password.""" - notify_setting = self._user_settings.get(user_id, None) + notify_setting = self._user_settings.get( # type: ignore + user_id, None) if notify_setting is None: raise ValueError('Cannot find user_id') @@ -246,12 +249,13 @@ async def async_notify_user(self, user_id: str, code: str) -> None: if self._user_settings is None: await self._async_load() - notify_setting = self._user_settings.get(user_id, None) + notify_setting = self._user_settings.get( # type: ignore + user_id, None) if notify_setting is None: _LOGGER.error('Cannot find user %s', user_id) return - await self.async_notify( + await self.async_notify( # type: ignore code, notify_setting.notify_service, notify_setting.target) async def async_notify(self, code: str, notify_service: str, @@ -336,7 +340,7 @@ async def async_step_setup( result = await self._auth_module.async_setup_user( self._user_id, { 'secret': self._ota_secret, - 'counter': self._counter + 1, # increase counter + 'counter': self._counter + 1, # type: ignore 'notify_service': self._notify_service, 'target': self._target, }) From fd65a1b5798494cee9d15cf150e19d80a851c69b Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 05:30:58 -0700 Subject: [PATCH 08/13] Change default message template --- homeassistant/auth/mfa_modules/notify.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index ec5755e0651bfa..cba66cbbc22a9e 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -24,8 +24,8 @@ CONFIG_SCHEMA = MULTI_FACTOR_AUTH_MODULE_SCHEMA.extend({ vol.Optional(CONF_INCLUDE): vol.All(cv.ensure_list, [cv.string]), vol.Optional(CONF_EXCLUDE): vol.All(cv.ensure_list, [cv.string]), - vol.Optional(CONF_MESSAGE, default='Your Home Assistant One-time Password' - ' is "{}"'): str + vol.Optional(CONF_MESSAGE, + default='{} is your Home Assistant login code'): str }, extra=vol.PREVENT_EXTRA) STORAGE_VERSION = 1 From 471b73a1866a884d7d29d7fd2cea2c8ce19c5f4b Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Wed, 5 Sep 2018 10:22:28 -0700 Subject: [PATCH 09/13] Change one-time password to 8 digit --- homeassistant/auth/mfa_modules/notify.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index cba66cbbc22a9e..e8fd742c4c7ba7 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -4,7 +4,6 @@ """ import logging from collections import OrderedDict -from random import SystemRandom from typing import Any, Dict, Optional, Tuple, List # noqa: F401 import attr @@ -60,7 +59,7 @@ def _generate_secret_and_init_counter() -> Tuple[str, int]: import pyotp ota_secret = pyotp.random_base32() - counter = SystemRandom().randint(0, 2 << 31) + counter = int(pyotp.random_base32(length=8, chars=list('1234567890'))) return ota_secret, counter @@ -68,14 +67,14 @@ def _generate_otp(secret: str, count: int) -> str: """Generate one time password.""" import pyotp - return str(pyotp.HOTP(secret).at(count)) + return str(pyotp.HOTP(secret, digits=8).at(count)) def _verify_otp(secret: str, otp: str, counter: int) -> bool: """Verify one time password.""" import pyotp - return bool(pyotp.HOTP(secret).verify(otp, counter)) + return bool(pyotp.HOTP(secret, digits=8).verify(otp, counter)) @MULTI_FACTOR_AUTH_MODULES.register('notify') @@ -334,7 +333,7 @@ async def async_step_setup( if user_input: hass = self._auth_module.hass verified = await hass.async_add_executor_job( - pyotp.HOTP(self._ota_secret).verify, + pyotp.HOTP(self._ota_secret, digits=8).verify, user_input['code'], self._counter) if verified: result = await self._auth_module.async_setup_user( From 942573dc36b8b92d206b1581ad2797acae361859 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Thu, 6 Sep 2018 12:53:22 -0700 Subject: [PATCH 10/13] Refactoring to not save secret --- homeassistant/auth/const.py | 1 + homeassistant/auth/mfa_modules/__init__.py | 3 - homeassistant/auth/mfa_modules/notify.py | 199 +++++++++------------ homeassistant/auth/providers/__init__.py | 17 +- tests/auth/mfa_modules/test_notify.py | 2 +- tests/auth/test_init.py | 2 +- 6 files changed, 94 insertions(+), 130 deletions(-) diff --git a/homeassistant/auth/const.py b/homeassistant/auth/const.py index 082d8966275670..b18cdaba49e013 100644 --- a/homeassistant/auth/const.py +++ b/homeassistant/auth/const.py @@ -2,3 +2,4 @@ from datetime import timedelta ACCESS_TOKEN_EXPIRATION = timedelta(minutes=30) +SESSION_EXPIRATION = timedelta(minutes=5) diff --git a/homeassistant/auth/mfa_modules/__init__.py b/homeassistant/auth/mfa_modules/__init__.py index 5401f08584cdf8..1746ef38f9580a 100644 --- a/homeassistant/auth/mfa_modules/__init__.py +++ b/homeassistant/auth/mfa_modules/__init__.py @@ -1,5 +1,4 @@ """Plugable auth modules for Home Assistant.""" -from datetime import timedelta import importlib import logging import types @@ -23,8 +22,6 @@ vol.Optional(CONF_ID): str, }, extra=vol.ALLOW_EXTRA) -SESSION_EXPIRATION = timedelta(minutes=5) - DATA_REQS = 'mfa_auth_module_reqs_processed' _LOGGER = logging.getLogger(__name__) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index e8fd742c4c7ba7..1f7790eed89b73 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -36,45 +36,44 @@ INPUT_FIELD_CODE = 'code' -DUMMY_SECRET = '7Z5EFWI4RFLVV67G' - _LOGGER = logging.getLogger(__name__) -@attr.s(slots=True) -class NotifySetting: - """Store notify setting for one user.""" - - secret = attr.ib(type=str) - counter = attr.ib(type=int) - notify_service = attr.ib(type=Optional[str], default=None) - target = attr.ib(type=Optional[str], default=None) - - -_UsersDict = Dict[str, NotifySetting] +def _generate_secret() -> str: + """Generate a secret.""" + import pyotp + return str(pyotp.random_base32()) -def _generate_secret_and_init_counter() -> Tuple[str, int]: - """Generate a secret and a random initial counter.""" +def _generate_random() -> int: + """Generate a 8 digit number.""" import pyotp - - ota_secret = pyotp.random_base32() - counter = int(pyotp.random_base32(length=8, chars=list('1234567890'))) - return ota_secret, counter + return int(pyotp.random_base32(length=8, chars=list('1234567890'))) def _generate_otp(secret: str, count: int) -> str: """Generate one time password.""" import pyotp - return str(pyotp.HOTP(secret, digits=8).at(count)) -def _verify_otp(secret: str, otp: str, counter: int) -> bool: +def _verify_otp(secret: str, otp: str, count: int) -> bool: """Verify one time password.""" import pyotp + return bool(pyotp.HOTP(secret, digits=8).verify(otp, count)) - return bool(pyotp.HOTP(secret, digits=8).verify(otp, counter)) + +@attr.s(slots=True) +class NotifySetting: + """Store notify setting for one user.""" + + secret = attr.ib(type=str, factory=_generate_secret) # not persistent + counter = attr.ib(type=int, factory=_generate_random) # not persistent + notify_service = attr.ib(type=Optional[str], default=None) + target = attr.ib(type=Optional[str], default=None) + + +_UsersDict = Dict[str, NotifySetting] @MULTI_FACTOR_AUTH_MODULES.register('notify') @@ -82,6 +81,7 @@ class NotifyAuthModule(MultiFactorAuthModule): """Auth module send hmac-based one time password by notify service.""" DEFAULT_TITLE = 'Notify One-Time Password' + DUMMY_SECRET = '7Z5EFWI4RFLVV67G' def __init__(self, hass: HomeAssistant, config: Dict[str, Any]) -> None: """Initialize the user data store.""" @@ -112,37 +112,24 @@ async def _async_load(self) -> None: async def _async_save(self) -> None: """Save data.""" + if self._user_settings is None: + return + await self._user_store.async_save({STORAGE_USERS: { - user_id: attr.asdict(notify_setting) + user_id: attr.asdict( + notify_setting, filter=attr.filters.exclude( + attr.fields(NotifySetting).secret, + attr.fields(NotifySetting).counter, + )) for user_id, notify_setting - in self._user_settings.items() # type: ignore + in self._user_settings.items() }}) - def _add_user_setup_data(self, user_id: str, - secret: Optional[str] = None, - counter: int = 0, - notify_service: Optional[str] = None, - target: Optional[str] = None) -> None: - """Create a ota_secret for user.""" - import pyotp - - ota_secret = secret or pyotp.random_base32() # type: str - init_counter = counter - - self._user_settings[user_id] = NotifySetting( # type: ignore - secret=ota_secret, - counter=init_counter, - notify_service=notify_service, - target=target, - ) - @callback def aync_get_available_notify_services(self) -> List[str]: """Return list of notify services.""" - unordered_services = [ - service_id for service_id in - self.hass.services.async_services().get( - 'notify', {}).keys()] + unordered_services = list(self.hass.services.async_services().get( + 'notify', {})) for exclude_service in self._exclude: if exclude_service in unordered_services: @@ -167,13 +154,11 @@ async def async_setup_user(self, user_id: str, setup_data: Any) -> Any: """Set up auth module for user.""" if self._user_settings is None: await self._async_load() + assert self._user_settings is not None - await self.hass.async_add_executor_job( - self._add_user_setup_data, user_id, - setup_data.get('secret'), - int(setup_data.get('counter', 0)), - setup_data.get('notify_service'), - setup_data.get('target'), + self._user_settings[user_id] = NotifySetting( + notify_service=setup_data.get('notify_service'), + target=setup_data.get('target'), ) await self._async_save() @@ -182,74 +167,67 @@ async def async_depose_user(self, user_id: str) -> None: """Depose auth module for user.""" if self._user_settings is None: await self._async_load() + assert self._user_settings - if self._user_settings.pop(user_id, None): # type: ignore + if self._user_settings.pop(user_id, None): await self._async_save() async def async_is_user_setup(self, user_id: str) -> bool: """Return whether user is setup.""" if self._user_settings is None: await self._async_load() + assert self._user_settings - return user_id in self._user_settings # type: ignore + return user_id in self._user_settings async def async_validate( self, user_id: str, user_input: Dict[str, Any]) -> bool: """Return True if validation passed.""" if self._user_settings is None: await self._async_load() + assert self._user_settings - # user_input has been validate in caller - result = await self.hass.async_add_executor_job( - self._validate_one_time_password, user_id, - user_input.get(INPUT_FIELD_CODE, '')) - - return result - - def _validate_one_time_password(self, user_id: str, code: str) -> bool: - """Validate one time password.""" - notify_setting = self._user_settings.get( # type: ignore - user_id, None) + notify_setting = self._user_settings.get(user_id, None) if notify_setting is None: - # even we cannot find user, we still do verify - # to make timing the same as if user was found. - _verify_otp(DUMMY_SECRET, code, 0) return False - return _verify_otp( - notify_setting.secret, code, notify_setting.counter) + # user_input has been validate in caller + return await self.hass.async_add_executor_job( + _verify_otp, notify_setting.secret, + user_input.get(INPUT_FIELD_CODE, ''), + notify_setting.counter) - async def async_generate(self, user_id: str) -> None: + async def async_initialize(self, user_id: str) -> None: """Generate code and notify user.""" if self._user_settings is None: await self._async_load() + assert self._user_settings - code = await self.hass.async_add_executor_job( - self._generate_and_send_one_time_password, user_id) - - # update counter in storage - await self._async_save() - - await self.async_notify_user(user_id, code) - - def _generate_and_send_one_time_password(self, user_id: str) -> str: - """Generate and send one time password.""" - notify_setting = self._user_settings.get( # type: ignore - user_id, None) + notify_setting = self._user_settings.get(user_id, None) if notify_setting is None: raise ValueError('Cannot find user_id') - # always move counter before generate new code - notify_setting.counter += 1 - return _generate_otp(notify_setting.secret, notify_setting.counter) + def generate_secret_and_one_time_password() -> str: + """Generate and send one time password.""" + assert notify_setting + # secret and counter are not persistent + notify_setting.secret = _generate_secret() + notify_setting.counter = _generate_random() + return _generate_otp( + notify_setting.secret, notify_setting.counter) + + code = await self.hass.async_add_executor_job( + generate_secret_and_one_time_password) + + await self.async_notify_user(user_id, code) async def async_notify_user(self, user_id: str, code: str) -> None: """Send code by user's notify service.""" if self._user_settings is None: await self._async_load() + assert self._user_settings - notify_setting = self._user_settings.get( # type: ignore - user_id, None) + notify_setting = self._user_settings.get(user_id, None) if notify_setting is None: _LOGGER.error('Cannot find user %s', user_id) return @@ -279,35 +257,29 @@ def __init__(self, auth_module: NotifyAuthModule, # to fix typing complaint self._auth_module = auth_module # type: NotifyAuthModule self._available_notify_services = available_notify_services - self._ota_secret = None # type: Optional[str] - self._counter = None # type Optional[int] + self._secret = None # type: Optional[str] + self._count = None # type: Optional[int] self._notify_service = None # type: Optional[str] self._target = None # type: Optional[str] async def async_step_init( self, user_input: Optional[Dict[str, str]] = None) \ -> Dict[str, Any]: - """Handle the first step of setup flow. - - Return self.async_show_form(step_id='init') if user_input == None. - Return self.async_create_entry(data={'result': result}) if finish. - """ + """Let user select available notify services.""" errors = {} # type: Dict[str, str] hass = self._auth_module.hass if user_input: self._notify_service = user_input['notify_service'] self._target = user_input.get('target') + self._secret = await hass.async_add_executor_job(_generate_secret) + self._count = await hass.async_add_executor_job(_generate_random) return await self.async_step_setup() if not self._available_notify_services: return self.async_abort(reason='no_available_service') - self._ota_secret, self._counter = \ - await hass.async_add_executor_job( # type: ignore - _generate_secret_and_init_counter) - schema = OrderedDict() # type: Dict[str, Any] schema['notify_service'] = vol.In(self._available_notify_services) schema['target'] = vol.Optional(str) @@ -321,43 +293,38 @@ async def async_step_init( async def async_step_setup( self, user_input: Optional[Dict[str, str]] = None) \ -> Dict[str, Any]: - """Handle the setup step of setup flow. - - Return self.async_show_form(step_id='init') if user_input == None. - Return self.async_create_entry(data={'result': result}) if finish. - """ - import pyotp - + """Verify user can recevie one-time password.""" errors = {} # type: Dict[str, str] + hass = self._auth_module.hass if user_input: - hass = self._auth_module.hass verified = await hass.async_add_executor_job( - pyotp.HOTP(self._ota_secret, digits=8).verify, - user_input['code'], self._counter) + _verify_otp, self._secret, user_input['code'], self._count) if verified: - result = await self._auth_module.async_setup_user( + await self._auth_module.async_setup_user( self._user_id, { - 'secret': self._ota_secret, - 'counter': self._counter + 1, # type: ignore 'notify_service': self._notify_service, 'target': self._target, }) return self.async_create_entry( title=self._auth_module.name, - data={'result': result} + data={} ) errors['base'] = 'invalid_code' - code = _generate_otp(self._ota_secret, self._counter) # type: ignore + # generate code every time, no retry logic + assert self._secret and self._count + code = await hass.async_add_executor_job( + _generate_otp, self._secret, self._count) - await self._auth_module.async_notify( # type: ignore + assert self._notify_service + await self._auth_module.async_notify( code, self._notify_service, self._target) return self.async_show_form( step_id='setup', data_schema=self._setup_schema, description_placeholders={'notify_service': self._notify_service}, - errors=errors + errors=errors, ) diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index f2dce6c2ebd8de..11edd4bad7e7a6 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/homeassistant/auth/providers/__init__.py @@ -15,8 +15,8 @@ from homeassistant.util.decorator import Registry from ..auth_store import AuthStore +from ..const import SESSION_EXPIRATION from ..models import Credentials, User, UserMeta # noqa: F401 -from ..mfa_modules import SESSION_EXPIRATION _LOGGER = logging.getLogger(__name__) DATA_REQS = 'auth_prov_reqs_processed' @@ -213,6 +213,8 @@ async def async_step_mfa( self, user_input: Optional[Dict[str, str]] = None) \ -> Dict[str, Any]: """Handle the step of mfa validation.""" + assert self.user + errors = {} auth_module = self._auth_manager.get_auth_mfa_module( @@ -222,6 +224,9 @@ async def async_step_mfa( # will show invalid_auth_module error return await self.async_step_select_mfa_module(user_input={}) + if user_input is None and hasattr(auth_module, 'async_initialize'): + await auth_module.async_initialize(self.user.id) + if user_input is not None: expires = self.created_at + SESSION_EXPIRATION if dt_util.utcnow() > expires: @@ -230,12 +235,11 @@ async def async_step_mfa( ) result = await auth_module.async_validate( - self.user.id, user_input) # type: ignore + self.user.id, user_input) if not result: errors['base'] = 'invalid_code' self.invalid_mfa_times += 1 - if (auth_module.MAX_RETRY_TIME > 0 and - self.invalid_mfa_times >= auth_module.MAX_RETRY_TIME): + if self.invalid_mfa_times >= auth_module.MAX_RETRY_TIME > 0: return self.async_abort( reason='too_many_retry' ) @@ -243,11 +247,6 @@ async def async_step_mfa( if not errors: return await self.async_finish(self.user) - # MFA module may have init code need generate - if (self.invalid_mfa_times == 0 and - hasattr(auth_module, 'async_generate')): - await auth_module.async_generate(self.user.id) # type: ignore - description_placeholders = { 'mfa_module_name': auth_module.name, 'mfa_module_id': auth_module.id, diff --git a/tests/auth/mfa_modules/test_notify.py b/tests/auth/mfa_modules/test_notify.py index fe5be14dc19ef1..220defb19bde71 100644 --- a/tests/auth/mfa_modules/test_notify.py +++ b/tests/auth/mfa_modules/test_notify.py @@ -68,7 +68,7 @@ async def test_validating_mfa_counter(hass): assert init_count is not None with patch('pyotp.HOTP.at', return_value=MOCK_CODE): - await notify_auth_module.async_generate('test-user') + await notify_auth_module.async_initialize('test-user') notify_setting = list(notify_auth_module._user_settings.values())[0] after_generate_count = notify_setting.counter diff --git a/tests/auth/test_init.py b/tests/auth/test_init.py index 8325bd2551aa30..0da8cc803b2aa4 100644 --- a/tests/auth/test_init.py +++ b/tests/auth/test_init.py @@ -9,7 +9,7 @@ from homeassistant import auth, data_entry_flow from homeassistant.auth import ( models as auth_models, auth_store, const as auth_const) -from homeassistant.auth.mfa_modules import SESSION_EXPIRATION +from homeassistant.auth.const import SESSION_EXPIRATION from homeassistant.util import dt as dt_util from tests.common import ( MockUser, ensure_auth_manager_loaded, flush_store, CLIENT_ID) From 8a8746581be1d3708499f8531ef6c9db2fc09f9b Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Thu, 6 Sep 2018 13:39:32 -0700 Subject: [PATCH 11/13] Bug fixing --- homeassistant/auth/mfa_modules/notify.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index 1f7790eed89b73..944c8e954e305f 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -167,7 +167,7 @@ async def async_depose_user(self, user_id: str) -> None: """Depose auth module for user.""" if self._user_settings is None: await self._async_load() - assert self._user_settings + assert self._user_settings is not None if self._user_settings.pop(user_id, None): await self._async_save() @@ -176,7 +176,7 @@ async def async_is_user_setup(self, user_id: str) -> bool: """Return whether user is setup.""" if self._user_settings is None: await self._async_load() - assert self._user_settings + assert self._user_settings is not None return user_id in self._user_settings @@ -185,7 +185,7 @@ async def async_validate( """Return True if validation passed.""" if self._user_settings is None: await self._async_load() - assert self._user_settings + assert self._user_settings is not None notify_setting = self._user_settings.get(user_id, None) if notify_setting is None: @@ -201,7 +201,7 @@ async def async_initialize(self, user_id: str) -> None: """Generate code and notify user.""" if self._user_settings is None: await self._async_load() - assert self._user_settings + assert self._user_settings is not None notify_setting = self._user_settings.get(user_id, None) if notify_setting is None: @@ -225,7 +225,7 @@ async def async_notify_user(self, user_id: str, code: str) -> None: """Send code by user's notify service.""" if self._user_settings is None: await self._async_load() - assert self._user_settings + assert self._user_settings is not None notify_setting = self._user_settings.get(user_id, None) if notify_setting is None: From 24f10cdab613357fbc48593a066c3579fc66b672 Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Thu, 6 Sep 2018 13:52:50 -0700 Subject: [PATCH 12/13] Change async_initialize method name to aysnc_initialize_login_mfa_step --- homeassistant/auth/mfa_modules/notify.py | 2 +- homeassistant/auth/providers/__init__.py | 5 +++-- tests/auth/mfa_modules/test_notify.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index 944c8e954e305f..9879bd89472055 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -197,7 +197,7 @@ async def async_validate( user_input.get(INPUT_FIELD_CODE, ''), notify_setting.counter) - async def async_initialize(self, user_id: str) -> None: + async def async_initialize_login_mfa_step(self, user_id: str) -> None: """Generate code and notify user.""" if self._user_settings is None: await self._async_load() diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index 11edd4bad7e7a6..0fcdc3eb3b28cf 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/homeassistant/auth/providers/__init__.py @@ -224,8 +224,9 @@ async def async_step_mfa( # will show invalid_auth_module error return await self.async_step_select_mfa_module(user_input={}) - if user_input is None and hasattr(auth_module, 'async_initialize'): - await auth_module.async_initialize(self.user.id) + if user_input is None and hasattr(auth_module, + 'async_initialize_login_mfa_step'): + await auth_module.async_initialize_login_mfa_step(self.user.id) if user_input is not None: expires = self.created_at + SESSION_EXPIRATION diff --git a/tests/auth/mfa_modules/test_notify.py b/tests/auth/mfa_modules/test_notify.py index 220defb19bde71..ffe0b103fc955f 100644 --- a/tests/auth/mfa_modules/test_notify.py +++ b/tests/auth/mfa_modules/test_notify.py @@ -68,7 +68,7 @@ async def test_validating_mfa_counter(hass): assert init_count is not None with patch('pyotp.HOTP.at', return_value=MOCK_CODE): - await notify_auth_module.async_initialize('test-user') + await notify_auth_module.async_initialize_login_mfa_step('test-user') notify_setting = list(notify_auth_module._user_settings.values())[0] after_generate_count = notify_setting.counter From fd20a9b37b1b9917852d969f41f0224eff53fadc Mon Sep 17 00:00:00 2001 From: Jason Hu Date: Sun, 23 Sep 2018 11:07:37 -0700 Subject: [PATCH 13/13] Address some simple fix code review comment --- homeassistant/auth/const.py | 2 +- homeassistant/auth/mfa_modules/notify.py | 19 +++++++------------ homeassistant/auth/providers/__init__.py | 4 ++-- tests/auth/test_init.py | 4 ++-- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/homeassistant/auth/const.py b/homeassistant/auth/const.py index b18cdaba49e013..2e57986958c39a 100644 --- a/homeassistant/auth/const.py +++ b/homeassistant/auth/const.py @@ -2,4 +2,4 @@ from datetime import timedelta ACCESS_TOKEN_EXPIRATION = timedelta(minutes=30) -SESSION_EXPIRATION = timedelta(minutes=5) +MFA_SESSION_EXPIRATION = timedelta(minutes=5) diff --git a/homeassistant/auth/mfa_modules/notify.py b/homeassistant/auth/mfa_modules/notify.py index 9879bd89472055..84f9de614c169a 100644 --- a/homeassistant/auth/mfa_modules/notify.py +++ b/homeassistant/auth/mfa_modules/notify.py @@ -31,8 +31,6 @@ STORAGE_KEY = 'auth_module.notify' STORAGE_USERS = 'users' STORAGE_USER_ID = 'user_id' -STORAGE_OTA_SECRET = 'ota_secret' -STORAGE_COUNTER = 'counter' INPUT_FIELD_CODE = 'code' @@ -54,13 +52,13 @@ def _generate_random() -> int: def _generate_otp(secret: str, count: int) -> str: """Generate one time password.""" import pyotp - return str(pyotp.HOTP(secret, digits=8).at(count)) + return str(pyotp.HOTP(secret).at(count)) def _verify_otp(secret: str, otp: str, count: int) -> bool: """Verify one time password.""" import pyotp - return bool(pyotp.HOTP(secret, digits=8).verify(otp, count)) + return bool(pyotp.HOTP(secret).verify(otp, count)) @attr.s(slots=True) @@ -81,7 +79,6 @@ class NotifyAuthModule(MultiFactorAuthModule): """Auth module send hmac-based one time password by notify service.""" DEFAULT_TITLE = 'Notify One-Time Password' - DUMMY_SECRET = '7Z5EFWI4RFLVV67G' def __init__(self, hass: HomeAssistant, config: Dict[str, Any]) -> None: """Initialize the user data store.""" @@ -128,16 +125,14 @@ async def _async_save(self) -> None: @callback def aync_get_available_notify_services(self) -> List[str]: """Return list of notify services.""" - unordered_services = list(self.hass.services.async_services().get( - 'notify', {})) + unordered_services = set() - for exclude_service in self._exclude: - if exclude_service in unordered_services: - unordered_services.remove(exclude_service) + for service in self.hass.services.async_services().get('notify', {}): + if service not in self._exclude: + unordered_services.add(service) if self._include: - unordered_services = [s for s in self._include - if s in unordered_services] + unordered_services &= set(self._include) return sorted(unordered_services) diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index 0fcdc3eb3b28cf..e96f6d7ebbaf38 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/homeassistant/auth/providers/__init__.py @@ -15,7 +15,7 @@ from homeassistant.util.decorator import Registry from ..auth_store import AuthStore -from ..const import SESSION_EXPIRATION +from ..const import MFA_SESSION_EXPIRATION from ..models import Credentials, User, UserMeta # noqa: F401 _LOGGER = logging.getLogger(__name__) @@ -229,7 +229,7 @@ async def async_step_mfa( await auth_module.async_initialize_login_mfa_step(self.user.id) if user_input is not None: - expires = self.created_at + SESSION_EXPIRATION + expires = self.created_at + MFA_SESSION_EXPIRATION if dt_util.utcnow() > expires: return self.async_abort( reason='login_expired' diff --git a/tests/auth/test_init.py b/tests/auth/test_init.py index 0da8cc803b2aa4..8fd9b8930e4109 100644 --- a/tests/auth/test_init.py +++ b/tests/auth/test_init.py @@ -9,7 +9,7 @@ from homeassistant import auth, data_entry_flow from homeassistant.auth import ( models as auth_models, auth_store, const as auth_const) -from homeassistant.auth.const import SESSION_EXPIRATION +from homeassistant.auth.const import MFA_SESSION_EXPIRATION from homeassistant.util import dt as dt_util from tests.common import ( MockUser, ensure_auth_manager_loaded, flush_store, CLIENT_ID) @@ -720,7 +720,7 @@ async def test_auth_module_expired_session(mock_hass): assert step['step_id'] == 'mfa' with patch('homeassistant.util.dt.utcnow', - return_value=dt_util.utcnow() + SESSION_EXPIRATION): + return_value=dt_util.utcnow() + MFA_SESSION_EXPIRATION): step = await manager.login_flow.async_configure(step['flow_id'], { 'pin': 'test-pin', })