From 9979e465aa39627f3710cc8365eb53994536f822 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 25 Aug 2020 14:22:50 +0200 Subject: [PATCH] Fix hassio auth data (#39244) Co-authored-by: Pascal Vizeli --- homeassistant/auth/providers/homeassistant.py | 3 +- .../config/auth_provider_homeassistant.py | 8 +- homeassistant/components/hassio/auth.py | 88 ++++++++----------- tests/components/hassio/test_auth.py | 62 ++++++------- 4 files changed, 66 insertions(+), 95 deletions(-) diff --git a/homeassistant/auth/providers/homeassistant.py b/homeassistant/auth/providers/homeassistant.py index cd10cf7cf95..70e2f5403cd 100644 --- a/homeassistant/auth/providers/homeassistant.py +++ b/homeassistant/auth/providers/homeassistant.py @@ -30,7 +30,8 @@ def _disallow_id(conf: Dict[str, Any]) -> Dict[str, Any]: CONFIG_SCHEMA = vol.All(AUTH_PROVIDER_SCHEMA, _disallow_id) -async def async_get_provider(hass: HomeAssistant) -> "HassAuthProvider": +@callback +def async_get_provider(hass: HomeAssistant) -> "HassAuthProvider": """Get the provider.""" for prv in hass.auth.auth_providers: if prv.type == "homeassistant": diff --git a/homeassistant/components/config/auth_provider_homeassistant.py b/homeassistant/components/config/auth_provider_homeassistant.py index 1eb410e3c98..696d215f68b 100644 --- a/homeassistant/components/config/auth_provider_homeassistant.py +++ b/homeassistant/components/config/auth_provider_homeassistant.py @@ -30,7 +30,7 @@ async def async_setup(hass): @websocket_api.async_response async def websocket_create(hass, connection, msg): """Create credentials and attach to a user.""" - provider = await auth_ha.async_get_provider(hass) + provider = auth_ha.async_get_provider(hass) user = await hass.auth.async_get_user(msg["user_id"]) if user is None: @@ -77,7 +77,7 @@ async def websocket_create(hass, connection, msg): @websocket_api.async_response async def websocket_delete(hass, connection, msg): """Delete username and related credential.""" - provider = await auth_ha.async_get_provider(hass) + provider = auth_ha.async_get_provider(hass) credentials = await provider.async_get_or_create_credentials( {"username": msg["username"]} ) @@ -120,7 +120,7 @@ async def websocket_change_password(hass, connection, msg): ) return - provider = await auth_ha.async_get_provider(hass) + provider = auth_ha.async_get_provider(hass) username = None for credential in user.credentials: if credential.auth_provider_type == provider.type: @@ -166,7 +166,7 @@ async def websocket_admin_change_password(hass, connection, msg): if not connection.user.is_owner: raise Unauthorized(context=connection.context(msg)) - provider = await auth_ha.async_get_provider(hass) + provider = auth_ha.async_get_provider(hass) try: await provider.async_change_password(msg["username"], msg["password"]) connection.send_message(websocket_api.result_message(msg["id"])) diff --git a/homeassistant/components/hassio/auth.py b/homeassistant/components/hassio/auth.py index 48f0abd6617..fb2b1dc757c 100644 --- a/homeassistant/components/hassio/auth.py +++ b/homeassistant/components/hassio/auth.py @@ -4,20 +4,16 @@ import logging import os from aiohttp import web -from aiohttp.web_exceptions import ( - HTTPInternalServerError, - HTTPNotFound, - HTTPUnauthorized, -) +from aiohttp.web_exceptions import HTTPNotFound, HTTPUnauthorized import voluptuous as vol from homeassistant.auth.models import User +from homeassistant.auth.providers import homeassistant as auth_ha from homeassistant.components.http import HomeAssistantView from homeassistant.components.http.const import KEY_HASS_USER from homeassistant.components.http.data_validator import RequestDataValidator from homeassistant.const import HTTP_OK from homeassistant.core import callback -from homeassistant.exceptions import HomeAssistantError import homeassistant.helpers.config_validation as cv from homeassistant.helpers.typing import HomeAssistantType @@ -26,21 +22,6 @@ from .const import ATTR_ADDON, ATTR_PASSWORD, ATTR_USERNAME _LOGGER = logging.getLogger(__name__) -SCHEMA_API_AUTH = vol.Schema( - { - vol.Required(ATTR_USERNAME): cv.string, - vol.Required(ATTR_PASSWORD): cv.string, - vol.Required(ATTR_ADDON): cv.string, - }, - extra=vol.ALLOW_EXTRA, -) - -SCHEMA_API_PASSWORD_RESET = vol.Schema( - {vol.Required(ATTR_USERNAME): cv.string, vol.Required(ATTR_PASSWORD): cv.string}, - extra=vol.ALLOW_EXTRA, -) - - @callback def async_setup_auth_view(hass: HomeAssistantType, user: User): """Auth setup.""" @@ -74,15 +55,6 @@ class HassIOBaseAuth(HomeAssistantView): _LOGGER.error("Invalid auth request from %s", request[KEY_HASS_USER].name) raise HTTPUnauthorized() - def _get_provider(self): - """Return Homeassistant auth provider.""" - prv = self.hass.auth.get_auth_provider("homeassistant", None) - if prv is not None: - return prv - - _LOGGER.error("Can't find Home Assistant auth") - raise HTTPNotFound() - class HassIOAuth(HassIOBaseAuth): """Hass.io view to handle auth requests.""" @@ -90,23 +62,30 @@ class HassIOAuth(HassIOBaseAuth): name = "api:hassio:auth" url = "/api/hassio_auth" - @RequestDataValidator(SCHEMA_API_AUTH) + @RequestDataValidator( + vol.Schema( + { + vol.Required(ATTR_USERNAME): cv.string, + vol.Required(ATTR_PASSWORD): cv.string, + vol.Required(ATTR_ADDON): cv.string, + }, + extra=vol.ALLOW_EXTRA, + ) + ) async def post(self, request, data): """Handle auth requests.""" self._check_access(request) - - await self._check_login(data[ATTR_USERNAME], data[ATTR_PASSWORD]) - return web.Response(status=HTTP_OK) - - async def _check_login(self, username, password): - """Check User credentials.""" - provider = self._get_provider() + provider = auth_ha.async_get_provider(request.app["hass"]) try: - await provider.async_validate_login(username, password) - except HomeAssistantError: + await provider.async_validate_login( + data[ATTR_USERNAME], data[ATTR_PASSWORD] + ) + except auth_ha.InvalidAuth: raise HTTPUnauthorized() from None + return web.Response(status=HTTP_OK) + class HassIOPasswordReset(HassIOBaseAuth): """Hass.io view to handle password reset requests.""" @@ -114,22 +93,25 @@ class HassIOPasswordReset(HassIOBaseAuth): name = "api:hassio:auth:password:reset" url = "/api/hassio_auth/password_reset" - @RequestDataValidator(SCHEMA_API_PASSWORD_RESET) + @RequestDataValidator( + vol.Schema( + { + vol.Required(ATTR_USERNAME): cv.string, + vol.Required(ATTR_PASSWORD): cv.string, + }, + extra=vol.ALLOW_EXTRA, + ) + ) async def post(self, request, data): """Handle password reset requests.""" self._check_access(request) - - await self._change_password(data[ATTR_USERNAME], data[ATTR_PASSWORD]) - return web.Response(status=HTTP_OK) - - async def _change_password(self, username, password): - """Check User credentials.""" - provider = self._get_provider() + provider = auth_ha.async_get_provider(request.app["hass"]) try: - await self.hass.async_add_executor_job( - provider.data.change_password, username, password + await provider.async_change_password( + data[ATTR_USERNAME], data[ATTR_PASSWORD] ) - await provider.data.async_save() - except HomeAssistantError: - raise HTTPInternalServerError() + except auth_ha.InvalidUser: + raise HTTPNotFound() + + return web.Response(status=HTTP_OK) diff --git a/tests/components/hassio/test_auth.py b/tests/components/hassio/test_auth.py index e97c5bc66fb..c5ac9df74b7 100644 --- a/tests/components/hassio/test_auth.py +++ b/tests/components/hassio/test_auth.py @@ -1,7 +1,6 @@ """The tests for the hassio component.""" -from homeassistant.const import HTTP_INTERNAL_SERVER_ERROR -from homeassistant.exceptions import HomeAssistantError +from homeassistant.auth.providers.homeassistant import InvalidAuth from tests.async_mock import Mock, patch @@ -59,7 +58,7 @@ async def test_login_error(hass, hassio_client_supervisor): with patch( "homeassistant.auth.providers.homeassistant." "HassAuthProvider.async_validate_login", - Mock(side_effect=HomeAssistantError()), + Mock(side_effect=InvalidAuth()), ) as mock_login: resp = await hassio_client_supervisor.post( "/api/hassio_auth", @@ -76,7 +75,7 @@ async def test_login_no_data(hass, hassio_client_supervisor): with patch( "homeassistant.auth.providers.homeassistant." "HassAuthProvider.async_validate_login", - Mock(side_effect=HomeAssistantError()), + Mock(side_effect=InvalidAuth()), ) as mock_login: resp = await hassio_client_supervisor.post("/api/hassio_auth") @@ -90,7 +89,7 @@ async def test_login_no_username(hass, hassio_client_supervisor): with patch( "homeassistant.auth.providers.homeassistant." "HassAuthProvider.async_validate_login", - Mock(side_effect=HomeAssistantError()), + Mock(side_effect=InvalidAuth()), ) as mock_login: resp = await hassio_client_supervisor.post( "/api/hassio_auth", json={"password": "123456", "addon": "samba"} @@ -125,7 +124,8 @@ async def test_login_success_extra(hass, hassio_client_supervisor): async def test_password_success(hass, hassio_client_supervisor): """Test no auth needed for .""" with patch( - "homeassistant.components.hassio.auth.HassIOPasswordReset._change_password", + "homeassistant.auth.providers.homeassistant." + "HassAuthProvider.async_change_password", ) as mock_change: resp = await hassio_client_supervisor.post( "/api/hassio_auth/password_reset", @@ -139,44 +139,32 @@ async def test_password_success(hass, hassio_client_supervisor): async def test_password_fails_no_supervisor(hass, hassio_client): """Test if only supervisor can access.""" - with patch( - "homeassistant.auth.providers.homeassistant.Data.async_save", - ) as mock_save: - resp = await hassio_client.post( - "/api/hassio_auth/password_reset", - json={"username": "test", "password": "123456"}, - ) + resp = await hassio_client.post( + "/api/hassio_auth/password_reset", + json={"username": "test", "password": "123456"}, + ) - # Check we got right response - assert resp.status == 401 - assert not mock_save.called + # Check we got right response + assert resp.status == 401 async def test_password_fails_no_auth(hass, hassio_noauth_client): """Test if only supervisor can access.""" - with patch( - "homeassistant.auth.providers.homeassistant.Data.async_save", - ) as mock_save: - resp = await hassio_noauth_client.post( - "/api/hassio_auth/password_reset", - json={"username": "test", "password": "123456"}, - ) + resp = await hassio_noauth_client.post( + "/api/hassio_auth/password_reset", + json={"username": "test", "password": "123456"}, + ) - # Check we got right response - assert resp.status == 401 - assert not mock_save.called + # Check we got right response + assert resp.status == 401 async def test_password_no_user(hass, hassio_client_supervisor): - """Test no auth needed for .""" - with patch( - "homeassistant.auth.providers.homeassistant.Data.async_save", - ) as mock_save: - resp = await hassio_client_supervisor.post( - "/api/hassio_auth/password_reset", - json={"username": "test", "password": "123456"}, - ) + """Test changing password for invalid user.""" + resp = await hassio_client_supervisor.post( + "/api/hassio_auth/password_reset", + json={"username": "test", "password": "123456"}, + ) - # Check we got right response - assert resp.status == HTTP_INTERNAL_SERVER_ERROR - assert not mock_save.called + # Check we got right response + assert resp.status == 404