From af49ba511889684c0699671668c1404c289fa3a2 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Fri, 11 Jan 2019 21:38:06 +0100 Subject: [PATCH 01/15] Added external auth provider that calls a configurable program Closes #19975 --- homeassistant/auth/providers/external.py | 142 +++++++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 homeassistant/auth/providers/external.py diff --git a/homeassistant/auth/providers/external.py b/homeassistant/auth/providers/external.py new file mode 100644 index 000000000000..bc6e52de3856 --- /dev/null +++ b/homeassistant/auth/providers/external.py @@ -0,0 +1,142 @@ +"""Auth provider that validates credentials via an external script.""" + +import typing as T + +import collections + +import asyncio.subprocess + +import voluptuous as vol + +from homeassistant.exceptions import HomeAssistantError + +from . import AuthProvider, AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, LoginFlow +from ..models import Credentials, UserMeta + + +CONF_PROGRAM = "program" +CONF_ARGS = "args" + +CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend({ + vol.Required(CONF_PROGRAM): str, + vol.Optional(CONF_ARGS, default=None): vol.Any(vol.DefaultTo(list), [str]), +}, extra=vol.PREVENT_EXTRA) + + +class InvalidAuthError(HomeAssistantError): + """Raised when submitting invalid authentication.""" + + +@AUTH_PROVIDERS.register("external") +class ExternalAuthProvider(AuthProvider): + """External auth provider validating credentials by calling a script.""" + + DEFAULT_TITLE = "External Authentication" + + # which keys to accept from a program's stdout + ALLOWED_META_KEYS = ("name",) + + def __init__(self, *args: T.Any, **kwargs: T.Any) -> None: + """Extend parent's __init__. + + Adds self._user_meta dictionary to hold the user-specific + attributes provided by external programs. + """ + super().__init__(*args, **kwargs) + self._user_meta = {} # type: T.Dict[str, T.Dict[str, T.Any]] + + async def async_login_flow(self, context: T.Optional[T.Dict]) -> LoginFlow: + """Return a flow to login.""" + return ExternalLoginFlow(self) + + async def async_validate_login(self, username: str, password: str) -> None: + """Validate a username and password.""" + env = { + "username": username, + "password": password, + } + process = await asyncio.subprocess.create_subprocess_exec( + self.config[CONF_PROGRAM], *self.config[CONF_ARGS], + env=env, + stdout=asyncio.subprocess.PIPE, + ) + stdout = (await process.communicate())[0] + + if process.returncode != 0: + raise InvalidAuthError + + meta = {} # type: T.Dict[str, str] + for _line in stdout.splitlines(): + try: + line = _line.decode().lstrip() + if line.startswith("#"): + continue + key, value = line.split("=", 1) + except ValueError: + # malformed line + continue + key = key.strip() + value = value.strip() + if key in self.ALLOWED_META_KEYS: + meta[key] = value + self._user_meta[username] = meta + + async def async_get_or_create_credentials( + self, flow_result: T.Dict[str, str] + ) -> Credentials: + """Get credentials based on the flow result.""" + username = flow_result["username"] + for credential in await self.async_credentials(): + if credential.data["username"] == username: + return credential + + # Create new credentials. + return self.async_create_credentials({ + "username": username, + }) + + async def async_user_meta_for_credentials( + self, credentials: Credentials + ) -> UserMeta: + """Return extra user metadata for credentials. + + Currently, only name is supported. + """ + meta = self._user_meta.get(credentials.data["username"], {}) + return UserMeta( + name=meta.get("name"), + is_active=True, + ) + + +class ExternalLoginFlow(LoginFlow): + """Handler for the login flow.""" + + async def async_step_init( + self, user_input: T.Optional[T.Dict[str, str]] = None + ) -> T.Dict[str, T.Any]: + """Handle the step of the form.""" + errors = {} + + if user_input is not None: + try: + await T.cast(ExternalAuthProvider, self._auth_provider) \ + .async_validate_login( + user_input["username"], user_input["password"] + ) + except InvalidAuthError: + errors["base"] = "invalid_auth" + + if not errors: + user_input.pop("password") + return await self.async_finish(user_input) + + schema = collections.OrderedDict() # type: T.Dict[str, type] + schema["username"] = str + schema["password"] = str + + return self.async_show_form( + step_id="init", + data_schema=vol.Schema(schema), + errors=errors, + ) From ba05606736f3bc93b01f72d4f7999823e7bf2518 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Fri, 11 Jan 2019 22:51:24 +0100 Subject: [PATCH 02/15] Raise proper InvalidAuth exception on OSError during program execution --- homeassistant/auth/providers/external.py | 25 +++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/homeassistant/auth/providers/external.py b/homeassistant/auth/providers/external.py index bc6e52de3856..034f8e3226d3 100644 --- a/homeassistant/auth/providers/external.py +++ b/homeassistant/auth/providers/external.py @@ -2,9 +2,9 @@ import typing as T -import collections - import asyncio.subprocess +import collections +import logging import voluptuous as vol @@ -22,9 +22,11 @@ vol.Optional(CONF_ARGS, default=None): vol.Any(vol.DefaultTo(list), [str]), }, extra=vol.PREVENT_EXTRA) +_LOGGER = logging.getLogger(__name__) + class InvalidAuthError(HomeAssistantError): - """Raised when submitting invalid authentication.""" + """Raised when authentication with given credentials fails.""" @AUTH_PROVIDERS.register("external") @@ -55,12 +57,17 @@ async def async_validate_login(self, username: str, password: str) -> None: "username": username, "password": password, } - process = await asyncio.subprocess.create_subprocess_exec( - self.config[CONF_PROGRAM], *self.config[CONF_ARGS], - env=env, - stdout=asyncio.subprocess.PIPE, - ) - stdout = (await process.communicate())[0] + try: + process = await asyncio.subprocess.create_subprocess_exec( + self.config[CONF_PROGRAM], *self.config[CONF_ARGS], + env=env, + stdout=asyncio.subprocess.PIPE, + ) + stdout = (await process.communicate())[0] + except OSError as err: + _LOGGER.error("Error while authenticating '%s': %s", + username, err) + raise InvalidAuthError if process.returncode != 0: raise InvalidAuthError From 38f9361dff1aa699e12e2e8a8b3771a14fe8928c Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 02:00:00 +0100 Subject: [PATCH 03/15] Changed name of external auth provider to command_line --- .../providers/{external.py => command_line.py} | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) rename homeassistant/auth/providers/{external.py => command_line.py} (92%) diff --git a/homeassistant/auth/providers/external.py b/homeassistant/auth/providers/command_line.py similarity index 92% rename from homeassistant/auth/providers/external.py rename to homeassistant/auth/providers/command_line.py index 034f8e3226d3..d07f15ff599b 100644 --- a/homeassistant/auth/providers/external.py +++ b/homeassistant/auth/providers/command_line.py @@ -29,11 +29,11 @@ class InvalidAuthError(HomeAssistantError): """Raised when authentication with given credentials fails.""" -@AUTH_PROVIDERS.register("external") -class ExternalAuthProvider(AuthProvider): - """External auth provider validating credentials by calling a script.""" +@AUTH_PROVIDERS.register("command_line") +class CommandLineAuthProvider(AuthProvider): + """Auth provider validating credentials by calling a command.""" - DEFAULT_TITLE = "External Authentication" + DEFAULT_TITLE = "Command Line Authentication" # which keys to accept from a program's stdout ALLOWED_META_KEYS = ("name",) @@ -49,7 +49,7 @@ def __init__(self, *args: T.Any, **kwargs: T.Any) -> None: async def async_login_flow(self, context: T.Optional[T.Dict]) -> LoginFlow: """Return a flow to login.""" - return ExternalLoginFlow(self) + return CommandLineLoginFlow(self) async def async_validate_login(self, username: str, password: str) -> None: """Validate a username and password.""" @@ -116,7 +116,7 @@ async def async_user_meta_for_credentials( ) -class ExternalLoginFlow(LoginFlow): +class CommandLineLoginFlow(LoginFlow): """Handler for the login flow.""" async def async_step_init( @@ -127,7 +127,7 @@ async def async_step_init( if user_input is not None: try: - await T.cast(ExternalAuthProvider, self._auth_provider) \ + await T.cast(CommandLineAuthProvider, self._auth_provider) \ .async_validate_login( user_input["username"], user_input["password"] ) From c3b14774a0344c870003f15d8c2819a882ae0e3d Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 10:24:49 +0100 Subject: [PATCH 04/15] Renamed program config option to command in command_line auth provider --- homeassistant/auth/providers/command_line.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index d07f15ff599b..7d52fb923176 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -5,6 +5,7 @@ import asyncio.subprocess import collections import logging +import os import voluptuous as vol @@ -14,11 +15,15 @@ from ..models import Credentials, UserMeta -CONF_PROGRAM = "program" +CONF_COMMAND = "command" CONF_ARGS = "args" CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend({ - vol.Required(CONF_PROGRAM): str, + vol.Required(CONF_COMMAND): vol.All( + str, + os.path.normpath, + msg="must be an absolute path" + ), vol.Optional(CONF_ARGS, default=None): vol.Any(vol.DefaultTo(list), [str]), }, extra=vol.PREVENT_EXTRA) @@ -59,12 +64,13 @@ async def async_validate_login(self, username: str, password: str) -> None: } try: process = await asyncio.subprocess.create_subprocess_exec( - self.config[CONF_PROGRAM], *self.config[CONF_ARGS], + self.config[CONF_COMMAND], *self.config[CONF_ARGS], env=env, stdout=asyncio.subprocess.PIPE, ) stdout = (await process.communicate())[0] except OSError as err: + # happens when command doesn't exist or permission is denied _LOGGER.error("Error while authenticating '%s': %s", username, err) raise InvalidAuthError From 5d100affd424215dc2132ed8b2c049824a148393 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 11:42:49 +0100 Subject: [PATCH 05/15] Made meta variable parsing in command_line auth provider optional --- homeassistant/auth/providers/command_line.py | 36 +++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index 7d52fb923176..048536e56a4c 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -1,4 +1,4 @@ -"""Auth provider that validates credentials via an external script.""" +"""Auth provider that validates credentials via an external command.""" import typing as T @@ -17,6 +17,7 @@ CONF_COMMAND = "command" CONF_ARGS = "args" +CONF_META = "meta" CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend({ vol.Required(CONF_COMMAND): vol.All( @@ -25,6 +26,7 @@ msg="must be an absolute path" ), vol.Optional(CONF_ARGS, default=None): vol.Any(vol.DefaultTo(list), [str]), + vol.Optional(CONF_META, default=False): bool, }, extra=vol.PREVENT_EXTRA) _LOGGER = logging.getLogger(__name__) @@ -66,7 +68,8 @@ async def async_validate_login(self, username: str, password: str) -> None: process = await asyncio.subprocess.create_subprocess_exec( self.config[CONF_COMMAND], *self.config[CONF_ARGS], env=env, - stdout=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE + if self.config[CONF_META] else None, ) stdout = (await process.communicate())[0] except OSError as err: @@ -78,21 +81,22 @@ async def async_validate_login(self, username: str, password: str) -> None: if process.returncode != 0: raise InvalidAuthError - meta = {} # type: T.Dict[str, str] - for _line in stdout.splitlines(): - try: - line = _line.decode().lstrip() - if line.startswith("#"): + if self.config[CONF_META]: + meta = {} # type: T.Dict[str, str] + for _line in stdout.splitlines(): + try: + line = _line.decode().lstrip() + if line.startswith("#"): + continue + key, value = line.split("=", 1) + except ValueError: + # malformed line continue - key, value = line.split("=", 1) - except ValueError: - # malformed line - continue - key = key.strip() - value = value.strip() - if key in self.ALLOWED_META_KEYS: - meta[key] = value - self._user_meta[username] = meta + key = key.strip() + value = value.strip() + if key in self.ALLOWED_META_KEYS: + meta[key] = value + self._user_meta[username] = meta async def async_get_or_create_credentials( self, flow_result: T.Dict[str, str] From 06c1355598b4aba3a8f6b6f6fe024d3c7585a520 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 11:47:29 +0100 Subject: [PATCH 06/15] Added tests for command_line auth provider --- tests/auth/providers/test_command_line.py | 115 ++++++++++++++++++ tests/auth/providers/test_command_line_cmd.sh | 12 ++ 2 files changed, 127 insertions(+) create mode 100644 tests/auth/providers/test_command_line.py create mode 100755 tests/auth/providers/test_command_line_cmd.sh diff --git a/tests/auth/providers/test_command_line.py b/tests/auth/providers/test_command_line.py new file mode 100644 index 000000000000..b8105ff1d8f6 --- /dev/null +++ b/tests/auth/providers/test_command_line.py @@ -0,0 +1,115 @@ +"""Tests for the command_line auth provider.""" + +from unittest.mock import Mock +import os +import uuid + +import pytest + +from homeassistant.auth import auth_store, models as auth_models, AuthManager +from homeassistant.auth.providers import command_line +from homeassistant.const import CONF_TYPE + +from tests.common import mock_coro + + +@pytest.fixture +def store(hass): + """Mock store.""" + return auth_store.AuthStore(hass) + + +@pytest.fixture +def provider(hass, store): + """Mock provider.""" + return command_line.CommandLineAuthProvider(hass, store, { + CONF_TYPE: "command_line", + command_line.CONF_COMMAND: os.path.join( + os.path.dirname(__file__), "test_command_line_cmd.sh" + ), + command_line.CONF_ARGS: [], + command_line.CONF_META: False, + }) + + +@pytest.fixture +def manager(hass, store, provider): + """Mock manager.""" + return AuthManager(hass, store, { + (provider.type, provider.id): provider + }, {}) + + +async def test_create_new_credential(manager, provider): + """Test that we create a new credential.""" + credentials = await provider.async_get_or_create_credentials({ + "username": "good-user", + "password": "good-pass", + }) + assert credentials.is_new is True + + user = await manager.async_get_or_create_user(credentials) + assert user.is_active + + +async def test_match_existing_credentials(store, provider): + """See if we match existing users.""" + existing = auth_models.Credentials( + id=uuid.uuid4(), + auth_provider_type="command_line", + auth_provider_id=None, + data={ + "username": "good-user" + }, + is_new=False, + ) + provider.async_credentials = Mock(return_value=mock_coro([existing])) + credentials = await provider.async_get_or_create_credentials({ + "username": "good-user", + "password": "irrelevant", + }) + assert credentials is existing + + +async def test_invalid_username(provider): + """Test we raise if incorrect user specified.""" + with pytest.raises(command_line.InvalidAuthError): + await provider.async_validate_login("bad-user", "good-pass") + + +async def test_invalid_password(provider): + """Test we raise if incorrect password specified.""" + with pytest.raises(command_line.InvalidAuthError): + await provider.async_validate_login("good-user", "bad-pass") + + +async def test_good_auth(provider): + """Test nothing is raised with good credentials.""" + await provider.async_validate_login("good-user", "good-pass") + + +async def test_good_auth_with_meta(manager, provider): + """Test metadata is added upon successful authentication.""" + provider.config[command_line.CONF_ARGS] = ["--with-meta"] + provider.config[command_line.CONF_META] = True + + await provider.async_validate_login("good-user", "good-pass") + + credentials = await provider.async_get_or_create_credentials({ + "username": "good-user", + "password": "good-pass", + }) + assert credentials.is_new is True + + user = await manager.async_get_or_create_user(credentials) + assert user.name == "Bob" + assert user.is_active + + +async def test_utf_8_username_password(provider): + """Test that we create a new credential.""" + credentials = await provider.async_get_or_create_credentials({ + "username": "ßßß", + "password": "äöü", + }) + assert credentials.is_new is True diff --git a/tests/auth/providers/test_command_line_cmd.sh b/tests/auth/providers/test_command_line_cmd.sh new file mode 100755 index 000000000000..0e689e338f1d --- /dev/null +++ b/tests/auth/providers/test_command_line_cmd.sh @@ -0,0 +1,12 @@ +#!/bin/sh + +if [ "$username" = "good-user" ] && [ "$password" = "good-pass" ]; then + echo "Auth should succeed." >&2 + if [ "$1" = "--with-meta" ]; then + echo "name=Bob" + fi + exit 0 +fi + +echo "Auth should fail." >&2 +exit 1 From 9c67785e7dbc2a4d02624b20a1ae2c9bcba90a7e Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 12:55:28 +0100 Subject: [PATCH 07/15] Fixed indentation --- homeassistant/auth/providers/command_line.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index 048536e56a4c..7b56a5b2fb41 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -69,7 +69,7 @@ async def async_validate_login(self, username: str, password: str) -> None: self.config[CONF_COMMAND], *self.config[CONF_ARGS], env=env, stdout=asyncio.subprocess.PIPE - if self.config[CONF_META] else None, + if self.config[CONF_META] else None, ) stdout = (await process.communicate())[0] except OSError as err: From a1c7a8dada3b2ce926520bfadc5d8bfb2289b63c Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 21:44:34 +0100 Subject: [PATCH 08/15] Suppressed wrong pylint warning --- homeassistant/auth/providers/command_line.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index 7b56a5b2fb41..f2e9d9bcb6c2 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -65,7 +65,7 @@ async def async_validate_login(self, username: str, password: str) -> None: "password": password, } try: - process = await asyncio.subprocess.create_subprocess_exec( + process = await asyncio.subprocess.create_subprocess_exec( # pylint: disable=no-member self.config[CONF_COMMAND], *self.config[CONF_ARGS], env=env, stdout=asyncio.subprocess.PIPE From c6c64aa44b1666a9d43569b19c4a692989187633 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Sat, 12 Jan 2019 21:47:07 +0100 Subject: [PATCH 09/15] Fixed linting --- homeassistant/auth/providers/command_line.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index f2e9d9bcb6c2..03f520949eac 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -65,7 +65,8 @@ async def async_validate_login(self, username: str, password: str) -> None: "password": password, } try: - process = await asyncio.subprocess.create_subprocess_exec( # pylint: disable=no-member + # pylint: disable=no-member + process = await asyncio.subprocess.create_subprocess_exec( self.config[CONF_COMMAND], *self.config[CONF_ARGS], env=env, stdout=asyncio.subprocess.PIPE From 458274e2eea8d615341c1e98ca046c629912c090 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Fri, 25 Jan 2019 13:40:43 +0100 Subject: [PATCH 10/15] Added test for command line auth provider login flow --- tests/auth/providers/test_command_line.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/auth/providers/test_command_line.py b/tests/auth/providers/test_command_line.py index b8105ff1d8f6..0a89ee8ae895 100644 --- a/tests/auth/providers/test_command_line.py +++ b/tests/auth/providers/test_command_line.py @@ -6,6 +6,7 @@ import pytest +from homeassistant import data_entry_flow from homeassistant.auth import auth_store, models as auth_models, AuthManager from homeassistant.auth.providers import command_line from homeassistant.const import CONF_TYPE @@ -113,3 +114,24 @@ async def test_utf_8_username_password(provider): "password": "äöü", }) assert credentials.is_new is True + + +async def test_login_flow_validates(provider): + """Test login flow.""" + flow = await provider.async_login_flow({}) + result = await flow.async_step_init() + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + + result = await flow.async_step_init({ + "username": "bad-user", + "password": "bad-pass", + }) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result['errors']["base"] == "invalid_auth" + + result = await flow.async_step_init({ + "username": "good-user", + "password": "good-pass", + }) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["username"] == "good-user" From 8c8c2570e52e7a6d9a2d2b5b41c636332554f8e0 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Fri, 25 Jan 2019 14:09:12 +0100 Subject: [PATCH 11/15] Log error when user fails authentication --- homeassistant/auth/providers/command_line.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index 03f520949eac..ff7f8642481b 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -75,11 +75,14 @@ async def async_validate_login(self, username: str, password: str) -> None: stdout = (await process.communicate())[0] except OSError as err: # happens when command doesn't exist or permission is denied - _LOGGER.error("Error while authenticating '%s': %s", - username, err) + _LOGGER.error("Error while authenticating %s: %s", + repr(username), err) raise InvalidAuthError if process.returncode != 0: + _LOGGER.error("User %s failed to authenticate, command exited " + "with code %d.", + repr(username), process.returncode) raise InvalidAuthError if self.config[CONF_META]: From 30facbf48434c4f4b733667c299769de8f9b6c6f Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Fri, 25 Jan 2019 20:24:26 +0100 Subject: [PATCH 12/15] Use %r formatter instead of explicit repr() --- homeassistant/auth/providers/command_line.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index ff7f8642481b..ebefc3535723 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -75,14 +75,14 @@ async def async_validate_login(self, username: str, password: str) -> None: stdout = (await process.communicate())[0] except OSError as err: # happens when command doesn't exist or permission is denied - _LOGGER.error("Error while authenticating %s: %s", - repr(username), err) + _LOGGER.error("Error while authenticating %r: %s", + username, err) raise InvalidAuthError if process.returncode != 0: - _LOGGER.error("User %s failed to authenticate, command exited " + _LOGGER.error("User %r failed to authenticate, command exited " "with code %d.", - repr(username), process.returncode) + username, process.returncode) raise InvalidAuthError if self.config[CONF_META]: From 8a339647a843493911670d4300a8c11151a7ae57 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Fri, 25 Jan 2019 20:29:34 +0100 Subject: [PATCH 13/15] Mix all used names of typing module into module namespace I consider this nasty and bad coding style, but was requested by @awarecan for consistency with the remaining codebase. --- homeassistant/auth/providers/command_line.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index ebefc3535723..e6e82305dcf8 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -1,6 +1,6 @@ """Auth provider that validates credentials via an external command.""" -import typing as T +from typing import Any, Dict, Optional, cast import asyncio.subprocess import collections @@ -45,16 +45,16 @@ class CommandLineAuthProvider(AuthProvider): # which keys to accept from a program's stdout ALLOWED_META_KEYS = ("name",) - def __init__(self, *args: T.Any, **kwargs: T.Any) -> None: + def __init__(self, *args: Any, **kwargs: Any) -> None: """Extend parent's __init__. Adds self._user_meta dictionary to hold the user-specific attributes provided by external programs. """ super().__init__(*args, **kwargs) - self._user_meta = {} # type: T.Dict[str, T.Dict[str, T.Any]] + self._user_meta = {} # type: Dict[str, Dict[str, Any]] - async def async_login_flow(self, context: T.Optional[T.Dict]) -> LoginFlow: + async def async_login_flow(self, context: Optional[dict]) -> LoginFlow: """Return a flow to login.""" return CommandLineLoginFlow(self) @@ -86,7 +86,7 @@ async def async_validate_login(self, username: str, password: str) -> None: raise InvalidAuthError if self.config[CONF_META]: - meta = {} # type: T.Dict[str, str] + meta = {} # type: Dict[str, str] for _line in stdout.splitlines(): try: line = _line.decode().lstrip() @@ -103,7 +103,7 @@ async def async_validate_login(self, username: str, password: str) -> None: self._user_meta[username] = meta async def async_get_or_create_credentials( - self, flow_result: T.Dict[str, str] + self, flow_result: Dict[str, str] ) -> Credentials: """Get credentials based on the flow result.""" username = flow_result["username"] @@ -134,14 +134,14 @@ class CommandLineLoginFlow(LoginFlow): """Handler for the login flow.""" async def async_step_init( - self, user_input: T.Optional[T.Dict[str, str]] = None - ) -> T.Dict[str, T.Any]: + self, user_input: Optional[Dict[str, str]] = None + ) -> Dict[str, Any]: """Handle the step of the form.""" errors = {} if user_input is not None: try: - await T.cast(CommandLineAuthProvider, self._auth_provider) \ + await cast(CommandLineAuthProvider, self._auth_provider) \ .async_validate_login( user_input["username"], user_input["password"] ) @@ -152,7 +152,7 @@ async def async_step_init( user_input.pop("password") return await self.async_finish(user_input) - schema = collections.OrderedDict() # type: T.Dict[str, type] + schema = collections.OrderedDict() # type: Dict[str, type] schema["username"] = str schema["password"] = str From bc36d17fddeca0c658a1971e879d8f0da5361a40 Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Mon, 28 Jan 2019 10:38:46 +0100 Subject: [PATCH 14/15] Small code style change --- homeassistant/auth/providers/command_line.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index e6e82305dcf8..680aaa2c407c 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -72,7 +72,7 @@ async def async_validate_login(self, username: str, password: str) -> None: stdout=asyncio.subprocess.PIPE if self.config[CONF_META] else None, ) - stdout = (await process.communicate())[0] + stdout, _ = (await process.communicate()) except OSError as err: # happens when command doesn't exist or permission is denied _LOGGER.error("Error while authenticating %r: %s", From f157848247a51d8a1bdb5de64af48e8f4792433b Mon Sep 17 00:00:00 2001 From: Robert Schindler Date: Mon, 4 Feb 2019 10:18:47 +0100 Subject: [PATCH 15/15] Strip usernames with command_line auth provider --- homeassistant/auth/providers/command_line.py | 1 + tests/auth/providers/test_command_line.py | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/homeassistant/auth/providers/command_line.py b/homeassistant/auth/providers/command_line.py index 680aaa2c407c..9cec34c13407 100644 --- a/homeassistant/auth/providers/command_line.py +++ b/homeassistant/auth/providers/command_line.py @@ -140,6 +140,7 @@ async def async_step_init( errors = {} if user_input is not None: + user_input["username"] = user_input["username"].strip() try: await cast(CommandLineAuthProvider, self._auth_provider) \ .async_validate_login( diff --git a/tests/auth/providers/test_command_line.py b/tests/auth/providers/test_command_line.py index 0a89ee8ae895..f22958e7e384 100644 --- a/tests/auth/providers/test_command_line.py +++ b/tests/auth/providers/test_command_line.py @@ -135,3 +135,14 @@ async def test_login_flow_validates(provider): }) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["data"]["username"] == "good-user" + + +async def test_strip_username(provider): + """Test authentication works with username with whitespace around.""" + flow = await provider.async_login_flow({}) + result = await flow.async_step_init({ + "username": "\t\ngood-user ", + "password": "good-pass", + }) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["username"] == "good-user"