From 634888473559a0d78ab3c6cf7383e58f24398cca Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Wed, 26 Aug 2020 23:37:33 +0200 Subject: [PATCH] Allow passing in user id instead of username to change password (#39266) --- .../config/auth_provider_homeassistant.py | 85 +++++++++---------- .../test_auth_provider_homeassistant.py | 39 +++++++-- 2 files changed, 75 insertions(+), 49 deletions(-) diff --git a/homeassistant/components/config/auth_provider_homeassistant.py b/homeassistant/components/config/auth_provider_homeassistant.py index 696d215f68b..44ac6f23e2d 100644 --- a/homeassistant/components/config/auth_provider_homeassistant.py +++ b/homeassistant/components/config/auth_provider_homeassistant.py @@ -34,29 +34,21 @@ async def websocket_create(hass, connection, msg): user = await hass.auth.async_get_user(msg["user_id"]) if user is None: - connection.send_message( - websocket_api.error_message(msg["id"], "not_found", "User not found") - ) + connection.send_error(msg["id"], "not_found", "User not found") return if user.system_generated: - connection.send_message( - websocket_api.error_message( - msg["id"], - "system_generated", - "Cannot add credentials to a system generated user.", - ) + connection.send_error( + msg["id"], + "system_generated", + "Cannot add credentials to a system generated user.", ) return try: await provider.async_add_auth(msg["username"], msg["password"]) except auth_ha.InvalidUser: - connection.send_message( - websocket_api.error_message( - msg["id"], "username_exists", "Username already exists" - ) - ) + connection.send_error(msg["id"], "username_exists", "Username already exists") return credentials = await provider.async_get_or_create_credentials( @@ -64,7 +56,7 @@ async def websocket_create(hass, connection, msg): ) await hass.auth.async_link_user(user, credentials) - connection.send_message(websocket_api.result_message(msg["id"])) + connection.send_result(msg["id"]) @decorators.websocket_command( @@ -87,20 +79,18 @@ async def websocket_delete(hass, connection, msg): if not credentials.is_new: await hass.auth.async_remove_credentials(credentials) - connection.send_message(websocket_api.result_message(msg["id"])) + connection.send_result(msg["id"]) return try: await provider.async_remove_auth(msg["username"]) except auth_ha.InvalidUser: - connection.send_message( - websocket_api.error_message( - msg["id"], "auth_not_found", "Given username was not found." - ) + connection.send_error( + msg["id"], "auth_not_found", "Given username was not found." ) return - connection.send_message(websocket_api.result_message(msg["id"])) + connection.send_result(msg["id"]) @decorators.websocket_command( @@ -115,9 +105,7 @@ async def websocket_change_password(hass, connection, msg): """Change current user password.""" user = connection.user if user is None: - connection.send_message( - websocket_api.error_message(msg["id"], "user_not_found", "User not found") - ) + connection.send_error(msg["id"], "user_not_found", "User not found") return provider = auth_ha.async_get_provider(hass) @@ -128,26 +116,20 @@ async def websocket_change_password(hass, connection, msg): break if username is None: - connection.send_message( - websocket_api.error_message( - msg["id"], "credentials_not_found", "Credentials not found" - ) + connection.send_error( + msg["id"], "credentials_not_found", "Credentials not found" ) return try: await provider.async_validate_login(username, msg["current_password"]) except auth_ha.InvalidAuth: - connection.send_message( - websocket_api.error_message( - msg["id"], "invalid_password", "Invalid password" - ) - ) + connection.send_error(msg["id"], "invalid_password", "Invalid password") return await provider.async_change_password(username, msg["new_password"]) - connection.send_message(websocket_api.result_message(msg["id"])) + connection.send_result(msg["id"]) @decorators.websocket_command( @@ -155,7 +137,7 @@ async def websocket_change_password(hass, connection, msg): vol.Required( "type" ): "config/auth_provider/homeassistant/admin_change_password", - vol.Required("username"): str, + vol.Required("user_id"): str, vol.Required("password"): str, } ) @@ -166,14 +148,31 @@ async def websocket_admin_change_password(hass, connection, msg): if not connection.user.is_owner: raise Unauthorized(context=connection.context(msg)) + user = await hass.auth.async_get_user(msg["user_id"]) + + if user is None: + connection.send_error(msg["id"], "user_not_found", "User not found") + return + 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"])) - except auth_ha.InvalidUser: - connection.send_message( - websocket_api.error_message( - msg["id"], "credentials_not_found", "Credentials not found" - ) + + username = None + for credential in user.credentials: + if credential.auth_provider_type == provider.type: + username = credential.data["username"] + break + + if username is None: + connection.send_error( + msg["id"], "credentials_not_found", "Credentials not found" + ) + return + + try: + await provider.async_change_password(username, msg["password"]) + connection.send_result(msg["id"]) + except auth_ha.InvalidUser: + connection.send_error( + msg["id"], "credentials_not_found", "Credentials not found" ) return diff --git a/tests/components/config/test_auth_provider_homeassistant.py b/tests/components/config/test_auth_provider_homeassistant.py index ab0682e8eaa..19568ff450b 100644 --- a/tests/components/config/test_auth_provider_homeassistant.py +++ b/tests/components/config/test_auth_provider_homeassistant.py @@ -323,7 +323,7 @@ async def test_admin_change_password_not_owner( { "id": 6, "type": "config/auth_provider/homeassistant/admin_change_password", - "username": "test-user", + "user_id": "test-user", "password": "new-pass", } ) @@ -336,15 +336,35 @@ async def test_admin_change_password_not_owner( await auth_provider.async_validate_login("test-user", "test-pass") -async def test_admin_change_password_no_creds(hass, hass_ws_client, owner_access_token): - """Test that change password fails with unknown credentials.""" +async def test_admin_change_password_no_user(hass, hass_ws_client, owner_access_token): + """Test that change password fails with unknown user.""" client = await hass_ws_client(hass, owner_access_token) await client.send_json( { "id": 6, "type": "config/auth_provider/homeassistant/admin_change_password", - "username": "non-existing", + "user_id": "non-existing", + "password": "new-pass", + } + ) + + result = await client.receive_json() + assert not result["success"], result + assert result["error"]["code"] == "user_not_found" + + +async def test_admin_change_password_no_cred( + hass, hass_ws_client, owner_access_token, hass_admin_user +): + """Test that change password fails with unknown credential.""" + client = await hass_ws_client(hass, owner_access_token) + + await client.send_json( + { + "id": 6, + "type": "config/auth_provider/homeassistant/admin_change_password", + "user_id": hass_admin_user.id, "password": "new-pass", } ) @@ -355,16 +375,23 @@ async def test_admin_change_password_no_creds(hass, hass_ws_client, owner_access async def test_admin_change_password( - hass, hass_ws_client, owner_access_token, auth_provider, test_user_credential + hass, + hass_ws_client, + owner_access_token, + auth_provider, + test_user_credential, + hass_admin_user, ): """Test that owners can change any password.""" + await hass.auth.async_link_user(hass_admin_user, test_user_credential) + client = await hass_ws_client(hass, owner_access_token) await client.send_json( { "id": 6, "type": "config/auth_provider/homeassistant/admin_change_password", - "username": "test-user", + "user_id": hass_admin_user.id, "password": "new-pass", } )