From ad214bc95bb427d7bb91253a6e5f74fa0ee1a1f3 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Wed, 18 Mar 2020 09:15:29 +0100 Subject: [PATCH 01/54] First wip solution of oauth authentication --- homeassistant/auth/providers/openid.py | 223 +++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 homeassistant/auth/providers/openid.py diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py new file mode 100644 index 00000000000000..07c9be57ca4278 --- /dev/null +++ b/homeassistant/auth/providers/openid.py @@ -0,0 +1,223 @@ +"""OpenId based authentication provider.""" + +import logging +import re +from typing import Any, Dict, Optional, cast + +from aiohttp.web import HTTPBadRequest, Request, Response +import jwt +import voluptuous as vol +from yarl import URL + +from homeassistant.components.http import HomeAssistantView +from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError +from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.config_entry_oauth2_flow import _decode_jwt, _encode_jwt + +from . import AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, AuthProvider, LoginFlow +from ..models import Credentials, UserMeta + +_LOGGER = logging.getLogger(__name__) + +CONF_CLIENT_ID = "client_id" +CONF_CLIENT_SECRET = "client_secret" +CONF_TOKEN_URI = "token_uri" +CONF_AUTHORIZATION_URI = "authorization_uri" +CONF_EMAILS = "emails" + +DATA_JWT_SECRET = "openid_jwt_secret" + +CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend( + { + vol.Required(CONF_CLIENT_ID): str, + vol.Required(CONF_CLIENT_SECRET): str, + vol.Required(CONF_TOKEN_URI): str, + vol.Required(CONF_AUTHORIZATION_URI): str, + vol.Required(CONF_EMAILS): [str], + }, + extra=vol.PREVENT_EXTRA, +) + +AUTH_CALLBACK_PATH = "/api/openid/redirect" + + +class InvalidAuthError(HomeAssistantError): + """Raised when submitting invalid authentication.""" + + +registered = False + + +@AUTH_PROVIDERS.register("openid") +class OpenIdAuthProvider(AuthProvider): + """Example auth provider based on hardcoded usernames and passwords.""" + + DEFAULT_TITLE = "OpenId Connect" + + def __init__(self, *args: Any, **kwargs: Any) -> None: + """Extend parent's __init__.""" + super().__init__(*args, **kwargs) + + @property + def redirect_uri(self) -> str: + """Return the redirect uri.""" + return f"{self.hass.config.api.base_url}{AUTH_CALLBACK_PATH}" # type: ignore + + async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: + """Return a flow to login.""" + global registered + if not registered: + self.hass.http.register_view(OpenIdCallbackView()) # type: ignore + registered = True + return OpenIdLoginFlow(self) + + async def async_retrieve_token(self, code: str) -> Dict[str, Any]: + """Convert a token code into an actual token.""" + payload = { + "grant_type": "authorization_code", + "code": code, + "redirect_uri": self.redirect_uri, + "client_id": self.config[CONF_CLIENT_ID], + "client_secret": self.config[CONF_CLIENT_SECRET], + } + uri = self.config[CONF_TOKEN_URI] + session = async_get_clientsession(self.hass) + + async with session.post(uri, data=payload) as response: + if 400 <= response.status: + if "json" in response.headers.get("CONTENT-TYPE", ""): + data = await response.json() + else: + data = await response.text() + raise InvalidAuthError(f"Token validation failed with error: {data}") + return cast(Dict[str, Any], await response.json()) + + async def async_validate_token(self, token: Dict[str, Any]) -> Dict[str, Any]: + """Validate a token.""" + id_token = jwt.decode(token["id_token"]) + + if id_token["email"] not in self.config[CONF_EMAILS]: + raise InvalidAuthError(f"Email {id_token['email']} not in allowed users") + return id_token + + @callback + def async_generate_authorize_url(self, flow_id: str) -> str: + """Generate a authorization url for a given flow.""" + return str( + URL(self.config["authorization_uri"]).with_query( + { + "response_type": "code", + "client_id": self.config["client_id"], + "redirect_uri": self.redirect_uri, + "state": _encode_jwt(self.hass, {"flow_id": flow_id}), + "scope": "openid email", + } + ) + ) + + async def async_get_or_create_credentials( + self, flow_result: Dict[str, str] + ) -> Credentials: + """Get credentials based on the flow result.""" + email = flow_result["email"] + + for credential in await self.async_credentials(): + if credential.data["email"] == email: + return credential + + # Create new credentials. + return self.async_create_credentials(flow_result) + + async def async_user_meta_for_credentials( + self, credentials: Credentials + ) -> UserMeta: + """Return extra user metadata for credentials. + + Will be used to populate info when creating a new user. + """ + email = credentials.data["email"] + match = re.match(r"([^@]+)", email) + if match: + name = str(match.groups(0)) + else: + name = str(email) + + return UserMeta(name=name, is_active=True) + + +class OpenIdLoginFlow(LoginFlow): + """Handler for the login flow.""" + + external_data: str + + async def async_step_init( + self, user_input: Optional[Dict[str, str]] = None + ) -> Dict[str, Any]: + """Handle the step of the form.""" + return await self.async_step_authenticate() + + async def async_step_authenticate( + self, user_input: Optional[Dict[str, str]] = None + ) -> Dict[str, Any]: + """Authenticate user using external step.""" + + provider = cast(OpenIdAuthProvider, self._auth_provider) + + if user_input: + self.external_data = str(user_input) + return self.async_external_step_done(next_step_id="authorize") + + url = provider.async_generate_authorize_url(self.flow_id) + return self.async_external_step(step_id="authenticate", url=url) + + async def async_step_authorize( + self, user_input: Optional[Dict[str, str]] = None + ) -> Dict[str, Any]: + """Authorize user received from external step.""" + + provider = cast(OpenIdAuthProvider, self._auth_provider) + try: + token = await provider.async_retrieve_token(self.external_data) + result = await provider.async_validate_token(token) + except InvalidAuthError: + return self.async_abort(reason="invalid_auth") + return await self.async_finish(result) + + +class OpenIdCallbackView(HomeAssistantView): + """Handle openid callback.""" + + url = AUTH_CALLBACK_PATH + name = "api:openid:redirect" + requires_auth = False + + def __init__(self) -> None: + """Initialize instance of the view.""" + super().__init__() + + async def get(self, request: Request) -> Response: + """Handle oauth token request.""" + hass = cast(HomeAssistant, request.app["hass"]) + + def check_get(param: str) -> Any: + if param not in request.query: + _LOGGER.error("State missing in request.") + raise HTTPBadRequest(text="Parameter {} not found".format(param)) + return request.query[param] + + state = check_get("state") + code = check_get("code") + + state = _decode_jwt(hass, state) + if state is None: + _LOGGER.error("State failed to decode.") + raise HTTPBadRequest(text="Invalid state") + + auth_manager = hass.auth # type: ignore + await auth_manager.login_flow.async_configure(state["flow_id"], user_input=code) + + return Response( + headers={"content-type": "text/html"}, + text="", + ) From a2b86826d9ddeab1661bb82271ec22c8f10d6701 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Wed, 18 Mar 2020 10:34:21 +0100 Subject: [PATCH 02/54] Adjust regexp --- homeassistant/auth/providers/openid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 07c9be57ca4278..faed4fa9a42b9c 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -137,9 +137,9 @@ async def async_user_meta_for_credentials( Will be used to populate info when creating a new user. """ email = credentials.data["email"] - match = re.match(r"([^@]+)", email) + match = re.match(r"[^@]+", email) if match: - name = str(match.groups(0)) + name = str(match.group(0)) else: name = str(email) From 3d3943e84787ca6a69510f0c704ff8c90dd5734b Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Wed, 18 Mar 2020 10:34:42 +0100 Subject: [PATCH 03/54] Just log text error --- homeassistant/auth/providers/openid.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index faed4fa9a42b9c..f5982de625479a 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -86,11 +86,8 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: async with session.post(uri, data=payload) as response: if 400 <= response.status: - if "json" in response.headers.get("CONTENT-TYPE", ""): - data = await response.json() - else: - data = await response.text() - raise InvalidAuthError(f"Token validation failed with error: {data}") + data = await response.text() + raise InvalidAuthError(f"Token retrieveal failed with error: {data}") return cast(Dict[str, Any], await response.json()) async def async_validate_token(self, token: Dict[str, Any]) -> Dict[str, Any]: From ab91143846d0c54340e0e58ed72aa38adbf2c2aa Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Wed, 18 Mar 2020 10:36:27 +0100 Subject: [PATCH 04/54] Don't verify id_token for now, we don't know certificate --- homeassistant/auth/providers/openid.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index f5982de625479a..cb65d67d30f72e 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -90,9 +90,13 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: raise InvalidAuthError(f"Token retrieveal failed with error: {data}") return cast(Dict[str, Any], await response.json()) + async def async_decode_id_token(self, id_token: str) -> Dict[str, Any]: + """Decode a token.""" + return jwt.decode(id_token, verify=False) + async def async_validate_token(self, token: Dict[str, Any]) -> Dict[str, Any]: """Validate a token.""" - id_token = jwt.decode(token["id_token"]) + id_token = await self.async_decode_id_token(token["id_token"]) if id_token["email"] not in self.config[CONF_EMAILS]: raise InvalidAuthError(f"Email {id_token['email']} not in allowed users") From d9871b07017551b24d1da21ba0a70067bb51c3ac Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Wed, 18 Mar 2020 15:18:07 +0100 Subject: [PATCH 05/54] Switch to using discovery document instead of hardcoding --- homeassistant/auth/providers/openid.py | 95 ++++++++++++++++++-------- 1 file changed, 67 insertions(+), 28 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index cb65d67d30f72e..beb0a4196040a9 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -4,13 +4,15 @@ import re from typing import Any, Dict, Optional, cast +from aiohttp import ClientResponseError +from aiohttp.client import ClientResponse from aiohttp.web import HTTPBadRequest, Request, Response import jwt import voluptuous as vol from yarl import URL from homeassistant.components.http import HomeAssistantView -from homeassistant.core import HomeAssistant, callback +from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.config_entry_oauth2_flow import _decode_jwt, _encode_jwt @@ -20,6 +22,7 @@ _LOGGER = logging.getLogger(__name__) +CONF_ISSUER = "issuer" CONF_CLIENT_ID = "client_id" CONF_CLIENT_SECRET = "client_secret" CONF_TOKEN_URI = "token_uri" @@ -30,10 +33,9 @@ CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend( { + vol.Required(CONF_ISSUER): str, vol.Required(CONF_CLIENT_ID): str, vol.Required(CONF_CLIENT_SECRET): str, - vol.Required(CONF_TOKEN_URI): str, - vol.Required(CONF_AUTHORIZATION_URI): str, vol.Required(CONF_EMAILS): [str], }, extra=vol.PREVENT_EXTRA, @@ -46,8 +48,24 @@ class InvalidAuthError(HomeAssistantError): """Raised when submitting invalid authentication.""" +async def raise_for_status(response: ClientResponse) -> None: + """Raise exception on data failure with logging.""" + if 400 <= response.status: + e = ClientResponseError( + response.request_info, + response.history, + code=response.status, + headers=response.headers, + ) + data = await response.text() + _LOGGER.error("Request failed: %s", data) + raise InvalidAuthError(data) from e + + registered = False +WANTED_SCOPES = set(["openid", "email", "profile"]) + @AUTH_PROVIDERS.register("openid") class OpenIdAuthProvider(AuthProvider): @@ -55,10 +73,29 @@ class OpenIdAuthProvider(AuthProvider): DEFAULT_TITLE = "OpenId Connect" + _discovery_document: Optional[Dict[str, Any]] + def __init__(self, *args: Any, **kwargs: Any) -> None: """Extend parent's __init__.""" super().__init__(*args, **kwargs) + async def async_get_discovery_document(self) -> Dict[str, Any]: + """Retrieve a discovery document for openid.""" + if self._discovery_document is None: + session = async_get_clientsession(self.hass) + async with session.get(self.discovery_url) as response: + await raise_for_status(response) + self._discovery_document = cast(Dict[str, Any], await response.json()) + + return self._discovery_document + + @property + def discovery_url(self) -> str: + """Construct discovery url based on config.""" + return str( + URL(self.config[CONF_ISSUER]).with_path(".well-known/openid-configuration") + ) + @property def redirect_uri(self) -> str: """Return the redirect uri.""" @@ -74,6 +111,8 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: async def async_retrieve_token(self, code: str) -> Dict[str, Any]: """Convert a token code into an actual token.""" + data = await self.async_get_discovery_document() + payload = { "grant_type": "authorization_code", "code": code, @@ -81,13 +120,10 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: "client_id": self.config[CONF_CLIENT_ID], "client_secret": self.config[CONF_CLIENT_SECRET], } - uri = self.config[CONF_TOKEN_URI] - session = async_get_clientsession(self.hass) - async with session.post(uri, data=payload) as response: - if 400 <= response.status: - data = await response.text() - raise InvalidAuthError(f"Token retrieveal failed with error: {data}") + session = async_get_clientsession(self.hass) + async with session.post(data["token_endpoint"], data=payload) as response: + await raise_for_status(response) return cast(Dict[str, Any], await response.json()) async def async_decode_id_token(self, id_token: str) -> Dict[str, Any]: @@ -102,20 +138,21 @@ async def async_validate_token(self, token: Dict[str, Any]) -> Dict[str, Any]: raise InvalidAuthError(f"Email {id_token['email']} not in allowed users") return id_token - @callback - def async_generate_authorize_url(self, flow_id: str) -> str: + async def async_generate_authorize_url(self, flow_id: str) -> str: """Generate a authorization url for a given flow.""" - return str( - URL(self.config["authorization_uri"]).with_query( - { - "response_type": "code", - "client_id": self.config["client_id"], - "redirect_uri": self.redirect_uri, - "state": _encode_jwt(self.hass, {"flow_id": flow_id}), - "scope": "openid email", - } - ) - ) + data = await self.async_get_discovery_document() + + scopes = WANTED_SCOPES.intersection(data["scopes_supported"]) + + query = { + "response_type": "code", + "client_id": self.config["client_id"], + "redirect_uri": self.redirect_uri, + "state": _encode_jwt(self.hass, {"flow_id": flow_id}), + "scope": " ".join(scopes), + } + + return str(URL(data["authorization_endpoint"]).with_query(query)) async def async_get_or_create_credentials( self, flow_result: Dict[str, str] @@ -127,7 +164,6 @@ async def async_get_or_create_credentials( if credential.data["email"] == email: return credential - # Create new credentials. return self.async_create_credentials(flow_result) async def async_user_meta_for_credentials( @@ -138,11 +174,14 @@ async def async_user_meta_for_credentials( Will be used to populate info when creating a new user. """ email = credentials.data["email"] - match = re.match(r"[^@]+", email) - if match: - name = str(match.group(0)) + if "name" in credentials.data: + name = credentials.data["name"] else: - name = str(email) + match = re.match(r"[^@]+", email) + if match: + name = str(match.group(0)) + else: + name = str(email) return UserMeta(name=name, is_active=True) @@ -169,7 +208,7 @@ async def async_step_authenticate( self.external_data = str(user_input) return self.async_external_step_done(next_step_id="authorize") - url = provider.async_generate_authorize_url(self.flow_id) + url = await provider.async_generate_authorize_url(self.flow_id) return self.async_external_step(step_id="authenticate", url=url) async def async_step_authorize( From 490555a329f04a27202c049f8aa6bb9c91321700 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 07:54:23 +0100 Subject: [PATCH 06/54] Drop unused constant --- homeassistant/auth/providers/openid.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index beb0a4196040a9..a1c42ee7535c41 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -29,8 +29,6 @@ CONF_AUTHORIZATION_URI = "authorization_uri" CONF_EMAILS = "emails" -DATA_JWT_SECRET = "openid_jwt_secret" - CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend( { vol.Required(CONF_ISSUER): str, From 2d744fb029b1a914d5b8ed5e35a591c5d4892fef Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 07:54:38 +0100 Subject: [PATCH 07/54] Don't allow multifactor --- homeassistant/auth/providers/openid.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index a1c42ee7535c41..695eed8b063c4c 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -152,6 +152,11 @@ async def async_generate_authorize_url(self, flow_id: str) -> str: return str(URL(data["authorization_endpoint"]).with_query(query)) + @property + def support_mfa(self) -> bool: + """Return whether multi-factor auth supported by the auth provider.""" + return False + async def async_get_or_create_credentials( self, flow_result: Dict[str, str] ) -> Credentials: From 706d9a85907510fd4d756bc78a4fd16ebc2daf94 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 08:17:32 +0100 Subject: [PATCH 08/54] Move view from global variable into hass.data --- homeassistant/auth/providers/openid.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 695eed8b063c4c..9dd70d0aae48d4 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -40,6 +40,7 @@ ) AUTH_CALLBACK_PATH = "/api/openid/redirect" +DATA_OPENID_VIEW = "openid_view" class InvalidAuthError(HomeAssistantError): @@ -60,8 +61,6 @@ async def raise_for_status(response: ClientResponse) -> None: raise InvalidAuthError(data) from e -registered = False - WANTED_SCOPES = set(["openid", "email", "profile"]) @@ -101,10 +100,11 @@ def redirect_uri(self) -> str: async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" - global registered - if not registered: - self.hass.http.register_view(OpenIdCallbackView()) # type: ignore - registered = True + if DATA_OPENID_VIEW not in self.hass.data: + self.hass.data[DATA_OPENID_VIEW] = self.hass.http.register_view( # type: ignore + OpenIdCallbackView() + ) + return OpenIdLoginFlow(self) async def async_retrieve_token(self, code: str) -> Dict[str, Any]: From 1b98d2d386dc673fa1f0c22f0168eb2bc5ccb1bb Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 08:19:12 +0100 Subject: [PATCH 09/54] Move well known string into constant --- homeassistant/auth/providers/openid.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 9dd70d0aae48d4..fea3ab4361d28c 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -40,6 +40,7 @@ ) AUTH_CALLBACK_PATH = "/api/openid/redirect" +OPENID_CONFIGURATION_PATH = ".well-known/openid-configuration" DATA_OPENID_VIEW = "openid_view" @@ -89,9 +90,7 @@ async def async_get_discovery_document(self) -> Dict[str, Any]: @property def discovery_url(self) -> str: """Construct discovery url based on config.""" - return str( - URL(self.config[CONF_ISSUER]).with_path(".well-known/openid-configuration") - ) + return str(URL(self.config[CONF_ISSUER]).with_path(OPENID_CONFIGURATION_PATH)) @property def redirect_uri(self) -> str: From d46cff21ff37a3d49985ad2cbc36d8d09d02a406 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 08:19:31 +0100 Subject: [PATCH 10/54] Discovery document must start as None --- homeassistant/auth/providers/openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index fea3ab4361d28c..aa466f34eb475a 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -71,7 +71,7 @@ class OpenIdAuthProvider(AuthProvider): DEFAULT_TITLE = "OpenId Connect" - _discovery_document: Optional[Dict[str, Any]] + _discovery_document: Optional[Dict[str, Any]] = None def __init__(self, *args: Any, **kwargs: Any) -> None: """Extend parent's __init__.""" From 3827d8643b54a23c28f5e136c1b6fe36f68d838d Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 08:19:45 +0100 Subject: [PATCH 11/54] Correct some lint errors --- homeassistant/auth/providers/openid.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index aa466f34eb475a..ab7a248d10ee55 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -50,8 +50,8 @@ class InvalidAuthError(HomeAssistantError): async def raise_for_status(response: ClientResponse) -> None: """Raise exception on data failure with logging.""" - if 400 <= response.status: - e = ClientResponseError( + if response.status >= 400: + standard = ClientResponseError( response.request_info, response.history, code=response.status, @@ -59,7 +59,7 @@ async def raise_for_status(response: ClientResponse) -> None: ) data = await response.text() _LOGGER.error("Request failed: %s", data) - raise InvalidAuthError(data) from e + raise InvalidAuthError(data) from standard WANTED_SCOPES = set(["openid", "email", "profile"]) From c11c7a72443f69ed72e76a03efc1d5e5f2723bf6 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 08:35:26 +0100 Subject: [PATCH 12/54] Send and validate nonce --- homeassistant/auth/providers/openid.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index ab7a248d10ee55..0b214e51c694e3 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -2,6 +2,7 @@ import logging import re +import secrets from typing import Any, Dict, Optional, cast from aiohttp import ClientResponseError @@ -127,15 +128,19 @@ async def async_decode_id_token(self, id_token: str) -> Dict[str, Any]: """Decode a token.""" return jwt.decode(id_token, verify=False) - async def async_validate_token(self, token: Dict[str, Any]) -> Dict[str, Any]: + async def async_validate_token( + self, token: Dict[str, Any], nonce: str + ) -> Dict[str, Any]: """Validate a token.""" id_token = await self.async_decode_id_token(token["id_token"]) + if id_token.get("nonce") != nonce: + raise InvalidAuthError(f"Nonce mismatch in id_token") if id_token["email"] not in self.config[CONF_EMAILS]: raise InvalidAuthError(f"Email {id_token['email']} not in allowed users") return id_token - async def async_generate_authorize_url(self, flow_id: str) -> str: + async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: """Generate a authorization url for a given flow.""" data = await self.async_get_discovery_document() @@ -147,6 +152,7 @@ async def async_generate_authorize_url(self, flow_id: str) -> str: "redirect_uri": self.redirect_uri, "state": _encode_jwt(self.hass, {"flow_id": flow_id}), "scope": " ".join(scopes), + "nonce": nonce, } return str(URL(data["authorization_endpoint"]).with_query(query)) @@ -192,6 +198,7 @@ class OpenIdLoginFlow(LoginFlow): """Handler for the login flow.""" external_data: str + nonce: str async def async_step_init( self, user_input: Optional[Dict[str, str]] = None @@ -209,8 +216,8 @@ async def async_step_authenticate( if user_input: self.external_data = str(user_input) return self.async_external_step_done(next_step_id="authorize") - - url = await provider.async_generate_authorize_url(self.flow_id) + self.nonce = secrets.token_hex() + url = await provider.async_generate_authorize_url(self.flow_id, self.nonce) return self.async_external_step(step_id="authenticate", url=url) async def async_step_authorize( @@ -221,7 +228,7 @@ async def async_step_authorize( provider = cast(OpenIdAuthProvider, self._auth_provider) try: token = await provider.async_retrieve_token(self.external_data) - result = await provider.async_validate_token(token) + result = await provider.async_validate_token(token, self.nonce) except InvalidAuthError: return self.async_abort(reason="invalid_auth") return await self.async_finish(result) From 268edf8e285724d033ef94447d2e52e093b66363 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 16:00:36 +0100 Subject: [PATCH 13/54] Cache discovery on start of flow --- homeassistant/auth/providers/openid.py | 36 +++++++++++++++----------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 0b214e51c694e3..c2538128b07ebe 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -72,21 +72,21 @@ class OpenIdAuthProvider(AuthProvider): DEFAULT_TITLE = "OpenId Connect" - _discovery_document: Optional[Dict[str, Any]] = None + _discovery_document: Dict[str, Any] def __init__(self, *args: Any, **kwargs: Any) -> None: """Extend parent's __init__.""" super().__init__(*args, **kwargs) - async def async_get_discovery_document(self) -> Dict[str, Any]: - """Retrieve a discovery document for openid.""" - if self._discovery_document is None: - session = async_get_clientsession(self.hass) - async with session.get(self.discovery_url) as response: - await raise_for_status(response) - self._discovery_document = cast(Dict[str, Any], await response.json()) + async def async_get_discovery_document(self) -> None: + """Cache discovery document for openid.""" + if hasattr(self, "._discovery_document"): + return - return self._discovery_document + session = async_get_clientsession(self.hass) + async with session.get(self.discovery_url) as response: + await raise_for_status(response) + self._discovery_document = cast(Dict[str, Any], await response.json()) @property def discovery_url(self) -> str: @@ -100,6 +100,9 @@ def redirect_uri(self) -> str: async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" + + await self.async_get_discovery_document() + if DATA_OPENID_VIEW not in self.hass.data: self.hass.data[DATA_OPENID_VIEW] = self.hass.http.register_view( # type: ignore OpenIdCallbackView() @@ -109,7 +112,6 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: async def async_retrieve_token(self, code: str) -> Dict[str, Any]: """Convert a token code into an actual token.""" - data = await self.async_get_discovery_document() payload = { "grant_type": "authorization_code", @@ -120,7 +122,9 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: } session = async_get_clientsession(self.hass) - async with session.post(data["token_endpoint"], data=payload) as response: + async with session.post( + self._discovery_document["token_endpoint"], data=payload + ) as response: await raise_for_status(response) return cast(Dict[str, Any], await response.json()) @@ -142,9 +146,9 @@ async def async_validate_token( async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: """Generate a authorization url for a given flow.""" - data = await self.async_get_discovery_document() - - scopes = WANTED_SCOPES.intersection(data["scopes_supported"]) + scopes = WANTED_SCOPES.intersection( + self._discovery_document["scopes_supported"] + ) query = { "response_type": "code", @@ -155,7 +159,9 @@ async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: "nonce": nonce, } - return str(URL(data["authorization_endpoint"]).with_query(query)) + return str( + URL(self._discovery_document["authorization_endpoint"]).with_query(query) + ) @property def support_mfa(self) -> bool: From 844dcd65298042708c46d8134b909dd4f841e92e Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 16:11:19 +0100 Subject: [PATCH 14/54] Add id_token verification --- homeassistant/auth/providers/openid.py | 35 +++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index c2538128b07ebe..3e9765ca4aab6d 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -1,5 +1,6 @@ """OpenId based authentication provider.""" +import json import logging import re import secrets @@ -8,6 +9,7 @@ from aiohttp import ClientResponseError from aiohttp.client import ClientResponse from aiohttp.web import HTTPBadRequest, Request, Response +from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat import jwt import voluptuous as vol from yarl import URL @@ -63,6 +65,21 @@ async def raise_for_status(response: ClientResponse) -> None: raise InvalidAuthError(data) from standard +def convert_jwks_to_pem(jwks: Dict[str, Any]) -> bytes: + """Convert a JWKS key set into a PEM combined certificate string.""" + + algorithms = jwt.algorithms.get_default_algorithms() + pems = bytes() + for key in jwks["keys"]: + algorithm = algorithms[key["alg"]] + public_key = algorithm.from_jwk(json.dumps(key)) + pem = public_key.public_bytes( + encoding=Encoding.PEM, format=PublicFormat.SubjectPublicKeyInfo + ) + pems += pem + return pems + + WANTED_SCOPES = set(["openid", "email", "profile"]) @@ -73,6 +90,7 @@ class OpenIdAuthProvider(AuthProvider): DEFAULT_TITLE = "OpenId Connect" _discovery_document: Dict[str, Any] + _jwks_pem: bytes def __init__(self, *args: Any, **kwargs: Any) -> None: """Extend parent's __init__.""" @@ -88,6 +106,18 @@ async def async_get_discovery_document(self) -> None: await raise_for_status(response) self._discovery_document = cast(Dict[str, Any], await response.json()) + async def async_get_jwks_pem(self) -> None: + """Cache the keys for id verification.""" + if hasattr(self, "._jwks_pem"): + return + + session = async_get_clientsession(self.hass) + async with session.get(self._discovery_document["jwks_uri"]) as response: + await raise_for_status(response) + jwks = cast(Dict[str, Any], await response.json()) + + self._jwks_pem = convert_jwks_to_pem(jwks) + @property def discovery_url(self) -> str: """Construct discovery url based on config.""" @@ -102,6 +132,7 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" await self.async_get_discovery_document() + await self.async_get_jwks_pem() if DATA_OPENID_VIEW not in self.hass.data: self.hass.data[DATA_OPENID_VIEW] = self.hass.http.register_view( # type: ignore @@ -130,7 +161,9 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: async def async_decode_id_token(self, id_token: str) -> Dict[str, Any]: """Decode a token.""" - return jwt.decode(id_token, verify=False) + return jwt.decode( + id_token, key=self._jwks_pem, audience=self.config[CONF_CLIENT_ID] + ) async def async_validate_token( self, token: Dict[str, Any], nonce: str From 35b1dc2ab10e10ef01d46afea56ebf7acdde4958 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 17:01:03 +0100 Subject: [PATCH 15/54] Switch to sub for credential lookup --- homeassistant/auth/providers/openid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 3e9765ca4aab6d..74f2ac6f9fb3b1 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -205,10 +205,10 @@ async def async_get_or_create_credentials( self, flow_result: Dict[str, str] ) -> Credentials: """Get credentials based on the flow result.""" - email = flow_result["email"] + subject = flow_result["sub"] for credential in await self.async_credentials(): - if credential.data["email"] == email: + if credential.data["sub"] == subject: return credential return self.async_create_credentials(flow_result) From 7dfb8c2357f3f1069da9a82a9ae29c0691896181 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 17:03:09 +0100 Subject: [PATCH 16/54] Switch to jose to jwk handling --- homeassistant/auth/providers/openid.py | 47 ++++++++------------------ 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 74f2ac6f9fb3b1..d620023121fc79 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -1,6 +1,5 @@ """OpenId based authentication provider.""" -import json import logging import re import secrets @@ -9,8 +8,7 @@ from aiohttp import ClientResponseError from aiohttp.client import ClientResponse from aiohttp.web import HTTPBadRequest, Request, Response -from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat -import jwt +from jose import jwt import voluptuous as vol from yarl import URL @@ -65,21 +63,6 @@ async def raise_for_status(response: ClientResponse) -> None: raise InvalidAuthError(data) from standard -def convert_jwks_to_pem(jwks: Dict[str, Any]) -> bytes: - """Convert a JWKS key set into a PEM combined certificate string.""" - - algorithms = jwt.algorithms.get_default_algorithms() - pems = bytes() - for key in jwks["keys"]: - algorithm = algorithms[key["alg"]] - public_key = algorithm.from_jwk(json.dumps(key)) - pem = public_key.public_bytes( - encoding=Encoding.PEM, format=PublicFormat.SubjectPublicKeyInfo - ) - pems += pem - return pems - - WANTED_SCOPES = set(["openid", "email", "profile"]) @@ -90,7 +73,7 @@ class OpenIdAuthProvider(AuthProvider): DEFAULT_TITLE = "OpenId Connect" _discovery_document: Dict[str, Any] - _jwks_pem: bytes + _jwks: Dict[str, Any] def __init__(self, *args: Any, **kwargs: Any) -> None: """Extend parent's __init__.""" @@ -106,17 +89,15 @@ async def async_get_discovery_document(self) -> None: await raise_for_status(response) self._discovery_document = cast(Dict[str, Any], await response.json()) - async def async_get_jwks_pem(self) -> None: + async def async_get_jwks(self) -> None: """Cache the keys for id verification.""" - if hasattr(self, "._jwks_pem"): + if hasattr(self, "._jwks"): return session = async_get_clientsession(self.hass) async with session.get(self._discovery_document["jwks_uri"]) as response: await raise_for_status(response) - jwks = cast(Dict[str, Any], await response.json()) - - self._jwks_pem = convert_jwks_to_pem(jwks) + self._jwks = cast(Dict[str, Any], await response.json()) @property def discovery_url(self) -> str: @@ -132,7 +113,7 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" await self.async_get_discovery_document() - await self.async_get_jwks_pem() + await self.async_get_jwks() if DATA_OPENID_VIEW not in self.hass.data: self.hass.data[DATA_OPENID_VIEW] = self.hass.http.register_view( # type: ignore @@ -159,17 +140,19 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: await raise_for_status(response) return cast(Dict[str, Any], await response.json()) - async def async_decode_id_token(self, id_token: str) -> Dict[str, Any]: - """Decode a token.""" - return jwt.decode( - id_token, key=self._jwks_pem, audience=self.config[CONF_CLIENT_ID] - ) - async def async_validate_token( self, token: Dict[str, Any], nonce: str ) -> Dict[str, Any]: """Validate a token.""" - id_token = await self.async_decode_id_token(token["id_token"]) + id_token = cast( + Dict[str, Any], + jwt.decode( + token["id_token"], + key=self._jwks, + audience=self.config[CONF_CLIENT_ID], + access_token=token["access_token"], + ), + ) if id_token.get("nonce") != nonce: raise InvalidAuthError(f"Nonce mismatch in id_token") From ff974225a3a9ca7451fd298b7ab63edcf007ca7f Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 17:44:03 +0100 Subject: [PATCH 17/54] Drop separate view --- homeassistant/auth/providers/openid.py | 56 +++---------------- .../helpers/config_entry_oauth2_flow.py | 22 ++++++-- 2 files changed, 23 insertions(+), 55 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index d620023121fc79..7c741168be752c 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -7,16 +7,17 @@ from aiohttp import ClientResponseError from aiohttp.client import ClientResponse -from aiohttp.web import HTTPBadRequest, Request, Response from jose import jwt import voluptuous as vol from yarl import URL -from homeassistant.components.http import HomeAssistantView -from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.aiohttp_client import async_get_clientsession -from homeassistant.helpers.config_entry_oauth2_flow import _decode_jwt, _encode_jwt +from homeassistant.helpers.config_entry_oauth2_flow import ( + AUTH_CALLBACK_PATH, + _encode_jwt, + async_register_view, +) from . import AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, AuthProvider, LoginFlow from ..models import Credentials, UserMeta @@ -40,9 +41,7 @@ extra=vol.PREVENT_EXTRA, ) -AUTH_CALLBACK_PATH = "/api/openid/redirect" OPENID_CONFIGURATION_PATH = ".well-known/openid-configuration" -DATA_OPENID_VIEW = "openid_view" class InvalidAuthError(HomeAssistantError): @@ -115,10 +114,7 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: await self.async_get_discovery_document() await self.async_get_jwks() - if DATA_OPENID_VIEW not in self.hass.data: - self.hass.data[DATA_OPENID_VIEW] = self.hass.http.register_view( # type: ignore - OpenIdCallbackView() - ) + async_register_view(self.hass) return OpenIdLoginFlow(self) @@ -170,7 +166,7 @@ async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: "response_type": "code", "client_id": self.config["client_id"], "redirect_uri": self.redirect_uri, - "state": _encode_jwt(self.hass, {"flow_id": flow_id}), + "state": _encode_jwt(self.hass, {"flow_id": flow_id, "flow_type": "login"}), "scope": " ".join(scopes), "nonce": nonce, } @@ -254,41 +250,3 @@ async def async_step_authorize( except InvalidAuthError: return self.async_abort(reason="invalid_auth") return await self.async_finish(result) - - -class OpenIdCallbackView(HomeAssistantView): - """Handle openid callback.""" - - url = AUTH_CALLBACK_PATH - name = "api:openid:redirect" - requires_auth = False - - def __init__(self) -> None: - """Initialize instance of the view.""" - super().__init__() - - async def get(self, request: Request) -> Response: - """Handle oauth token request.""" - hass = cast(HomeAssistant, request.app["hass"]) - - def check_get(param: str) -> Any: - if param not in request.query: - _LOGGER.error("State missing in request.") - raise HTTPBadRequest(text="Parameter {} not found".format(param)) - return request.query[param] - - state = check_get("state") - code = check_get("code") - - state = _decode_jwt(hass, state) - if state is None: - _LOGGER.error("State failed to decode.") - raise HTTPBadRequest(text="Invalid state") - - auth_manager = hass.auth # type: ignore - await auth_manager.login_flow.async_configure(state["flow_id"], user_input=code) - - return Response( - headers={"content-type": "text/html"}, - text="", - ) diff --git a/homeassistant/helpers/config_entry_oauth2_flow.py b/homeassistant/helpers/config_entry_oauth2_flow.py index ace1365df1b135..95005d7ef2e05e 100644 --- a/homeassistant/helpers/config_entry_oauth2_flow.py +++ b/homeassistant/helpers/config_entry_oauth2_flow.py @@ -315,16 +315,21 @@ def async_register_implementation( async_register_implementation(hass, cls.DOMAIN, local_impl) +@callback +def async_register_view(hass: HomeAssistant) -> None: + """Make sure callback view is registered.""" + if not hass.data.get(DATA_VIEW_REGISTERED, False): + hass.http.register_view(OAuth2AuthorizeCallbackView()) # type: ignore + hass.data[DATA_VIEW_REGISTERED] = True + + @callback def async_register_implementation( hass: HomeAssistant, domain: str, implementation: AbstractOAuth2Implementation ) -> None: """Register an OAuth2 flow implementation for an integration.""" - if isinstance(implementation, LocalOAuth2Implementation) and not hass.data.get( - DATA_VIEW_REGISTERED, False - ): - hass.http.register_view(OAuth2AuthorizeCallbackView()) # type: ignore - hass.data[DATA_VIEW_REGISTERED] = True + if isinstance(implementation, LocalOAuth2Implementation): + async_register_view(hass) implementations = hass.data.setdefault(DATA_IMPLEMENTATIONS, {}) implementations.setdefault(domain, {})[implementation.domain] = implementation @@ -403,7 +408,12 @@ async def get(self, request: web.Request) -> web.Response: if state is None: return web.Response(text="Invalid state") - await hass.config_entries.flow.async_configure( + if state.get("flow_type") == "login": + flow_mgr = hass.auth.login_flow + else: + flow_mgr = hass.config_entries.flow + + await flow_mgr.async_configure( flow_id=state["flow_id"], user_input=request.query["code"] ) From b6d245648ad700de99ab84105bf6ff8914e22bc0 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 22:02:19 +0100 Subject: [PATCH 18/54] Add python jose to dependencies --- homeassistant/auth/providers/openid.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 7c741168be752c..374fc71bec0d6f 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -22,6 +22,8 @@ from . import AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, AuthProvider, LoginFlow from ..models import Credentials, UserMeta +REQUIREMENTS = ["python-jose=3.1.0"] + _LOGGER = logging.getLogger(__name__) CONF_ISSUER = "issuer" From 632406a820b27e6ccb51c5ad813ea84a16f5f2d1 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 19 Mar 2020 22:49:26 +0100 Subject: [PATCH 19/54] Pin requirements properly and update all requirements --- homeassistant/auth/providers/openid.py | 2 +- requirements_all.txt | 3 +++ requirements_test_all.txt | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 374fc71bec0d6f..87f844369492d7 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -22,7 +22,7 @@ from . import AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, AuthProvider, LoginFlow from ..models import Credentials, UserMeta -REQUIREMENTS = ["python-jose=3.1.0"] +REQUIREMENTS = ["python-jose==3.1.0"] _LOGGER = logging.getLogger(__name__) diff --git a/requirements_all.txt b/requirements_all.txt index d69ae1a8e85385..e3bae73a0de47b 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1759,6 +1759,9 @@ python-izone==1.1.2 # homeassistant.components.joaoapps_join python-join-api==0.0.6 +# homeassistant.auth.providers.openid +python-jose==3.1.0 + # homeassistant.components.juicenet python-juicenet==1.0.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index af43479ed0c805..99baf6e48684c8 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -843,6 +843,9 @@ python-forecastio==1.4.0 # homeassistant.components.izone python-izone==1.1.2 +# homeassistant.auth.providers.openid +python-jose==3.1.0 + # homeassistant.components.juicenet python-juicenet==1.0.1 From c06382f83ddf3af78a1fa7561dc4cea8cc4ea244 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 20 Mar 2020 00:01:48 +0100 Subject: [PATCH 20/54] Keep scopes sorted to simplify test --- homeassistant/auth/providers/openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 87f844369492d7..6fdfb546d5a272 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -169,7 +169,7 @@ async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: "client_id": self.config["client_id"], "redirect_uri": self.redirect_uri, "state": _encode_jwt(self.hass, {"flow_id": flow_id, "flow_type": "login"}), - "scope": " ".join(scopes), + "scope": " ".join(sorted(scopes)), "nonce": nonce, } From afaaa46e12bf4304fe8022d569ae190c88f529c5 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 20 Mar 2020 00:02:08 +0100 Subject: [PATCH 21/54] Import token_hex only to simplify test --- homeassistant/auth/providers/openid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 6fdfb546d5a272..119f6896879024 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -2,7 +2,7 @@ import logging import re -import secrets +from secrets import token_hex from typing import Any, Dict, Optional, cast from aiohttp import ClientResponseError @@ -236,7 +236,7 @@ async def async_step_authenticate( if user_input: self.external_data = str(user_input) return self.async_external_step_done(next_step_id="authorize") - self.nonce = secrets.token_hex() + self.nonce = token_hex() url = await provider.async_generate_authorize_url(self.flow_id, self.nonce) return self.async_external_step(step_id="authenticate", url=url) From f223a86f0b18ad46ac0a3fb4cd176cec8cedbc4e Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 20 Mar 2020 00:02:42 +0100 Subject: [PATCH 22/54] Add initial test for login --- tests/auth/providers/test_openid.py | 115 ++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 tests/auth/providers/test_openid.py diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py new file mode 100644 index 00000000000000..6cc00f3b89064f --- /dev/null +++ b/tests/auth/providers/test_openid.py @@ -0,0 +1,115 @@ +"""Test openid auth provider.""" +from datetime import datetime, timezone + +from asynctest import patch +from jose import jwt +import pytest + +from homeassistant import data_entry_flow +from homeassistant.auth import auth_manager_from_config +from homeassistant.helpers.config_entry_oauth2_flow import _encode_jwt +from homeassistant.setup import async_setup_component + +CONST_CLIENT_ID = "123client_id456" +CONST_CLIENT_SECRET = "123client_secret456" + +CONST_JWKS_URI = "https://jwks.test/jwks" +CONST_JWKS_KEY = "bla" +CONST_JWKS = {"keys": [CONST_JWKS_KEY]} + +CONST_AUTHORIZATION_ENDPOINT = "https://openid.test/authorize" +CONST_TOKEN_ENDPOINT = "https://openid.test/authorize" + +CONST_DESCRIPTION_URI = "https://openid.test/.well-known/openid-configuration" +CONST_DESCRIPTION = { + "issuer": "https://openid.test/", + "jwks_uri": CONST_JWKS_URI, + "authorization_endpoint": CONST_AUTHORIZATION_ENDPOINT, + "token_endpoint": CONST_TOKEN_ENDPOINT, + "scopes_supported": ["openid", "email", "profile"], +} + +CONST_ACCESS_TOKEN = "dummy_access_token" + +CONST_NONCE = "dummy_nonce" +CONST_EMAIL = "john.doe@openid.test" + +CONST_ID_TOKEN = { + "iss": "https://openid.test/", + "sub": "248289761001", + "aud": CONST_CLIENT_ID, + "nonce": CONST_NONCE, + "exp": datetime(2099, 1, 1, tzinfo=timezone.utc).timestamp(), + "iat": datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp(), + "name": "John Doe", + "email": CONST_EMAIL, +} + + +@pytest.fixture(name="openid_server") +async def openid_server_fixture(hass, aioclient_mock): + """Mock openid server.""" + aioclient_mock.get( + CONST_DESCRIPTION_URI, json=CONST_DESCRIPTION, + ) + + aioclient_mock.get( + CONST_JWKS_URI, json=CONST_JWKS, + ) + + aioclient_mock.post( + CONST_TOKEN_ENDPOINT, + json={ + "access_token": CONST_ACCESS_TOKEN, + "type": "bearer", + "expires_in": 60, + "id_token": jwt.encode( + CONST_ID_TOKEN, CONST_JWKS_KEY, access_token=CONST_ACCESS_TOKEN + ), + }, + ) + + +async def test_login_flow_validates(hass, aiohttp_client, openid_server): + """Test login flow.""" + assert await async_setup_component(hass, "http", {}) + + hass.config.api.base_url = "https://example.com" + + manager = await auth_manager_from_config( + hass, + [ + { + "type": "openid", + "issuer": "https://openid.test/", + "client_id": CONST_CLIENT_ID, + "client_secret": CONST_CLIENT_SECRET, + "emails": [CONST_EMAIL], + } + ], + [], + ) + hass.auth = manager + + with patch("homeassistant.auth.providers.openid.token_hex") as token_hex: + token_hex.return_value = CONST_NONCE + result = await manager.login_flow.async_init(("openid", None)) + + state = _encode_jwt(hass, {"flow_id": result["flow_id"], "flow_type": "login"}) + + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["url"] == ( + f"{CONST_AUTHORIZATION_ENDPOINT}?response_type=code&client_id={CONST_CLIENT_ID}" + "&redirect_uri=https://example.com/auth/external/callback" + f"&state={state}&scope=email+openid+profile&nonce={CONST_NONCE}" + ) + + client = await aiohttp_client(hass.http.app) + resp = await client.get(f"/auth/external/callback?code=abcd&state={state}") + assert resp.status == 200 + assert resp.headers["content-type"] == "text/html; charset=utf-8" + + result = await manager.login_flow.async_configure(result["flow_id"]) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["email"] == CONST_EMAIL From 43e5d86ca1fc397ae3042759822e564be14edae8 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 20 Mar 2020 00:23:34 +0100 Subject: [PATCH 23/54] Since jwt must be imported where used This module is using the REQUIREMENTS const description --- homeassistant/auth/providers/openid.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 119f6896879024..e00cc9b676e55f 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -7,7 +7,6 @@ from aiohttp import ClientResponseError from aiohttp.client import ClientResponse -from jose import jwt import voluptuous as vol from yarl import URL @@ -142,6 +141,8 @@ async def async_validate_token( self, token: Dict[str, Any], nonce: str ) -> Dict[str, Any]: """Validate a token.""" + from jose import jwt + id_token = cast( Dict[str, Any], jwt.decode( From ab0693730df7b34ee7f2d4ead0ba9cd7fd96aece Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 20 Mar 2020 00:58:03 +0100 Subject: [PATCH 24/54] Drop unneded init --- homeassistant/auth/providers/openid.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index e00cc9b676e55f..f676dffbd70ea0 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -75,10 +75,6 @@ class OpenIdAuthProvider(AuthProvider): _discovery_document: Dict[str, Any] _jwks: Dict[str, Any] - def __init__(self, *args: Any, **kwargs: Any) -> None: - """Extend parent's __init__.""" - super().__init__(*args, **kwargs) - async def async_get_discovery_document(self) -> None: """Cache discovery document for openid.""" if hasattr(self, "._discovery_document"): From 38c25fb33b7c8bbb90c70f56020b4adf84835d7d Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Fri, 20 Mar 2020 22:59:02 +0100 Subject: [PATCH 25/54] Trigger a message in opening window when --- homeassistant/helpers/config_entry_oauth2_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/helpers/config_entry_oauth2_flow.py b/homeassistant/helpers/config_entry_oauth2_flow.py index 95005d7ef2e05e..158df071907751 100644 --- a/homeassistant/helpers/config_entry_oauth2_flow.py +++ b/homeassistant/helpers/config_entry_oauth2_flow.py @@ -419,7 +419,7 @@ async def get(self, request: web.Request) -> web.Response: return web.Response( headers={"content-type": "text/html"}, - text="", + text="", ) From f6ff5aebc186e4cef5ddf8ca3b02a0a35d69f9da Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 21 Mar 2020 16:42:18 +0100 Subject: [PATCH 26/54] Add some logging for openid on login --- homeassistant/auth/providers/openid.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index f676dffbd70ea0..38edd706fc6a71 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -187,8 +187,10 @@ async def async_get_or_create_credentials( for credential in await self.async_credentials(): if credential.data["sub"] == subject: + _LOGGER.info("Accepting credential for %s", flow_result["email"]) return credential + _LOGGER.info("Creating credential for %s", flow_result["email"]) return self.async_create_credentials(flow_result) async def async_user_meta_for_credentials( From e254751e7714cb6c7b07ae81ff6ab38be050d8ab Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 2 May 2020 20:58:57 +0200 Subject: [PATCH 27/54] Specify sets with modern python syntax --- homeassistant/auth/providers/openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 38edd706fc6a71..e16897e427d58a 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -63,7 +63,7 @@ async def raise_for_status(response: ClientResponse) -> None: raise InvalidAuthError(data) from standard -WANTED_SCOPES = set(["openid", "email", "profile"]) +WANTED_SCOPES = {"openid", "email", "profile"} @AUTH_PROVIDERS.register("openid") From febb8c324a11ef962f8439289f8bfa25eadf4b53 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 2 May 2020 21:07:48 +0200 Subject: [PATCH 28/54] Specify signing algorithms and issuer --- homeassistant/auth/providers/openid.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index e16897e427d58a..7eba83157c04f9 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -139,10 +139,18 @@ async def async_validate_token( """Validate a token.""" from jose import jwt + algorithms = self._discovery_document.get( + "id_token_signing_alg_values_supported", ["RS256"] + ) + + issuer = self._discovery_document.get("issuer", None) + id_token = cast( Dict[str, Any], jwt.decode( token["id_token"], + algorithms=algorithms, + issuer=issuer, key=self._jwks, audience=self.config[CONF_CLIENT_ID], access_token=token["access_token"], From 48762c33681ab86516fb63e8c5a5568f473ad1cc Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 2 May 2020 21:30:40 +0200 Subject: [PATCH 29/54] These are mandatory fields --- homeassistant/auth/providers/openid.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 7eba83157c04f9..05ad0e2a4f177f 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -139,11 +139,8 @@ async def async_validate_token( """Validate a token.""" from jose import jwt - algorithms = self._discovery_document.get( - "id_token_signing_alg_values_supported", ["RS256"] - ) - - issuer = self._discovery_document.get("issuer", None) + algorithms = self._discovery_document["id_token_signing_alg_values_supported"] + issuer = self._discovery_document["issuer"] id_token = cast( Dict[str, Any], From fe8a315adc5ea2f8f42a566656927bc6d57c74ad Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 2 May 2020 22:15:28 +0200 Subject: [PATCH 30/54] Validate discovery document, and specify url directly --- homeassistant/auth/providers/openid.py | 57 ++++++++++++++------------ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 05ad0e2a4f177f..fdd3e01005becb 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -25,16 +25,14 @@ _LOGGER = logging.getLogger(__name__) -CONF_ISSUER = "issuer" CONF_CLIENT_ID = "client_id" CONF_CLIENT_SECRET = "client_secret" -CONF_TOKEN_URI = "token_uri" -CONF_AUTHORIZATION_URI = "authorization_uri" +CONF_CONFIGURATION = "configuration" CONF_EMAILS = "emails" CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend( { - vol.Required(CONF_ISSUER): str, + vol.Required(CONF_CONFIGURATION): str, vol.Required(CONF_CLIENT_ID): str, vol.Required(CONF_CLIENT_SECRET): str, vol.Required(CONF_EMAILS): [str], @@ -42,7 +40,21 @@ extra=vol.PREVENT_EXTRA, ) -OPENID_CONFIGURATION_PATH = ".well-known/openid-configuration" +OPENID_CONFIGURATION_SCHEMA = vol.Schema( + { + vol.Required("issuer"): str, + vol.Required("jwks_uri"): str, + vol.Required("id_token_signing_alg_values_supported"): list, + vol.Optional("scopes_supported"): list, + vol.Required("token_endpoint"): str, + vol.Required("authorization_endpoint"): str, + vol.Required("response_types_supported"): vol.Contains("code"), + vol.Required("token_endpoint_auth_methods_supported"): vol.Contains( + "client_secret_post" + ), + }, + extra=vol.ALLOW_EXTRA, +) class InvalidAuthError(HomeAssistantError): @@ -72,18 +84,20 @@ class OpenIdAuthProvider(AuthProvider): DEFAULT_TITLE = "OpenId Connect" - _discovery_document: Dict[str, Any] + _configuration: Dict[str, Any] _jwks: Dict[str, Any] - async def async_get_discovery_document(self) -> None: + async def async_get_configuration(self) -> None: """Cache discovery document for openid.""" - if hasattr(self, "._discovery_document"): + if hasattr(self, "._configuration"): return session = async_get_clientsession(self.hass) - async with session.get(self.discovery_url) as response: + async with session.get(self.config[CONF_CONFIGURATION]) as response: await raise_for_status(response) - self._discovery_document = cast(Dict[str, Any], await response.json()) + data = await response.json() + + self._configuration = OPENID_CONFIGURATION_SCHEMA(data) async def async_get_jwks(self) -> None: """Cache the keys for id verification.""" @@ -91,15 +105,10 @@ async def async_get_jwks(self) -> None: return session = async_get_clientsession(self.hass) - async with session.get(self._discovery_document["jwks_uri"]) as response: + async with session.get(self._configuration["jwks_uri"]) as response: await raise_for_status(response) self._jwks = cast(Dict[str, Any], await response.json()) - @property - def discovery_url(self) -> str: - """Construct discovery url based on config.""" - return str(URL(self.config[CONF_ISSUER]).with_path(OPENID_CONFIGURATION_PATH)) - @property def redirect_uri(self) -> str: """Return the redirect uri.""" @@ -108,7 +117,7 @@ def redirect_uri(self) -> str: async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" - await self.async_get_discovery_document() + await self.async_get_configuration() await self.async_get_jwks() async_register_view(self.hass) @@ -128,7 +137,7 @@ async def async_retrieve_token(self, code: str) -> Dict[str, Any]: session = async_get_clientsession(self.hass) async with session.post( - self._discovery_document["token_endpoint"], data=payload + self._configuration["token_endpoint"], data=payload ) as response: await raise_for_status(response) return cast(Dict[str, Any], await response.json()) @@ -139,8 +148,8 @@ async def async_validate_token( """Validate a token.""" from jose import jwt - algorithms = self._discovery_document["id_token_signing_alg_values_supported"] - issuer = self._discovery_document["issuer"] + algorithms = self._configuration["id_token_signing_alg_values_supported"] + issuer = self._configuration["issuer"] id_token = cast( Dict[str, Any], @@ -162,9 +171,7 @@ async def async_validate_token( async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: """Generate a authorization url for a given flow.""" - scopes = WANTED_SCOPES.intersection( - self._discovery_document["scopes_supported"] - ) + scopes = WANTED_SCOPES.intersection(self._configuration["scopes_supported"]) query = { "response_type": "code", @@ -175,9 +182,7 @@ async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: "nonce": nonce, } - return str( - URL(self._discovery_document["authorization_endpoint"]).with_query(query) - ) + return str(URL(self._configuration["authorization_endpoint"]).with_query(query)) @property def support_mfa(self) -> bool: From 5bf36a7efb4c7a9057e1aab4f7296878214a0999 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 2 May 2020 22:26:26 +0200 Subject: [PATCH 31/54] Validate grant types and defaults --- homeassistant/auth/providers/openid.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index fdd3e01005becb..4a09a5ec5d817a 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -45,13 +45,16 @@ vol.Required("issuer"): str, vol.Required("jwks_uri"): str, vol.Required("id_token_signing_alg_values_supported"): list, - vol.Optional("scopes_supported"): list, + vol.Optional("scopes_supported"): vol.Contains("openid"), vol.Required("token_endpoint"): str, vol.Required("authorization_endpoint"): str, vol.Required("response_types_supported"): vol.Contains("code"), - vol.Required("token_endpoint_auth_methods_supported"): vol.Contains( - "client_secret_post" - ), + vol.Optional( + "token_endpoint_auth_methods_supported", default=["client_secret_basic"] + ): vol.Contains("client_secret_post"), + vol.Optional( + "grant_types_supported", default=["authorization_code", "implicit"] + ): vol.Contains("authorization_code"), }, extra=vol.ALLOW_EXTRA, ) From 77a057c0cbae3cdc2e95084fe2ddfc7b8cd46daa Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 10 May 2020 14:07:28 +0200 Subject: [PATCH 32/54] Correct tests for configuration validation cahnge to uri --- tests/auth/providers/test_openid.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index 6cc00f3b89064f..de4c45168fff1e 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -26,7 +26,10 @@ "jwks_uri": CONST_JWKS_URI, "authorization_endpoint": CONST_AUTHORIZATION_ENDPOINT, "token_endpoint": CONST_TOKEN_ENDPOINT, + "token_endpoint_auth_methods_supported": "client_secret_post", + "id_token_signing_alg_values_supported": ["RS256", "HS256"], "scopes_supported": ["openid", "email", "profile"], + "response_types_supported": "code", } CONST_ACCESS_TOKEN = "dummy_access_token" @@ -35,7 +38,7 @@ CONST_EMAIL = "john.doe@openid.test" CONST_ID_TOKEN = { - "iss": "https://openid.test/", + "iss": CONST_DESCRIPTION_URI, "sub": "248289761001", "aud": CONST_CLIENT_ID, "nonce": CONST_NONCE, @@ -81,7 +84,7 @@ async def test_login_flow_validates(hass, aiohttp_client, openid_server): [ { "type": "openid", - "issuer": "https://openid.test/", + "configuration": CONST_DESCRIPTION_URI, "client_id": CONST_CLIENT_ID, "client_secret": CONST_CLIENT_SECRET, "emails": [CONST_EMAIL], From a7bf08343c75e4ed25f07d98bbcf2a83494f0309 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 10 May 2020 15:41:17 +0200 Subject: [PATCH 33/54] Make sure we log authentication failures in log --- homeassistant/auth/providers/openid.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 4a09a5ec5d817a..5d3f0af8535773 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -166,10 +166,14 @@ async def async_validate_token( ), ) if id_token.get("nonce") != nonce: - raise InvalidAuthError(f"Nonce mismatch in id_token") + raise InvalidAuthError("Nonce mismatch in id_token") if id_token["email"] not in self.config[CONF_EMAILS]: raise InvalidAuthError(f"Email {id_token['email']} not in allowed users") + + if not id_token["email_verified"]: + raise InvalidAuthError(f"Email {id_token['email']} must be verified") + return id_token async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: @@ -261,6 +265,7 @@ async def async_step_authorize( try: token = await provider.async_retrieve_token(self.external_data) result = await provider.async_validate_token(token, self.nonce) - except InvalidAuthError: + except InvalidAuthError as error: + _LOGGER.error("Login failed: %s", str(error)) return self.async_abort(reason="invalid_auth") return await self.async_finish(result) From a6e6980f94c3a227b17435b743e560f3b5fb3de0 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 10 May 2020 16:15:35 +0200 Subject: [PATCH 34/54] Refactor functions getting configuration --- homeassistant/auth/providers/openid.py | 27 ++++++++++++-------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 5d3f0af8535773..17d9cf1962cc36 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -90,27 +90,21 @@ class OpenIdAuthProvider(AuthProvider): _configuration: Dict[str, Any] _jwks: Dict[str, Any] - async def async_get_configuration(self) -> None: - """Cache discovery document for openid.""" - if hasattr(self, "._configuration"): - return - + async def async_get_configuration(self) -> Dict[str, Any]: + """Get discovery document for openid.""" session = async_get_clientsession(self.hass) async with session.get(self.config[CONF_CONFIGURATION]) as response: await raise_for_status(response) data = await response.json() + return cast(Dict[str, Any], OPENID_CONFIGURATION_SCHEMA(data)) - self._configuration = OPENID_CONFIGURATION_SCHEMA(data) - - async def async_get_jwks(self) -> None: - """Cache the keys for id verification.""" - if hasattr(self, "._jwks"): - return - + async def async_get_jwks(self) -> Dict[str, Any]: + """Get the keys for id verification.""" session = async_get_clientsession(self.hass) async with session.get(self._configuration["jwks_uri"]) as response: await raise_for_status(response) - self._jwks = cast(Dict[str, Any], await response.json()) + data = await response.json() + return cast(Dict[str, Any], data) @property def redirect_uri(self) -> str: @@ -120,8 +114,11 @@ def redirect_uri(self) -> str: async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" - await self.async_get_configuration() - await self.async_get_jwks() + if not hasattr(self, "_configuration"): + self._configuration = await self.async_get_configuration() + + if not hasattr(self, "_jwks"): + self._jwks = await self.async_get_jwks() async_register_view(self.hass) From e95da8d3c9fea4e21043aafbed179d8d76769558 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 10 May 2020 18:31:27 +0200 Subject: [PATCH 35/54] Switch to get_url --- homeassistant/auth/providers/openid.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 17d9cf1962cc36..1de671c6d8f034 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -17,6 +17,7 @@ _encode_jwt, async_register_view, ) +from homeassistant.helpers.network import get_url from . import AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, AuthProvider, LoginFlow from ..models import Credentials, UserMeta @@ -109,7 +110,8 @@ async def async_get_jwks(self) -> Dict[str, Any]: @property def redirect_uri(self) -> str: """Return the redirect uri.""" - return f"{self.hass.config.api.base_url}{AUTH_CALLBACK_PATH}" # type: ignore + url = URL(get_url(self.hass, prefer_external=True, allow_cloud=False)) + return str(url.with_path(AUTH_CALLBACK_PATH)) async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" From ee1828e59eb7d308b37e0d9670ecca485b99e6d7 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 17 May 2020 11:24:41 +0200 Subject: [PATCH 36/54] Switch to new external_url instead of base_url --- tests/auth/providers/test_openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index de4c45168fff1e..595b751aed61b7 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -77,7 +77,7 @@ async def test_login_flow_validates(hass, aiohttp_client, openid_server): """Test login flow.""" assert await async_setup_component(hass, "http", {}) - hass.config.api.base_url = "https://example.com" + hass.config.external_url = "https://example.com" manager = await auth_manager_from_config( hass, From df897a03868d9f522866b30d6d3186dc40d9445b Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 17 May 2020 11:24:56 +0200 Subject: [PATCH 37/54] Set correct issuer for test --- tests/auth/providers/test_openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index 595b751aed61b7..0591d6a7520ba6 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -38,7 +38,7 @@ CONST_EMAIL = "john.doe@openid.test" CONST_ID_TOKEN = { - "iss": CONST_DESCRIPTION_URI, + "iss": "https://openid.test/", "sub": "248289761001", "aud": CONST_CLIENT_ID, "nonce": CONST_NONCE, From 85e9f00261a33770f4847ca452ee00ef8d0ad7ba Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 17 May 2020 11:27:47 +0200 Subject: [PATCH 38/54] Set email_verified --- tests/auth/providers/test_openid.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index 0591d6a7520ba6..f119dc81065a5c 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -46,6 +46,7 @@ "iat": datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp(), "name": "John Doe", "email": CONST_EMAIL, + "email_verified": True, } From 289252036a8ad1a85ebe2dcc757c06fe93d8a0f3 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 17 May 2020 15:20:48 +0200 Subject: [PATCH 39/54] Add linting exception since these files have no manifest --- homeassistant/auth/providers/openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 1de671c6d8f034..31fccd081930e6 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -148,7 +148,7 @@ async def async_validate_token( self, token: Dict[str, Any], nonce: str ) -> Dict[str, Any]: """Validate a token.""" - from jose import jwt + from jose import jwt # noqa: pylint: disable=import-outside-toplevel algorithms = self._configuration["id_token_signing_alg_values_supported"] issuer = self._configuration["issuer"] From b434ab7528a74728f3b3765c8b3bef44e9dd11a8 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 4 Jun 2020 22:17:53 +0200 Subject: [PATCH 40/54] Adjust default title --- homeassistant/auth/providers/openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 31fccd081930e6..2dd7c0739fbb7a 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -86,7 +86,7 @@ async def raise_for_status(response: ClientResponse) -> None: class OpenIdAuthProvider(AuthProvider): """Example auth provider based on hardcoded usernames and passwords.""" - DEFAULT_TITLE = "OpenId Connect" + DEFAULT_TITLE = "OpenID Connect" _configuration: Dict[str, Any] _jwks: Dict[str, Any] From 284edf19239af8c0a382b798412087ac25794c61 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 6 Jun 2020 11:56:26 +0200 Subject: [PATCH 41/54] Some more adjustments for casing for openid --- homeassistant/auth/providers/openid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 2dd7c0739fbb7a..6e2fee25de0316 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -1,4 +1,4 @@ -"""OpenId based authentication provider.""" +"""OpenID based authentication provider.""" import logging import re @@ -92,7 +92,7 @@ class OpenIdAuthProvider(AuthProvider): _jwks: Dict[str, Any] async def async_get_configuration(self) -> Dict[str, Any]: - """Get discovery document for openid.""" + """Get discovery document for OpenID.""" session = async_get_clientsession(self.hass) async with session.get(self.config[CONF_CONFIGURATION]) as response: await raise_for_status(response) From 18090b65eb19d8231696900e2c2eb5142814725c Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 6 Jun 2020 12:14:13 +0200 Subject: [PATCH 42/54] Log using subject instead of email --- homeassistant/auth/providers/openid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 6e2fee25de0316..4b481acf6cea47 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -203,10 +203,10 @@ async def async_get_or_create_credentials( for credential in await self.async_credentials(): if credential.data["sub"] == subject: - _LOGGER.info("Accepting credential for %s", flow_result["email"]) + _LOGGER.info("Accepting credential for %s", subject) return credential - _LOGGER.info("Creating credential for %s", flow_result["email"]) + _LOGGER.info("Creating credential for %s", subject) return self.async_create_credentials(flow_result) async def async_user_meta_for_credentials( From 88d3274d7d81bc5e09c785caf3456d3313dc2468 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 6 Jun 2020 12:19:10 +0200 Subject: [PATCH 43/54] Use preferred_username then given_name then name, then email --- homeassistant/auth/providers/openid.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 4b481acf6cea47..581f91f93f4c38 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -216,15 +216,21 @@ async def async_user_meta_for_credentials( Will be used to populate info when creating a new user. """ - email = credentials.data["email"] - if "name" in credentials.data: + if "preferred_username" in credentials.data: + name = credentials.data["preferred_username"] + elif "given_name" in credentials.data: + name = credentials.data["given_name"] + elif "name" in credentials.data: name = credentials.data["name"] - else: + elif "email" in credentials.data: + email = credentials.data["email"] match = re.match(r"[^@]+", email) if match: name = str(match.group(0)) else: name = str(email) + else: + name = credentials.data["sub"] return UserMeta(name=name, is_active=True) From c335558d7addb9d5900872ed9f33dfa6d79f3e6f Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 6 Jun 2020 12:48:39 +0200 Subject: [PATCH 44/54] Support subject whitelist --- homeassistant/auth/providers/openid.py | 18 ++++--- tests/auth/providers/test_openid.py | 70 +++++++++++++++++++------- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 581f91f93f4c38..415cdeef2f8adc 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -30,13 +30,15 @@ CONF_CLIENT_SECRET = "client_secret" CONF_CONFIGURATION = "configuration" CONF_EMAILS = "emails" +CONF_SUBJECTS = "subjects" CONFIG_SCHEMA = AUTH_PROVIDER_SCHEMA.extend( { vol.Required(CONF_CONFIGURATION): str, vol.Required(CONF_CLIENT_ID): str, vol.Required(CONF_CLIENT_SECRET): str, - vol.Required(CONF_EMAILS): [str], + vol.Optional(CONF_EMAILS, default=[]): [str], + vol.Optional(CONF_SUBJECTS, default=[]): [str], }, extra=vol.PREVENT_EXTRA, ) @@ -167,13 +169,17 @@ async def async_validate_token( if id_token.get("nonce") != nonce: raise InvalidAuthError("Nonce mismatch in id_token") - if id_token["email"] not in self.config[CONF_EMAILS]: - raise InvalidAuthError(f"Email {id_token['email']} not in allowed users") + if id_token["sub"] in self.config[CONF_SUBJECTS]: + return id_token - if not id_token["email_verified"]: - raise InvalidAuthError(f"Email {id_token['email']} must be verified") + if "email" in id_token and "email_verified" in id_token: + if ( + id_token["email"] in self.config[CONF_EMAILS] + and id_token["email_verified"] + ): + return id_token - return id_token + raise InvalidAuthError(f"Subject {id_token['sub']} is not allowed") async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: """Generate a authorization url for a given flow.""" diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index f119dc81065a5c..b3e1c0d847e66e 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -36,10 +36,11 @@ CONST_NONCE = "dummy_nonce" CONST_EMAIL = "john.doe@openid.test" +CONST_SUBJECT = "248289761001" CONST_ID_TOKEN = { "iss": "https://openid.test/", - "sub": "248289761001", + "sub": CONST_SUBJECT, "aud": CONST_CLIENT_ID, "nonce": CONST_NONCE, "exp": datetime(2099, 1, 1, tzinfo=timezone.utc).timestamp(), @@ -74,7 +75,29 @@ async def openid_server_fixture(hass, aioclient_mock): ) -async def test_login_flow_validates(hass, aiohttp_client, openid_server): +async def _run_external_flow(hass, manager, aiohttp_client): + with patch("homeassistant.auth.providers.openid.token_hex") as token_hex: + token_hex.return_value = CONST_NONCE + result = await manager.login_flow.async_init(("openid", None)) + + state = _encode_jwt(hass, {"flow_id": result["flow_id"], "flow_type": "login"}) + + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["url"] == ( + f"{CONST_AUTHORIZATION_ENDPOINT}?response_type=code&client_id={CONST_CLIENT_ID}" + "&redirect_uri=https://example.com/auth/external/callback" + f"&state={state}&scope=email+openid+profile&nonce={CONST_NONCE}" + ) + + client = await aiohttp_client(hass.http.app) + resp = await client.get(f"/auth/external/callback?code=abcd&state={state}") + assert resp.status == 200 + assert resp.headers["content-type"] == "text/html; charset=utf-8" + + return result["flow_id"] + + +async def test_login_flow_validates_email(hass, aiohttp_client, openid_server): """Test login flow.""" assert await async_setup_component(hass, "http", {}) @@ -95,25 +118,38 @@ async def test_login_flow_validates(hass, aiohttp_client, openid_server): ) hass.auth = manager - with patch("homeassistant.auth.providers.openid.token_hex") as token_hex: - token_hex.return_value = CONST_NONCE - result = await manager.login_flow.async_init(("openid", None)) + flow_id = await _run_external_flow(hass, manager, aiohttp_client) - state = _encode_jwt(hass, {"flow_id": result["flow_id"], "flow_type": "login"}) + result = await manager.login_flow.async_configure(flow_id) - assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP - assert result["url"] == ( - f"{CONST_AUTHORIZATION_ENDPOINT}?response_type=code&client_id={CONST_CLIENT_ID}" - "&redirect_uri=https://example.com/auth/external/callback" - f"&state={state}&scope=email+openid+profile&nonce={CONST_NONCE}" + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["email"] == CONST_EMAIL + + +async def test_login_flow_validates_subject(hass, aiohttp_client, openid_server): + """Test login flow.""" + assert await async_setup_component(hass, "http", {}) + + hass.config.external_url = "https://example.com" + + manager = await auth_manager_from_config( + hass, + [ + { + "type": "openid", + "configuration": CONST_DESCRIPTION_URI, + "client_id": CONST_CLIENT_ID, + "client_secret": CONST_CLIENT_SECRET, + "subjects": [CONST_SUBJECT], + } + ], + [], ) + hass.auth = manager - client = await aiohttp_client(hass.http.app) - resp = await client.get(f"/auth/external/callback?code=abcd&state={state}") - assert resp.status == 200 - assert resp.headers["content-type"] == "text/html; charset=utf-8" + flow_id = await _run_external_flow(hass, manager, aiohttp_client) - result = await manager.login_flow.async_configure(result["flow_id"]) + result = await manager.login_flow.async_configure(flow_id) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["data"]["email"] == CONST_EMAIL + assert result["data"]["sub"] == CONST_SUBJECT From a977c4bc6e718d3e7243ebbefd1e8552bbdaa3c2 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 6 Jun 2020 12:49:55 +0200 Subject: [PATCH 45/54] Add a test for non whitelisted user --- tests/auth/providers/test_openid.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index b3e1c0d847e66e..7226c5d0d1ffa8 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -153,3 +153,31 @@ async def test_login_flow_validates_subject(hass, aiohttp_client, openid_server) assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY assert result["data"]["sub"] == CONST_SUBJECT + + +async def test_login_flow_not_whitelisted(hass, aiohttp_client, openid_server): + """Test login flow.""" + assert await async_setup_component(hass, "http", {}) + + hass.config.external_url = "https://example.com" + + manager = await auth_manager_from_config( + hass, + [ + { + "type": "openid", + "configuration": CONST_DESCRIPTION_URI, + "client_id": CONST_CLIENT_ID, + "client_secret": CONST_CLIENT_SECRET, + "subjects": [], + } + ], + [], + ) + hass.auth = manager + + flow_id = await _run_external_flow(hass, manager, aiohttp_client) + + result = await manager.login_flow.async_configure(flow_id) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT From 60615f0171afe6087f0e98b7f10701d52693651b Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 18 Jul 2020 13:24:56 +0200 Subject: [PATCH 46/54] Don't use a default list It is the same list everywhere can could potentially be changed. --- homeassistant/auth/providers/openid.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 415cdeef2f8adc..356b6ed1abacf9 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -37,8 +37,8 @@ vol.Required(CONF_CONFIGURATION): str, vol.Required(CONF_CLIENT_ID): str, vol.Required(CONF_CLIENT_SECRET): str, - vol.Optional(CONF_EMAILS, default=[]): [str], - vol.Optional(CONF_SUBJECTS, default=[]): [str], + vol.Optional(CONF_EMAILS): [str], + vol.Optional(CONF_SUBJECTS): [str], }, extra=vol.PREVENT_EXTRA, ) @@ -169,12 +169,12 @@ async def async_validate_token( if id_token.get("nonce") != nonce: raise InvalidAuthError("Nonce mismatch in id_token") - if id_token["sub"] in self.config[CONF_SUBJECTS]: + if id_token["sub"] in self.config.get(CONF_SUBJECTS, []): return id_token if "email" in id_token and "email_verified" in id_token: if ( - id_token["email"] in self.config[CONF_EMAILS] + id_token["email"] in self.config.get(CONF_EMAILS, []) and id_token["email_verified"] ): return id_token From bc70f60f16c2f20168fe87c52095e1b974fd8ab3 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 18 Jul 2020 13:38:10 +0200 Subject: [PATCH 47/54] Adjust stale doc string --- homeassistant/auth/providers/openid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 356b6ed1abacf9..c6331f99bb026e 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -86,7 +86,7 @@ async def raise_for_status(response: ClientResponse) -> None: @AUTH_PROVIDERS.register("openid") class OpenIdAuthProvider(AuthProvider): - """Example auth provider based on hardcoded usernames and passwords.""" + """Auth provider using openid connect as the authentication source.""" DEFAULT_TITLE = "OpenID Connect" From 93401e36cc07b246c0c47752948f8ee65a91ab36 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sat, 18 Jul 2020 13:41:49 +0200 Subject: [PATCH 48/54] Simplify name extraction from email --- homeassistant/auth/providers/openid.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index c6331f99bb026e..ef51af0f94ad03 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -1,7 +1,6 @@ """OpenID based authentication provider.""" import logging -import re from secrets import token_hex from typing import Any, Dict, Optional, cast @@ -229,12 +228,7 @@ async def async_user_meta_for_credentials( elif "name" in credentials.data: name = credentials.data["name"] elif "email" in credentials.data: - email = credentials.data["email"] - match = re.match(r"[^@]+", email) - if match: - name = str(match.group(0)) - else: - name = str(email) + name = cast(str, credentials.data["email"]).split("@", 1)[0] else: name = credentials.data["sub"] From 1c112d062adddcb74c660910fb82754dc616ab98 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 22 Mar 2020 18:12:50 +0100 Subject: [PATCH 49/54] Use standard OAuth helper --- homeassistant/auth/providers/openid.py | 68 +++++++------------ .../components/cloud/account_link.py | 10 ++- .../helpers/config_entry_oauth2_flow.py | 42 ++++++++---- 3 files changed, 61 insertions(+), 59 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index ef51af0f94ad03..2d97fc3abb67b4 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -7,16 +7,14 @@ from aiohttp import ClientResponseError from aiohttp.client import ClientResponse import voluptuous as vol -from yarl import URL from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.config_entry_oauth2_flow import ( - AUTH_CALLBACK_PATH, - _encode_jwt, + AbstractOAuth2Implementation, + LocalOAuth2Implementation, async_register_view, ) -from homeassistant.helpers.network import get_url from . import AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, AuthProvider, LoginFlow from ..models import Credentials, UserMeta @@ -91,6 +89,7 @@ class OpenIdAuthProvider(AuthProvider): _configuration: Dict[str, Any] _jwks: Dict[str, Any] + oauth2: AbstractOAuth2Implementation async def async_get_configuration(self) -> Dict[str, Any]: """Get discovery document for OpenID.""" @@ -109,10 +108,13 @@ async def async_get_jwks(self) -> Dict[str, Any]: return cast(Dict[str, Any], data) @property - def redirect_uri(self) -> str: - """Return the redirect uri.""" - url = URL(get_url(self.hass, prefer_external=True, allow_cloud=False)) - return str(url.with_path(AUTH_CALLBACK_PATH)) + def scope(self) -> str: + """Return scopes wanted from endpoint.""" + return " ".join( + sorted( + WANTED_SCOPES.intersection(self._configuration["scopes_supported"]) + ) + ) async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" @@ -123,28 +125,19 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: if not hasattr(self, "_jwks"): self._jwks = await self.async_get_jwks() + self.oauth2 = LocalOAuth2Implementation( + self.hass, + "auth", + self.config[CONF_CLIENT_ID], + self.config[CONF_CLIENT_SECRET], + self._configuration["authorization_endpoint"], + self._configuration["token_endpoint"], + ) + async_register_view(self.hass) return OpenIdLoginFlow(self) - async def async_retrieve_token(self, code: str) -> Dict[str, Any]: - """Convert a token code into an actual token.""" - - payload = { - "grant_type": "authorization_code", - "code": code, - "redirect_uri": self.redirect_uri, - "client_id": self.config[CONF_CLIENT_ID], - "client_secret": self.config[CONF_CLIENT_SECRET], - } - - session = async_get_clientsession(self.hass) - async with session.post( - self._configuration["token_endpoint"], data=payload - ) as response: - await raise_for_status(response) - return cast(Dict[str, Any], await response.json()) - async def async_validate_token( self, token: Dict[str, Any], nonce: str ) -> Dict[str, Any]: @@ -180,21 +173,6 @@ async def async_validate_token( raise InvalidAuthError(f"Subject {id_token['sub']} is not allowed") - async def async_generate_authorize_url(self, flow_id: str, nonce: str) -> str: - """Generate a authorization url for a given flow.""" - scopes = WANTED_SCOPES.intersection(self._configuration["scopes_supported"]) - - query = { - "response_type": "code", - "client_id": self.config["client_id"], - "redirect_uri": self.redirect_uri, - "state": _encode_jwt(self.hass, {"flow_id": flow_id, "flow_type": "login"}), - "scope": " ".join(sorted(scopes)), - "nonce": nonce, - } - - return str(URL(self._configuration["authorization_endpoint"]).with_query(query)) - @property def support_mfa(self) -> bool: """Return whether multi-factor auth supported by the auth provider.""" @@ -258,7 +236,9 @@ async def async_step_authenticate( self.external_data = str(user_input) return self.async_external_step_done(next_step_id="authorize") self.nonce = token_hex() - url = await provider.async_generate_authorize_url(self.flow_id, self.nonce) + url = await provider.oauth2.async_generate_authorize_url( + self.flow_id, flow_type="login", nonce=self.nonce, scope=provider.scope + ) return self.async_external_step(step_id="authenticate", url=url) async def async_step_authorize( @@ -268,7 +248,9 @@ async def async_step_authorize( provider = cast(OpenIdAuthProvider, self._auth_provider) try: - token = await provider.async_retrieve_token(self.external_data) + token = await provider.oauth2.async_resolve_external_data( + self.external_data + ) result = await provider.async_validate_token(token, self.nonce) except InvalidAuthError as error: _LOGGER.error("Login failed: %s", str(error)) diff --git a/homeassistant/components/cloud/account_link.py b/homeassistant/components/cloud/account_link.py index 4a3a2dd77f828f..59028a449307a5 100644 --- a/homeassistant/components/cloud/account_link.py +++ b/homeassistant/components/cloud/account_link.py @@ -1,7 +1,7 @@ """Account linking via the cloud.""" import asyncio import logging -from typing import Any +from typing import Any, Optional import aiohttp from hass_nabucasa import account_link @@ -109,7 +109,13 @@ def domain(self) -> str: """Domain that is providing the implementation.""" return DOMAIN - async def async_generate_authorize_url(self, flow_id: str) -> str: + async def async_generate_authorize_url( + self, + flow_id: str, + flow_type: Optional[str] = None, + nonce: Optional[str] = None, + scope: Optional[str] = None, + ) -> str: """Generate a url for the user to authorize.""" helper = account_link.AuthorizeAccountHelper( self.hass.data[DOMAIN], self.service diff --git a/homeassistant/helpers/config_entry_oauth2_flow.py b/homeassistant/helpers/config_entry_oauth2_flow.py index 158df071907751..f4985dda58994f 100644 --- a/homeassistant/helpers/config_entry_oauth2_flow.py +++ b/homeassistant/helpers/config_entry_oauth2_flow.py @@ -48,7 +48,13 @@ def domain(self) -> str: """Domain that is providing the implementation.""" @abstractmethod - async def async_generate_authorize_url(self, flow_id: str) -> str: + async def async_generate_authorize_url( + self, + flow_id: str, + flow_type: Optional[str] = None, + nonce: Optional[str] = None, + scope: Optional[str] = None, + ) -> str: """Generate a url for the user to authorize. This step is called when a config flow is initialized. It should redirect the @@ -129,20 +135,28 @@ def extra_authorize_data(self) -> dict: """Extra data that needs to be appended to the authorize url.""" return {} - async def async_generate_authorize_url(self, flow_id: str) -> str: + async def async_generate_authorize_url( + self, + flow_id: str, + flow_type: Optional[str] = None, + nonce: Optional[str] = None, + scope: Optional[str] = None, + ) -> str: """Generate a url for the user to authorize.""" - return str( - URL(self.authorize_url) - .with_query( - { - "response_type": "code", - "client_id": self.client_id, - "redirect_uri": self.redirect_uri, - "state": _encode_jwt(self.hass, {"flow_id": flow_id}), - } - ) - .update_query(self.extra_authorize_data) - ) + query = { + "response_type": "code", + "client_id": self.client_id, + "redirect_uri": self.redirect_uri, + "state": _encode_jwt( + self.hass, {"flow_type": flow_type, "flow_id": flow_id} + ), + } + if nonce: + query["nonce"] = nonce + if scope: + query["scope"] = scope + + return str(URL(self.authorize_url).with_query(query).update_query(self.extra_authorize_data)) async def async_resolve_external_data(self, external_data: Any) -> dict: """Resolve the authorization code to tokens.""" From 31924eed81150399b12690775ac2989ab8d19ef9 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Mon, 20 Jul 2020 08:22:04 +0200 Subject: [PATCH 50/54] Adjust some orders to make tests pass --- homeassistant/helpers/config_entry_oauth2_flow.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/helpers/config_entry_oauth2_flow.py b/homeassistant/helpers/config_entry_oauth2_flow.py index f4985dda58994f..002ba3c1c021a9 100644 --- a/homeassistant/helpers/config_entry_oauth2_flow.py +++ b/homeassistant/helpers/config_entry_oauth2_flow.py @@ -148,13 +148,13 @@ async def async_generate_authorize_url( "client_id": self.client_id, "redirect_uri": self.redirect_uri, "state": _encode_jwt( - self.hass, {"flow_type": flow_type, "flow_id": flow_id} + self.hass, {"flow_id": flow_id, "flow_type": flow_type} ), } - if nonce: - query["nonce"] = nonce if scope: query["scope"] = scope + if nonce: + query["nonce"] = nonce return str(URL(self.authorize_url).with_query(query).update_query(self.extra_authorize_data)) From 44475e145cba2707795f13fd89ecf13920826ed5 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Mon, 12 Oct 2020 21:04:07 +0200 Subject: [PATCH 51/54] Reduce changes to core oauth2 flow --- homeassistant/auth/providers/openid.py | 103 +++++++++++++----- .../helpers/config_entry_oauth2_flow.py | 36 +++--- tests/auth/providers/test_openid.py | 42 ++++--- 3 files changed, 118 insertions(+), 63 deletions(-) diff --git a/homeassistant/auth/providers/openid.py b/homeassistant/auth/providers/openid.py index 2d97fc3abb67b4..32da2afcb44992 100644 --- a/homeassistant/auth/providers/openid.py +++ b/homeassistant/auth/providers/openid.py @@ -8,10 +8,10 @@ from aiohttp.client import ClientResponse import voluptuous as vol +from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.aiohttp_client import async_get_clientsession from homeassistant.helpers.config_entry_oauth2_flow import ( - AbstractOAuth2Implementation, LocalOAuth2Implementation, async_register_view, ) @@ -81,6 +81,49 @@ async def raise_for_status(response: ClientResponse) -> None: WANTED_SCOPES = {"openid", "email", "profile"} +class OpenIdLocalOAuth2Implementation(LocalOAuth2Implementation): + """Local OAuth2 implementation for Toon.""" + + _nonce: Optional[str] = None + _scope: str + + def __init__( + self, + hass: HomeAssistant, + client_id: str, + client_secret: str, + configuration: Dict[str, Any], + ): + """Initialize local auth implementation.""" + super().__init__( + hass, + "auth", + client_id, + client_secret, + configuration["authorization_endpoint"], + configuration["token_endpoint"], + "login", + ) + + self._scope = " ".join( + sorted(WANTED_SCOPES.intersection(configuration["scopes_supported"])) + ) + + @property + def extra_authorize_data(self) -> dict: + """Extra data that needs to be appended to the authorize url.""" + return {"scope": self._scope, "nonce": self._nonce} + + async def async_generate_authorize_url_with_nonce( + self, flow_id: str, nonce: str + ) -> str: + """Generate an authorize url with a given nonce.""" + self._nonce = nonce + url = await self.async_generate_authorize_url(flow_id) + self._nonce = None + return url + + @AUTH_PROVIDERS.register("openid") class OpenIdAuthProvider(AuthProvider): """Auth provider using openid connect as the authentication source.""" @@ -89,7 +132,7 @@ class OpenIdAuthProvider(AuthProvider): _configuration: Dict[str, Any] _jwks: Dict[str, Any] - oauth2: AbstractOAuth2Implementation + _oauth2: OpenIdLocalOAuth2Implementation async def async_get_configuration(self) -> Dict[str, Any]: """Get discovery document for OpenID.""" @@ -107,15 +150,6 @@ async def async_get_jwks(self) -> Dict[str, Any]: data = await response.json() return cast(Dict[str, Any], data) - @property - def scope(self) -> str: - """Return scopes wanted from endpoint.""" - return " ".join( - sorted( - WANTED_SCOPES.intersection(self._configuration["scopes_supported"]) - ) - ) - async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: """Return a flow to login.""" @@ -125,23 +159,19 @@ async def async_login_flow(self, context: Optional[Dict]) -> LoginFlow: if not hasattr(self, "_jwks"): self._jwks = await self.async_get_jwks() - self.oauth2 = LocalOAuth2Implementation( + self._oauth2 = OpenIdLocalOAuth2Implementation( self.hass, - "auth", self.config[CONF_CLIENT_ID], self.config[CONF_CLIENT_SECRET], - self._configuration["authorization_endpoint"], - self._configuration["token_endpoint"], + self._configuration, ) async_register_view(self.hass) return OpenIdLoginFlow(self) - async def async_validate_token( - self, token: Dict[str, Any], nonce: str - ) -> Dict[str, Any]: - """Validate a token.""" + def _decode_id_token(self, token: Dict[str, Any], nonce: str) -> Dict[str, Any]: + """Decode openid id_token.""" from jose import jwt # noqa: pylint: disable=import-outside-toplevel algorithms = self._configuration["id_token_signing_alg_values_supported"] @@ -161,6 +191,11 @@ async def async_validate_token( if id_token.get("nonce") != nonce: raise InvalidAuthError("Nonce mismatch in id_token") + return id_token + + def _authorize_id_token(self, id_token: Dict[str, Any]) -> Dict[str, Any]: + """Authorize an id_token according to our internal database.""" + if id_token["sub"] in self.config.get(CONF_SUBJECTS, []): return id_token @@ -173,6 +208,22 @@ async def async_validate_token( raise InvalidAuthError(f"Subject {id_token['sub']} is not allowed") + async def async_generate_authorize_url_with_nonce( + self, flow_id: str, nonce: str + ) -> str: + """Generate an authorize url with a given nonce.""" + return await self._oauth2.async_generate_authorize_url_with_nonce( + flow_id, nonce + ) + + async def async_authorize_external_data( + self, external_data: str, nonce: str + ) -> Dict[str, Any]: + """Authorize external data.""" + token = await self._oauth2.async_resolve_external_data(external_data) + id_token = self._decode_id_token(token, nonce) + return self._authorize_id_token(id_token) + @property def support_mfa(self) -> bool: """Return whether multi-factor auth supported by the auth provider.""" @@ -217,7 +268,7 @@ class OpenIdLoginFlow(LoginFlow): """Handler for the login flow.""" external_data: str - nonce: str + _nonce: str async def async_step_init( self, user_input: Optional[Dict[str, str]] = None @@ -235,9 +286,10 @@ async def async_step_authenticate( if user_input: self.external_data = str(user_input) return self.async_external_step_done(next_step_id="authorize") - self.nonce = token_hex() - url = await provider.oauth2.async_generate_authorize_url( - self.flow_id, flow_type="login", nonce=self.nonce, scope=provider.scope + + self._nonce = token_hex() + url = await provider.async_generate_authorize_url_with_nonce( + self.flow_id, self._nonce ) return self.async_external_step(step_id="authenticate", url=url) @@ -248,10 +300,9 @@ async def async_step_authorize( provider = cast(OpenIdAuthProvider, self._auth_provider) try: - token = await provider.oauth2.async_resolve_external_data( - self.external_data + result = await provider.async_authorize_external_data( + self.external_data, self._nonce ) - result = await provider.async_validate_token(token, self.nonce) except InvalidAuthError as error: _LOGGER.error("Login failed: %s", str(error)) return self.async_abort(reason="invalid_auth") diff --git a/homeassistant/helpers/config_entry_oauth2_flow.py b/homeassistant/helpers/config_entry_oauth2_flow.py index 002ba3c1c021a9..4422224aaeec28 100644 --- a/homeassistant/helpers/config_entry_oauth2_flow.py +++ b/homeassistant/helpers/config_entry_oauth2_flow.py @@ -51,9 +51,6 @@ def domain(self) -> str: async def async_generate_authorize_url( self, flow_id: str, - flow_type: Optional[str] = None, - nonce: Optional[str] = None, - scope: Optional[str] = None, ) -> str: """Generate a url for the user to authorize. @@ -106,6 +103,7 @@ def __init__( client_secret: str, authorize_url: str, token_url: str, + flow_type: Optional[str] = None, ): """Initialize local auth implementation.""" self.hass = hass @@ -114,6 +112,7 @@ def __init__( self.client_secret = client_secret self.authorize_url = authorize_url self.token_url = token_url + self.flow_type = flow_type @property def name(self) -> str: @@ -138,25 +137,22 @@ def extra_authorize_data(self) -> dict: async def async_generate_authorize_url( self, flow_id: str, - flow_type: Optional[str] = None, - nonce: Optional[str] = None, - scope: Optional[str] = None, ) -> str: """Generate a url for the user to authorize.""" - query = { - "response_type": "code", - "client_id": self.client_id, - "redirect_uri": self.redirect_uri, - "state": _encode_jwt( - self.hass, {"flow_id": flow_id, "flow_type": flow_type} - ), - } - if scope: - query["scope"] = scope - if nonce: - query["nonce"] = nonce - - return str(URL(self.authorize_url).with_query(query).update_query(self.extra_authorize_data)) + return str( + URL(self.authorize_url) + .with_query( + { + "response_type": "code", + "client_id": self.client_id, + "redirect_uri": self.redirect_uri, + "state": _encode_jwt( + self.hass, {"flow_id": flow_id, "flow_type": self.flow_type} + ), + } + ) + .update_query(self.extra_authorize_data) + ) async def async_resolve_external_data(self, external_data: Any) -> dict: """Resolve the authorization code to tokens.""" diff --git a/tests/auth/providers/test_openid.py b/tests/auth/providers/test_openid.py index 7226c5d0d1ffa8..33bfae87d538e9 100644 --- a/tests/auth/providers/test_openid.py +++ b/tests/auth/providers/test_openid.py @@ -7,6 +7,7 @@ from homeassistant import data_entry_flow from homeassistant.auth import auth_manager_from_config +from homeassistant.config import async_process_ha_core_config from homeassistant.helpers.config_entry_oauth2_flow import _encode_jwt from homeassistant.setup import async_setup_component @@ -55,11 +56,13 @@ async def openid_server_fixture(hass, aioclient_mock): """Mock openid server.""" aioclient_mock.get( - CONST_DESCRIPTION_URI, json=CONST_DESCRIPTION, + CONST_DESCRIPTION_URI, + json=CONST_DESCRIPTION, ) aioclient_mock.get( - CONST_JWKS_URI, json=CONST_JWKS, + CONST_JWKS_URI, + json=CONST_JWKS, ) aioclient_mock.post( @@ -75,6 +78,17 @@ async def openid_server_fixture(hass, aioclient_mock): ) +@pytest.fixture(name="endpoints") +async def endpoints_fixture(hass, current_request): + """Initialize the needed endpoints and redirects.""" + await async_process_ha_core_config( + hass, + {"external_url": "https://example.com"}, + ) + + assert await async_setup_component(hass, "http", {}) + + async def _run_external_flow(hass, manager, aiohttp_client): with patch("homeassistant.auth.providers.openid.token_hex") as token_hex: token_hex.return_value = CONST_NONCE @@ -97,12 +111,10 @@ async def _run_external_flow(hass, manager, aiohttp_client): return result["flow_id"] -async def test_login_flow_validates_email(hass, aiohttp_client, openid_server): +async def test_login_flow_validates_email( + hass, aiohttp_client, openid_server, endpoints +): """Test login flow.""" - assert await async_setup_component(hass, "http", {}) - - hass.config.external_url = "https://example.com" - manager = await auth_manager_from_config( hass, [ @@ -126,12 +138,10 @@ async def test_login_flow_validates_email(hass, aiohttp_client, openid_server): assert result["data"]["email"] == CONST_EMAIL -async def test_login_flow_validates_subject(hass, aiohttp_client, openid_server): +async def test_login_flow_validates_subject( + hass, aiohttp_client, openid_server, endpoints +): """Test login flow.""" - assert await async_setup_component(hass, "http", {}) - - hass.config.external_url = "https://example.com" - manager = await auth_manager_from_config( hass, [ @@ -155,12 +165,10 @@ async def test_login_flow_validates_subject(hass, aiohttp_client, openid_server) assert result["data"]["sub"] == CONST_SUBJECT -async def test_login_flow_not_whitelisted(hass, aiohttp_client, openid_server): +async def test_login_flow_not_whitelisted( + hass, aiohttp_client, openid_server, endpoints +): """Test login flow.""" - assert await async_setup_component(hass, "http", {}) - - hass.config.external_url = "https://example.com" - manager = await auth_manager_from_config( hass, [ From 470fae9a3e9f2891be1a4c25f35d2f3efc491e0a Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Mon, 12 Oct 2020 22:35:53 +0200 Subject: [PATCH 52/54] Drop unneded change in cloud oauth2 --- homeassistant/components/cloud/account_link.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/homeassistant/components/cloud/account_link.py b/homeassistant/components/cloud/account_link.py index 59028a449307a5..1cbedaa36cc94b 100644 --- a/homeassistant/components/cloud/account_link.py +++ b/homeassistant/components/cloud/account_link.py @@ -1,7 +1,7 @@ """Account linking via the cloud.""" import asyncio import logging -from typing import Any, Optional +from typing import Any import aiohttp from hass_nabucasa import account_link @@ -112,9 +112,6 @@ def domain(self) -> str: async def async_generate_authorize_url( self, flow_id: str, - flow_type: Optional[str] = None, - nonce: Optional[str] = None, - scope: Optional[str] = None, ) -> str: """Generate a url for the user to authorize.""" helper = account_link.AuthorizeAccountHelper( From 1c65a22a1250941689d876a8ed618e8c2ae7c9f4 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Mon, 12 Oct 2020 22:37:07 +0200 Subject: [PATCH 53/54] Adjust home connect test --- tests/components/home_connect/test_config_flow.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/components/home_connect/test_config_flow.py b/tests/components/home_connect/test_config_flow.py index 5d65df98e5bf10..3f225b87a9fd96 100644 --- a/tests/components/home_connect/test_config_flow.py +++ b/tests/components/home_connect/test_config_flow.py @@ -31,7 +31,9 @@ async def test_full_flow(hass, aiohttp_client, aioclient_mock, current_request): result = await hass.config_entries.flow.async_init( "home_connect", context={"source": config_entries.SOURCE_USER} ) - state = config_entry_oauth2_flow._encode_jwt(hass, {"flow_id": result["flow_id"]}) + state = config_entry_oauth2_flow._encode_jwt( + hass, {"flow_id": result["flow_id"], "flow_type": None} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP assert result["url"] == ( From bfa9abb9b669907658626ab9901eb849f3e91443 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Mon, 12 Oct 2020 22:53:42 +0200 Subject: [PATCH 54/54] Avoid changing state parameter for non login flow Avoids fixing tests --- homeassistant/helpers/config_entry_oauth2_flow.py | 7 ++++--- tests/components/home_connect/test_config_flow.py | 4 +--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/homeassistant/helpers/config_entry_oauth2_flow.py b/homeassistant/helpers/config_entry_oauth2_flow.py index 4422224aaeec28..491dbfe0978432 100644 --- a/homeassistant/helpers/config_entry_oauth2_flow.py +++ b/homeassistant/helpers/config_entry_oauth2_flow.py @@ -139,6 +139,9 @@ async def async_generate_authorize_url( flow_id: str, ) -> str: """Generate a url for the user to authorize.""" + state = {"flow_id": flow_id} + if self.flow_type: + state["flow_type"] = self.flow_type return str( URL(self.authorize_url) .with_query( @@ -146,9 +149,7 @@ async def async_generate_authorize_url( "response_type": "code", "client_id": self.client_id, "redirect_uri": self.redirect_uri, - "state": _encode_jwt( - self.hass, {"flow_id": flow_id, "flow_type": self.flow_type} - ), + "state": _encode_jwt(self.hass, state), } ) .update_query(self.extra_authorize_data) diff --git a/tests/components/home_connect/test_config_flow.py b/tests/components/home_connect/test_config_flow.py index 3f225b87a9fd96..5d65df98e5bf10 100644 --- a/tests/components/home_connect/test_config_flow.py +++ b/tests/components/home_connect/test_config_flow.py @@ -31,9 +31,7 @@ async def test_full_flow(hass, aiohttp_client, aioclient_mock, current_request): result = await hass.config_entries.flow.async_init( "home_connect", context={"source": config_entries.SOURCE_USER} ) - state = config_entry_oauth2_flow._encode_jwt( - hass, {"flow_id": result["flow_id"], "flow_type": None} - ) + state = config_entry_oauth2_flow._encode_jwt(hass, {"flow_id": result["flow_id"]}) assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP assert result["url"] == (