Blow up startup if init auth providers or modules failed

homeassistant/auth/__init__.py

@@ -24,7 +24,11 @@ async def auth_manager_from_config(
         hass: HomeAssistant,
         provider_configs: List[Dict[str, Any]],
         module_configs: List[Dict[str, Any]]) -> 'AuthManager':
-    """Initialize an auth manager from config."""
+    """Initialize an auth manager from config.
+
+    CORE_CONFIG_SCHEMA will make sure do duplicated auth providers or
+    mfa modules exist in configs.
+    """
     store = auth_store.AuthStore(hass)
     if provider_configs:
         providers = await asyncio.gather(
@@ -35,17 +39,7 @@ async def auth_manager_from_config(
     # So returned auth providers are in same order as config
     provider_hash = OrderedDict()  # type: _ProviderDict
     for provider in providers:
-        if provider is None:
-            continue
-
         key = (provider.type, provider.id)
-
-        if key in provider_hash:
-            _LOGGER.error(
-                'Found duplicate provider: %s. Please add unique IDs if you '
-                'want to have the same provider twice.', key)
-            continue
-
         provider_hash[key] = provider
 
     if module_configs:
@@ -57,15 +51,6 @@ async def auth_manager_from_config(
     # So returned auth modules are in same order as config
     module_hash = OrderedDict()  # type: _MfaModuleDict
     for module in modules:
-        if module is None:
-            continue
-
-        if module.id in module_hash:
-            _LOGGER.error(
-                'Found duplicate multi-factor module: %s. Please add unique '
-                'IDs if you want to have the same module twice.', module.id)
-            continue
-
         module_hash[module.id] = module
 
     manager = AuthManager(hass, store, provider_hash, module_hash)

homeassistant/auth/mfa_modules/__init__.py

@@ -11,6 +11,7 @@
 from homeassistant import requirements, data_entry_flow
 from homeassistant.const import CONF_ID, CONF_NAME, CONF_TYPE
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.util.decorator import Registry
 
 MULTI_FACTOR_AUTH_MODULES = Registry()
@@ -127,34 +128,32 @@ async def async_step_init(
 
 async def auth_mfa_module_from_config(
         hass: HomeAssistant, config: Dict[str, Any]) \
-        -> Optional[MultiFactorAuthModule]:
+        -> MultiFactorAuthModule:
     """Initialize an auth module from a config."""
     module_name = config[CONF_TYPE]
     module = await _load_mfa_module(hass, module_name)
 
-    if module is None:
-        return None
-
     try:
         config = module.CONFIG_SCHEMA(config)  # type: ignore
     except vol.Invalid as err:
         _LOGGER.error('Invalid configuration for multi-factor module %s: %s',
                       module_name, humanize_error(config, err))
-        return None
+        raise
 
     return MULTI_FACTOR_AUTH_MODULES[module_name](hass, config)  # type: ignore
 
 
 async def _load_mfa_module(hass: HomeAssistant, module_name: str) \
-        -> Optional[types.ModuleType]:
+        -> types.ModuleType:
     """Load an mfa auth module."""
     module_path = 'homeassistant.auth.mfa_modules.{}'.format(module_name)
 
     try:
         module = importlib.import_module(module_path)
     except ImportError as err:
         _LOGGER.error('Unable to load mfa module %s: %s', module_name, err)
-        return None
+        raise HomeAssistantError('Unable to load mfa module {}: {}'.format(
+            module_name, err))
 
     if hass.config.skip_pip or not hasattr(module, 'REQUIREMENTS'):
         return module
@@ -170,7 +169,9 @@ async def _load_mfa_module(hass: HomeAssistant, module_name: str) \
         hass, module_path, module.REQUIREMENTS)    # type: ignore
 
     if not req_success:
-        return None
+        raise HomeAssistantError(
+            'Unable to process requirements of mfa module {}'.format(
+                module_name))
 
     processed.add(module_name)
     return module

homeassistant/auth/providers/__init__.py

@@ -10,6 +10,7 @@
 from homeassistant import data_entry_flow, requirements
 from homeassistant.core import callback, HomeAssistant
 from homeassistant.const import CONF_ID, CONF_NAME, CONF_TYPE
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.util import dt as dt_util
 from homeassistant.util.decorator import Registry
 
@@ -110,33 +111,31 @@ async def async_user_meta_for_credentials(
 
 async def auth_provider_from_config(
         hass: HomeAssistant, store: AuthStore,
-        config: Dict[str, Any]) -> Optional[AuthProvider]:
+        config: Dict[str, Any]) -> AuthProvider:
     """Initialize an auth provider from a config."""
     provider_name = config[CONF_TYPE]
     module = await load_auth_provider_module(hass, provider_name)
 
-    if module is None:
-        return None
-
     try:
         config = module.CONFIG_SCHEMA(config)  # type: ignore
     except vol.Invalid as err:
         _LOGGER.error('Invalid configuration for auth provider %s: %s',
                       provider_name, humanize_error(config, err))
-        return None
+        raise
 
     return AUTH_PROVIDERS[provider_name](hass, store, config)  # type: ignore
 
 
 async def load_auth_provider_module(
-        hass: HomeAssistant, provider: str) -> Optional[types.ModuleType]:
+        hass: HomeAssistant, provider: str) -> types.ModuleType:
     """Load an auth provider."""
     try:
         module = importlib.import_module(
             'homeassistant.auth.providers.{}'.format(provider))
     except ImportError as err:
         _LOGGER.error('Unable to load auth provider %s: %s', provider, err)
-        return None
+        raise HomeAssistantError('Unable to load auth provider {}: {}'.format(
+            provider, err))
 
     if hass.config.skip_pip or not hasattr(module, 'REQUIREMENTS'):
         return module
@@ -154,7 +153,9 @@ async def load_auth_provider_module(
         hass, 'auth provider {}'.format(provider), reqs)
 
     if not req_success:
-        return None
+        raise HomeAssistantError(
+            'Unable to process requirements of auth provider {}'.format(
+                provider))
 
     processed.add(provider)
     return module

homeassistant/bootstrap.py

@@ -61,7 +61,6 @@ def from_config_dict(config: Dict[str, Any],
             config, hass, config_dir, enable_log, verbose, skip_pip,
             log_rotate_days, log_file, log_no_color)
     )
-
     return hass
 
 
@@ -94,8 +93,13 @@ async def async_from_config_dict(config: Dict[str, Any],
     try:
         await conf_util.async_process_ha_core_config(
             hass, core_config, has_api_password, has_trusted_networks)
-    except vol.Invalid as ex:
-        conf_util.async_log_exception(ex, 'homeassistant', core_config, hass)
+    except vol.Invalid as config_err:
+        conf_util.async_log_exception(
+            config_err, 'homeassistant', core_config, hass)
+        return None
+    except HomeAssistantError:
+        _LOGGER.error("Home Assistant core failed to initialize. "
+                      "Further initialization aborted")
         return None
 
     await hass.async_add_executor_job(
@@ -130,7 +134,7 @@ async def async_from_config_dict(config: Dict[str, Any],
     res = await core_components.async_setup(hass, config)
     if not res:
         _LOGGER.error("Home Assistant core failed to initialize. "
-                      "further initialization aborted")
+                      "Further initialization aborted")
         return hass
 
     await persistent_notification.async_setup(hass, config)

homeassistant/config.py

@@ -8,7 +8,7 @@
 import shutil
 # pylint: disable=unused-import
 from typing import (  # noqa: F401
-    Any, Tuple, Optional, Dict, List, Union, Callable)
+    Any, Tuple, Optional, Dict, List, Union, Callable, Sequence, Set)
 from types import ModuleType
 import voluptuous as vol
 from voluptuous.humanize import humanize_error
@@ -23,7 +23,7 @@
     CONF_UNIT_SYSTEM_IMPERIAL, CONF_TEMPERATURE_UNIT, TEMP_CELSIUS,
     __version__, CONF_CUSTOMIZE, CONF_CUSTOMIZE_DOMAIN, CONF_CUSTOMIZE_GLOB,
     CONF_WHITELIST_EXTERNAL_DIRS, CONF_AUTH_PROVIDERS, CONF_AUTH_MFA_MODULES,
-    CONF_TYPE)
+    CONF_TYPE, CONF_ID)
 from homeassistant.core import callback, DOMAIN as CONF_CORE, HomeAssistant
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.loader import get_component, get_platform
@@ -128,6 +128,48 @@
 """
 
 
+def _no_duplicate_auth_provider(configs: Sequence[Dict[str, Any]]) \
+        -> Sequence[Dict[str, Any]]:
+    """No duplicate auth provider config allowed in a list.
+
+    Each type of auth provider can only have one config without optional id.
+    Unique id is required if same type of auth provider used multiple times.
+    """
+    config_keys = set()  # type: Set[Tuple[str, Optional[str]]]
+    for config in configs:
+        key = (config[CONF_TYPE], config.get(CONF_ID))
+        if key in config_keys:
+            raise vol.Invalid(
+                'Duplicate auth provider {} found. Please add unique IDs if '
+                'you want to have the same auth provider twice'.format(
+                    config[CONF_TYPE]
+                ))
+        config_keys.add(key)
+    return configs
+
+
+def _no_duplicate_auth_mfa_module(configs: Sequence[Dict[str, Any]]) \
+        -> Sequence[Dict[str, Any]]:
+    """No duplicate auth mfa module item allowed in a list.
+
+    Each type of mfa module can only have one config without optional id.
+    A global unique id is required if same type of mfa module used multiple
+    times.
+    Note: this is different than auth provider
+    """
+    config_keys = set()  # type: Set[str]
+    for config in configs:
+        key = config.get(CONF_ID, config[CONF_TYPE])
+        if key in config_keys:
+            raise vol.Invalid(
+                'Duplicate mfa module {} found. Please add unique IDs if '
+                'you want to have the same mfa module twice'.format(
+                    config[CONF_TYPE]
+                ))
+        config_keys.add(key)
+    return configs
+
+
 PACKAGES_CONFIG_SCHEMA = vol.Schema({
     cv.slug: vol.Schema(  # Package names are slugs
         {cv.slug: vol.Any(dict, list, None)})  # Only slugs for component names
@@ -166,10 +208,16 @@
                     CONF_TYPE: vol.NotIn(['insecure_example'],
                                          'The insecure_example auth provider'
                                          ' is for testing only.')
-                })]),
+                })],
+                _no_duplicate_auth_provider),
     vol.Optional(CONF_AUTH_MFA_MODULES):
         vol.All(cv.ensure_list,
-                [auth_mfa_modules.MULTI_FACTOR_AUTH_MODULE_SCHEMA]),
+                [auth_mfa_modules.MULTI_FACTOR_AUTH_MODULE_SCHEMA.extend({
+                    CONF_TYPE: vol.NotIn(['insecure_example'],
+                                         'The insecure_example mfa module'
+                                         ' is for testing only.')
+                })],
+                _no_duplicate_auth_mfa_module),
 })
 
 

tests/auth/providers/test_homeassistant.py

@@ -3,6 +3,7 @@
 
 import base64
 import pytest
+import voluptuous as vol
 
 from homeassistant import data_entry_flow
 from homeassistant.auth import auth_manager_from_config, auth_store
@@ -111,11 +112,11 @@ async def test_saving_loading(data, hass):
 async def test_not_allow_set_id():
     """Test we are not allowed to set an ID in config."""
     hass = Mock()
-    provider = await auth_provider_from_config(hass, None, {
-        'type': 'homeassistant',
-        'id': 'invalid',
-    })
-    assert provider is None
+    with pytest.raises(vol.Invalid):
+        await auth_provider_from_config(hass, None, {
+            'type': 'homeassistant',
+            'id': 'invalid',
+        })
 
 
 async def test_new_users_populate_values(hass, data):