From 59878ea1ef94c870368a73285b6c86a2b2c0fdc1 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 16 Aug 2022 17:17:10 -0400 Subject: [PATCH] Indieauth updates (#76880) --- homeassistant/components/auth/__init__.py | 58 +++++++++------- homeassistant/components/auth/login_flow.py | 53 ++++++++++++--- tests/components/auth/test_init.py | 23 +++++-- tests/components/auth/test_init_link_user.py | 7 +- tests/components/auth/test_login_flow.py | 70 +++++++++++++++++++- 5 files changed, 169 insertions(+), 42 deletions(-) diff --git a/homeassistant/components/auth/__init__.py b/homeassistant/components/auth/__init__.py index a8c7019030f..ac5035de645 100644 --- a/homeassistant/components/auth/__init__.py +++ b/homeassistant/components/auth/__init__.py @@ -177,6 +177,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: hass.data[DOMAIN] = store_result hass.http.register_view(TokenView(retrieve_result)) + hass.http.register_view(RevokeTokenView()) hass.http.register_view(LinkUserView(retrieve_result)) hass.http.register_view(OAuth2AuthorizeCallbackView()) @@ -192,8 +193,37 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: return True +class RevokeTokenView(HomeAssistantView): + """View to revoke tokens.""" + + url = "/auth/revoke" + name = "api:auth:revocation" + requires_auth = False + cors_allowed = True + + async def post(self, request: web.Request) -> web.Response: + """Revoke a token.""" + hass: HomeAssistant = request.app["hass"] + data = cast(MultiDictProxy[str], await request.post()) + + # OAuth 2.0 Token Revocation [RFC7009] + # 2.2 The authorization server responds with HTTP status code 200 + # if the token has been revoked successfully or if the client + # submitted an invalid token. + if (token := data.get("token")) is None: + return web.Response(status=HTTPStatus.OK) + + refresh_token = await hass.auth.async_get_refresh_token_by_token(token) + + if refresh_token is None: + return web.Response(status=HTTPStatus.OK) + + await hass.auth.async_remove_refresh_token(refresh_token) + return web.Response(status=HTTPStatus.OK) + + class TokenView(HomeAssistantView): - """View to issue or revoke tokens.""" + """View to issue tokens.""" url = "/auth/token" name = "api:auth:token" @@ -217,7 +247,9 @@ class TokenView(HomeAssistantView): # The revocation request includes an additional parameter, # action=revoke. if data.get("action") == "revoke": - return await self._async_handle_revoke_token(hass, data) + # action=revoke is deprecated. Use /auth/revoke instead. + # Keep here for backwards compat + return await RevokeTokenView.post(self, request) # type: ignore[arg-type] if grant_type == "authorization_code": return await self._async_handle_auth_code(hass, data, request.remote) @@ -229,28 +261,6 @@ class TokenView(HomeAssistantView): {"error": "unsupported_grant_type"}, status_code=HTTPStatus.BAD_REQUEST ) - async def _async_handle_revoke_token( - self, - hass: HomeAssistant, - data: MultiDictProxy[str], - ) -> web.Response: - """Handle revoke token request.""" - - # OAuth 2.0 Token Revocation [RFC7009] - # 2.2 The authorization server responds with HTTP status code 200 - # if the token has been revoked successfully or if the client - # submitted an invalid token. - if (token := data.get("token")) is None: - return web.Response(status=HTTPStatus.OK) - - refresh_token = await hass.auth.async_get_refresh_token_by_token(token) - - if refresh_token is None: - return web.Response(status=HTTPStatus.OK) - - await hass.auth.async_remove_refresh_token(refresh_token) - return web.Response(status=HTTPStatus.OK) - async def _async_handle_auth_code( self, hass: HomeAssistant, diff --git a/homeassistant/components/auth/login_flow.py b/homeassistant/components/auth/login_flow.py index dc094cd581f..bb13431bfa7 100644 --- a/homeassistant/components/auth/login_flow.py +++ b/homeassistant/components/auth/login_flow.py @@ -101,11 +101,32 @@ async def async_setup( hass: HomeAssistant, store_result: Callable[[str, Credentials], str] ) -> None: """Component to allow users to login.""" + hass.http.register_view(WellKnownOAuthInfoView) hass.http.register_view(AuthProvidersView) hass.http.register_view(LoginFlowIndexView(hass.auth.login_flow, store_result)) hass.http.register_view(LoginFlowResourceView(hass.auth.login_flow, store_result)) +class WellKnownOAuthInfoView(HomeAssistantView): + """View to host the OAuth2 information.""" + + requires_auth = False + url = "/.well-known/oauth-authorization-server" + name = "well-known/oauth-authorization-server" + + async def get(self, request: web.Request) -> web.Response: + """Return the well known OAuth2 authorization info.""" + return self.json( + { + "authorization_endpoint": "/auth/authorize", + "token_endpoint": "/auth/token", + "revocation_endpoint": "/auth/revoke", + "response_types_supported": ["code"], + "service_documentation": "https://developers.home-assistant.io/docs/auth_api", + } + ) + + class AuthProvidersView(HomeAssistantView): """View to get available auth providers.""" @@ -172,6 +193,7 @@ class LoginFlowBaseView(HomeAssistantView): self, request: web.Request, client_id: str, + redirect_uri: str, result: data_entry_flow.FlowResult, ) -> web.Response: """Convert the flow result to a response.""" @@ -190,9 +212,13 @@ class LoginFlowBaseView(HomeAssistantView): await process_wrong_login(request) return self.json(_prepare_result_json(result)) + hass: HomeAssistant = request.app["hass"] + + if not await indieauth.verify_redirect_uri(hass, client_id, redirect_uri): + return self.json_message("Invalid redirect URI", HTTPStatus.FORBIDDEN) + result.pop("data") - hass: HomeAssistant = request.app["hass"] result_obj: Credentials = result.pop("result") # Result can be None if credential was never linked to a user before. @@ -234,14 +260,11 @@ class LoginFlowIndexView(LoginFlowBaseView): @log_invalid_auth async def post(self, request: web.Request, data: dict[str, Any]) -> web.Response: """Create a new login flow.""" - hass: HomeAssistant = request.app["hass"] client_id: str = data["client_id"] redirect_uri: str = data["redirect_uri"] - if not await indieauth.verify_redirect_uri(hass, client_id, redirect_uri): - return self.json_message( - "invalid client id or redirect uri", HTTPStatus.BAD_REQUEST - ) + if not indieauth.verify_client_id(client_id): + return self.json_message("Invalid client id", HTTPStatus.BAD_REQUEST) handler: tuple[str, ...] | str if isinstance(data["handler"], list): @@ -264,7 +287,9 @@ class LoginFlowIndexView(LoginFlowBaseView): "Handler does not support init", HTTPStatus.BAD_REQUEST ) - return await self._async_flow_result_to_response(request, client_id, result) + return await self._async_flow_result_to_response( + request, client_id, redirect_uri, result + ) class LoginFlowResourceView(LoginFlowBaseView): @@ -277,13 +302,19 @@ class LoginFlowResourceView(LoginFlowBaseView): """Do not allow getting status of a flow in progress.""" return self.json_message("Invalid flow specified", HTTPStatus.NOT_FOUND) - @RequestDataValidator(vol.Schema({"client_id": str}, extra=vol.ALLOW_EXTRA)) + @RequestDataValidator( + vol.Schema( + {vol.Required("client_id"): str, vol.Required("redirect_uri"): str}, + extra=vol.ALLOW_EXTRA, + ) + ) @log_invalid_auth async def post( self, request: web.Request, data: dict[str, Any], flow_id: str ) -> web.Response: """Handle progressing a login flow request.""" - client_id = data.pop("client_id") + client_id: str = data.pop("client_id") + redirect_uri: str = data.pop("redirect_uri") if not indieauth.verify_client_id(client_id): return self.json_message("Invalid client id", HTTPStatus.BAD_REQUEST) @@ -299,7 +330,9 @@ class LoginFlowResourceView(LoginFlowBaseView): except vol.Invalid: return self.json_message("User input malformed", HTTPStatus.BAD_REQUEST) - return await self._async_flow_result_to_response(request, client_id, result) + return await self._async_flow_result_to_response( + request, client_id, redirect_uri, result + ) async def delete(self, request: web.Request, flow_id: str) -> web.Response: """Cancel a flow in progress.""" diff --git a/tests/components/auth/test_init.py b/tests/components/auth/test_init.py index 706221d9371..09a74cf9bc9 100644 --- a/tests/components/auth/test_init.py +++ b/tests/components/auth/test_init.py @@ -62,7 +62,12 @@ async def test_login_new_user_and_trying_refresh_token(hass, aiohttp_client): resp = await client.post( f"/auth/login_flow/{step['flow_id']}", - json={"client_id": CLIENT_ID, "username": "test-user", "password": "test-pass"}, + json={ + "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, + "username": "test-user", + "password": "test-pass", + }, ) assert resp.status == HTTPStatus.OK @@ -126,7 +131,12 @@ async def test_auth_code_checks_local_only_user(hass, aiohttp_client): resp = await client.post( f"/auth/login_flow/{step['flow_id']}", - json={"client_id": CLIENT_ID, "username": "test-user", "password": "test-pass"}, + json={ + "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, + "username": "test-user", + "password": "test-pass", + }, ) assert resp.status == HTTPStatus.OK @@ -358,7 +368,10 @@ async def test_refresh_token_provider_rejected( assert result["error_description"] == "Invalid access" -async def test_revoking_refresh_token(hass, aiohttp_client): +@pytest.mark.parametrize( + "url,base_data", [("/auth/token", {"action": "revoke"}), ("/auth/revoke", {})] +) +async def test_revoking_refresh_token(url, base_data, hass, aiohttp_client): """Test that we can revoke refresh tokens.""" client = await async_setup_auth(hass, aiohttp_client) refresh_token = await async_setup_user_refresh_token(hass) @@ -380,9 +393,7 @@ async def test_revoking_refresh_token(hass, aiohttp_client): ) # Revoke refresh token - resp = await client.post( - "/auth/token", data={"token": refresh_token.token, "action": "revoke"} - ) + resp = await client.post(url, data={**base_data, "token": refresh_token.token}) assert resp.status == HTTPStatus.OK # Old access token should be no longer valid diff --git a/tests/components/auth/test_init_link_user.py b/tests/components/auth/test_init_link_user.py index 036dad4265f..bad6e3bfefe 100644 --- a/tests/components/auth/test_init_link_user.py +++ b/tests/components/auth/test_init_link_user.py @@ -46,7 +46,12 @@ async def async_get_code(hass, aiohttp_client): resp = await client.post( f"/auth/login_flow/{step['flow_id']}", - json={"client_id": CLIENT_ID, "username": "2nd-user", "password": "2nd-pass"}, + json={ + "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, + "username": "2nd-user", + "password": "2nd-pass", + }, ) assert resp.status == HTTPStatus.OK diff --git a/tests/components/auth/test_login_flow.py b/tests/components/auth/test_login_flow.py index 1fa06045de6..ce547149786 100644 --- a/tests/components/auth/test_login_flow.py +++ b/tests/components/auth/test_login_flow.py @@ -61,6 +61,7 @@ async def test_invalid_username_password(hass, aiohttp_client): f"/auth/login_flow/{step['flow_id']}", json={ "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, "username": "wrong-user", "password": "test-pass", }, @@ -81,6 +82,7 @@ async def test_invalid_username_password(hass, aiohttp_client): f"/auth/login_flow/{step['flow_id']}", json={ "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, "username": "test-user", "password": "wrong-pass", }, @@ -93,6 +95,49 @@ async def test_invalid_username_password(hass, aiohttp_client): assert step["step_id"] == "init" assert step["errors"]["base"] == "invalid_auth" + # Incorrect username and invalid redirect URI fails on wrong login + with patch( + "homeassistant.components.auth.login_flow.process_wrong_login" + ) as mock_process_wrong_login: + resp = await client.post( + f"/auth/login_flow/{step['flow_id']}", + json={ + "client_id": CLIENT_ID, + "redirect_uri": "http://some-other-domain.com", + "username": "wrong-user", + "password": "test-pass", + }, + ) + + assert resp.status == HTTPStatus.OK + step = await resp.json() + assert len(mock_process_wrong_login.mock_calls) == 1 + + assert step["step_id"] == "init" + assert step["errors"]["base"] == "invalid_auth" + + # Incorrect redirect URI + with patch( + "homeassistant.components.auth.indieauth.fetch_redirect_uris", return_value=[] + ), patch( + "homeassistant.components.http.ban.process_wrong_login" + ) as mock_process_wrong_login: + resp = await client.post( + f"/auth/login_flow/{step['flow_id']}", + json={ + "client_id": CLIENT_ID, + "redirect_uri": "http://some-other-domain.com", + "username": "test-user", + "password": "test-pass", + }, + ) + + assert resp.status == HTTPStatus.FORBIDDEN + data = await resp.json() + assert len(mock_process_wrong_login.mock_calls) == 1 + + assert data["message"] == "Invalid redirect URI" + async def test_login_exist_user(hass, aiohttp_client): """Test logging in with exist user.""" @@ -120,6 +165,7 @@ async def test_login_exist_user(hass, aiohttp_client): f"/auth/login_flow/{step['flow_id']}", json={ "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, "username": "test-user", "password": "test-pass", }, @@ -160,6 +206,7 @@ async def test_login_local_only_user(hass, aiohttp_client): f"/auth/login_flow/{step['flow_id']}", json={ "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, "username": "test-user", "password": "test-pass", }, @@ -202,9 +249,30 @@ async def test_login_exist_user_ip_changes(hass, aiohttp_client): resp = await client.post( f"/auth/login_flow/{step['flow_id']}", - json={"client_id": CLIENT_ID, "username": "test-user", "password": "test-pass"}, + json={ + "client_id": CLIENT_ID, + "redirect_uri": CLIENT_REDIRECT_URI, + "username": "test-user", + "password": "test-pass", + }, ) assert resp.status == 400 response = await resp.json() assert response == {"message": "IP address changed"} + + +async def test_well_known_auth_info(hass, aiohttp_client): + """Test logging in and the ip address changes results in an rejection.""" + client = await async_setup_auth(hass, aiohttp_client, setup_api=True) + resp = await client.get( + "/.well-known/oauth-authorization-server", + ) + assert resp.status == 200 + assert await resp.json() == { + "authorization_endpoint": "/auth/authorize", + "token_endpoint": "/auth/token", + "revocation_endpoint": "/auth/revoke", + "response_types_supported": ["code"], + "service_documentation": "https://developers.home-assistant.io/docs/auth_api", + }