From baf3ac6b5a63da76fbe47cf6f81783518da38a34 Mon Sep 17 00:00:00 2001 From: Christiaan Goossens <9487666+christiaangoossens@users.noreply.github.com> Date: Tue, 14 Apr 2026 09:43:58 +0200 Subject: [PATCH] Fixes for known bugs in v1.0.0-rc1 (#241) * Fix #238 for same-site cookies * Redirect in Python + bump to rc2 --- custom_components/auth_oidc/__init__.py | 2 +- custom_components/auth_oidc/config/const.py | 4 +- .../auth_oidc/endpoints/injected_auth_page.py | 54 +++++++++-- custom_components/auth_oidc/manifest.json | 3 +- custom_components/auth_oidc/provider.py | 14 +-- .../auth_oidc/static/injection.js | 81 ++++++---------- pyproject.toml | 1 - tests/test_hass_auth_provider.py | 33 ++++++- tests/test_hass_webserver.py | 97 +++++++++++++++---- uv.lock | 2 - 10 files changed, 190 insertions(+), 101 deletions(-) diff --git a/custom_components/auth_oidc/__init__.py b/custom_components/auth_oidc/__init__.py index b8638b3..7fb452c 100644 --- a/custom_components/auth_oidc/__init__.py +++ b/custom_components/auth_oidc/__init__.py @@ -157,6 +157,6 @@ async def _setup_oidc_provider(hass: HomeAssistant, my_config: dict, display_nam _LOGGER.info("Registered OIDC views") # Inject OIDC code into the frontend for /auth/authorize for automatic redirect - await OIDCInjectedAuthPage.inject(hass) + await OIDCInjectedAuthPage.inject(hass, force_https) return True diff --git a/custom_components/auth_oidc/config/const.py b/custom_components/auth_oidc/config/const.py index 22626b5..0eef999 100644 --- a/custom_components/auth_oidc/config/const.py +++ b/custom_components/auth_oidc/config/const.py @@ -8,9 +8,7 @@ from typing import Any, Dict DEFAULT_TITLE = "OpenID Connect (SSO)" DOMAIN = "auth_oidc" -REPO_ROOT_URL = ( - "https://github.com/christiaangoossens/hass-oidc-auth/tree/v1.0.0-rc1" -) +REPO_ROOT_URL = "https://github.com/christiaangoossens/hass-oidc-auth/tree/v1.0.0-rc1" ## === ## Config keys diff --git a/custom_components/auth_oidc/endpoints/injected_auth_page.py b/custom_components/auth_oidc/endpoints/injected_auth_page.py index ba5c5c4..bb9b45e 100644 --- a/custom_components/auth_oidc/endpoints/injected_auth_page.py +++ b/custom_components/auth_oidc/endpoints/injected_auth_page.py @@ -1,12 +1,18 @@ """Injected authorization page, replacing the original""" +import base64 import logging from functools import partial -from homeassistant.components.http import HomeAssistantView, StaticPathConfig -from homeassistant.core import HomeAssistant +from urllib.parse import quote, unquote from aiohttp import web from aiofiles import open as async_open +from homeassistant.components.http import HomeAssistantView, StaticPathConfig +from homeassistant.core import HomeAssistant + +from .welcome import PATH as WELCOME_PATH +from ..tools.helpers import get_url + PATH = "/auth/authorize" _LOGGER = logging.getLogger(__name__) @@ -18,7 +24,7 @@ async def read_file(path: str) -> str: return await f.read() -async def frontend_injection(hass: HomeAssistant) -> None: +async def frontend_injection(hass: HomeAssistant, force_https: bool) -> None: """Inject new frontend code into /auth/authorize.""" router = hass.http.app.router frontend_path = None @@ -61,7 +67,7 @@ async def frontend_injection(hass: HomeAssistant) -> None: frontend_code = await read_file(frontend_path) # Inject JS and register that route - injection_js = "" + injection_js = "" frontend_code = frontend_code.replace("", f"{injection_js}") await hass.http.async_register_static_paths( @@ -80,7 +86,7 @@ async def frontend_injection(hass: HomeAssistant) -> None: ) # If everything is succesful, register a fake view that just returns the modified HTML - hass.http.register_view(OIDCInjectedAuthPage(frontend_code)) + hass.http.register_view(OIDCInjectedAuthPage(frontend_code, force_https)) _LOGGER.info("Performed OIDC frontend injection") @@ -91,18 +97,46 @@ class OIDCInjectedAuthPage(HomeAssistantView): url = PATH name = "auth:oidc:authorize_page" - def __init__(self, html: str) -> None: + def __init__(self, html: str, force_https: bool) -> None: """Initialize the injected auth page.""" self.html = html + self.force_https = force_https @staticmethod - async def inject(hass: HomeAssistant) -> None: + async def inject(hass: HomeAssistant, force_https: bool) -> None: """Inject the OIDC auth page into the frontend.""" try: - await frontend_injection(hass) + await frontend_injection(hass, force_https) except Exception as e: # pylint: disable=broad-except _LOGGER.error("Failed to inject OIDC auth page: %s", e) - async def get(self, _) -> web.Response: - """Return the screen""" + @staticmethod + def _should_do_oidc_redirect(req: web.Request) -> bool: + """Check if we should redirect to the OIDC flow.""" + if req.query.get("skip_oidc_redirect") == "true": + return False + + redirect_uri = req.query.get("redirect_uri") + if not redirect_uri: + return False + + # Handle both encoded and plain redirect_uri values. + decoded_redirect_uri = unquote(redirect_uri) + return "skip_oidc_redirect=true" not in decoded_redirect_uri + + def _get_welcome_redirect_location(self, req: web.Request) -> str: + """Build the welcome URL for the injected auth page redirect.""" + encoded_current_url = quote( + base64.b64encode(str(req.url).encode("utf-8")).decode("ascii") + ) + return get_url( + f"{WELCOME_PATH}?redirect_uri={encoded_current_url}", + self.force_https, + ) + + async def get(self, req: web.Request) -> web.Response: + """Return the original page or redirect into the OIDC flow.""" + if self._should_do_oidc_redirect(req): + raise web.HTTPFound(location=self._get_welcome_redirect_location(req)) + return web.Response(text=self.html, content_type="text/html") diff --git a/custom_components/auth_oidc/manifest.json b/custom_components/auth_oidc/manifest.json index 5b4bbcd..82832d2 100644 --- a/custom_components/auth_oidc/manifest.json +++ b/custom_components/auth_oidc/manifest.json @@ -16,8 +16,7 @@ "requirements": [ "aiofiles", "jinja2", - "bcrypt", "joserfc" ], - "version": "1.0.0-rc1" + "version": "1.0.0-rc2" } \ No newline at end of file diff --git a/custom_components/auth_oidc/provider.py b/custom_components/auth_oidc/provider.py index 590a97f..8e55e77 100644 --- a/custom_components/auth_oidc/provider.py +++ b/custom_components/auth_oidc/provider.py @@ -6,7 +6,6 @@ import logging from typing import Dict, Optional import asyncio -import bcrypt from homeassistant.auth import EVENT_USER_ADDED from homeassistant.auth.providers import ( AUTH_PROVIDERS, @@ -236,7 +235,7 @@ class OpenIDAuthProvider(AuthProvider): # Keep cookie lifetime aligned with state lifetime in storage (5 minutes). "set-cookie": f"{COOKIE_NAME}=" + state_id - + "; Path=/auth/; SameSite=Strict; HttpOnly; Max-Age=300" + + "; Path=/auth/; SameSite=Lax; HttpOnly; Max-Age=300" + secure_flag, } @@ -367,14 +366,6 @@ class OpenIdLoginFlow(LoginFlow): """Handler for the login flow.""" async def _finalize_user(self, state_id: str) -> AuthFlowResult: - # Verify a dummy hash to make it last a bit longer - # as security measure (limits the amount of attempts you have in 5 min) - # Similar to what the HomeAssistant auth provider does - dummy = b"$2b$12$CiuFGszHx9eNHxPuQcwBWez4CwDTOcLTX5CbOpV6gef2nYuXkY7BO" - bcrypt.checkpw(b"foo", dummy) - - # Actually look up the auth provider after, - # this doesn't take a lot of time (regardless of it's in there or not) sub = await self._auth_provider.async_get_subject(state_id) if sub: return await self.async_finish( @@ -396,11 +387,10 @@ class OpenIdLoginFlow(LoginFlow): state_cookie = req.cookies.get(COOKIE_NAME) if state_cookie: - _LOGGER.debug("State cookie found on login: %s", state_cookie) try: return await self._finalize_user(state_cookie) except InvalidAuthError: - pass + return self.async_abort(reason="oidc_cookie_invalid") # If no cookie is found, abort. # User should either be redirected or start manually on the welcome diff --git a/custom_components/auth_oidc/static/injection.js b/custom_components/auth_oidc/static/injection.js index d2e299b..95c556a 100644 --- a/custom_components/auth_oidc/static/injection.js +++ b/custom_components/auth_oidc/static/injection.js @@ -1,58 +1,19 @@ /** - * OIDC Frontend Redirect injection script - * This script is injected because the 'hass-oidc-auth' custom component is active. + * hass-oidc-auth - UX script to automatically select the Home Assistant auth provider when the "Login aborted" message is shown. */ -function attempt_oidc_redirect() { - // Get URL parameters - const urlParams = new URLSearchParams(window.location.search); +let authFlowElement = null - // Check if we have skip_oidc_redirect directly here - if (urlParams.get('skip_oidc_redirect') === 'true') { - // No console log because this is intended behavior - return; - } - - const originalUrl = urlParams.get('redirect_uri'); - if (!originalUrl) { - console.warn('[OIDC] No OAuth2 redirect_uri parameter found in the URL. Frontend redirect cancelled.'); - return; - } - - try { - // Parse the redirect URI - const redirectUrl = new URL(originalUrl); - - // Check if redirect URI has a query parameter to stop OIDC injection - if (redirectUrl.searchParams.get('skip_oidc_redirect') === 'true') { - // No console log because this is intended behavior - return; - } - } catch (error) { - console.error('[OIDC] Invalid redirect_uri parameter:', error); - } - - window.stop(); // Stop loading the current page before redirecting - - // Redirect to the OIDC auth URL - const base64encodeUrl = btoa(window.location.href); - const oidcAuthUrl = '/auth/oidc/welcome?redirect_uri=' + encodeURIComponent(base64encodeUrl); - window.location.href = oidcAuthUrl; -} - -function click_alternative_provider_instead() { - setTimeout(() => { - // Find ha-auth-flow - const authFlowElement = document.querySelector('ha-auth-flow'); +function update() { + // Find ha-auth-flow + authFlowElement = document.querySelector('ha-auth-flow'); if (!authFlowElement) { - console.warn("[OIDC] ha-auth-flow element not found. Not automatically selecting HA provider."); return; } // Check if the text "Login aborted" is present on the page if (!authFlowElement.innerText.includes('Login aborted')) { - console.warn("[OIDC] 'Login aborted' text not found. Not automatically selecting HA provider."); return; } @@ -60,7 +21,6 @@ function click_alternative_provider_instead() { const authProviderElement = document.querySelector('ha-pick-auth-provider'); if (!authProviderElement) { - console.warn("[OIDC] ha-pick-auth-provider not found. Not automatically selecting HA provider."); return; } @@ -72,11 +32,30 @@ function click_alternative_provider_instead() { } firstListItem.click(); - }, 500); } -// Run OIDC injection upon load -(() => { - attempt_oidc_redirect(); - click_alternative_provider_instead(); -})(); \ No newline at end of file +// Hide the content until ready +let ready = false +document.querySelector(".content").style.display = "none" + +const observer = new MutationObserver((mutationsList, observer) => { + update(); + + if (!ready) { + ready = Boolean(authFlowElement) + if (ready) { + document.querySelector(".content").style.display = "" + } + } +}) + +observer.observe(document.body, { childList: true, subtree: true }) + +setTimeout(() => { + if (!ready) { + console.warn("[hass-oidc-auth]: Document was not ready after 300ms seconds, showing content anyway.") + } + + // Force display the content + document.querySelector(".content").style.display = ""; +}, 300) \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 6636e4b..10f6812 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,6 @@ license = "MIT" dependencies = [ "aiofiles~=25.1", "jinja2~=3.1", - "bcrypt~=5.0", "joserfc~=1.6.0", ] readme = "README.md" diff --git a/tests/test_hass_auth_provider.py b/tests/test_hass_auth_provider.py index 4d2e6d7..baa80b5 100644 --- a/tests/test_hass_auth_provider.py +++ b/tests/test_hass_auth_provider.py @@ -93,7 +93,7 @@ async def test_provider_cookie_header_sets_secure_when_requested(hass: HomeAssis provider = hass.auth.get_auth_providers(DOMAIN)[0] cookie_header = provider.get_cookie_header("state-id", secure=True)["set-cookie"] - assert "SameSite=Strict" in cookie_header + assert "SameSite=Lax" in cookie_header assert "HttpOnly" in cookie_header assert "Secure" in cookie_header @@ -342,5 +342,36 @@ async def test_login_with_invalid_cookie_aborts(hass: HomeAssistant): result = await flow.async_step_init({}) + assert result["type"] == FlowResultType.ABORT + assert result["reason"] == "oidc_cookie_invalid" + + +@pytest.mark.asyncio +async def test_login_with_no_cookie_aborts(hass: HomeAssistant): + """Missing cookie should fail closed.""" + await setup( + hass, + { + CLIENT_ID: "dummy", + DISCOVERY_URL: MockOIDCServer.get_discovery_url(), + FEATURES: { + FEATURES_AUTOMATIC_PERSON_CREATION: False, + FEATURES_AUTOMATIC_USER_LINKING: False, + }, + }, + True, + ) + + provider = hass.auth.get_auth_providers(DOMAIN)[0] + flow = await provider.async_login_flow({}) + + fake_request = SimpleNamespace(cookies={}, remote="127.0.0.1") + with patch( + "custom_components.auth_oidc.provider.http.current_request" + ) as current_request: + current_request.get.return_value = fake_request + + result = await flow.async_step_init({}) + assert result["type"] == FlowResultType.ABORT assert result["reason"] == "no_oidc_cookie_found" diff --git a/tests/test_hass_webserver.py b/tests/test_hass_webserver.py index 8fac182..5a4adad 100644 --- a/tests/test_hass_webserver.py +++ b/tests/test_hass_webserver.py @@ -2,6 +2,7 @@ import base64 import os +from urllib.parse import parse_qs, quote, unquote, urlparse from unittest.mock import AsyncMock, MagicMock, patch from auth_oidc.config.const import DISCOVERY_URL, CLIENT_ID import pytest @@ -45,6 +46,22 @@ async def setup( assert result +async def setup_mock_authorize_route(hass: HomeAssistant) -> None: + """Register a mock /auth/authorize page so frontend injection can hook into it.""" + await async_setup_component(hass, HTTP_DOMAIN, {}) + + mock_html_path = os.path.join(os.path.dirname(__file__), "mocks", "auth_page.html") + await hass.http.async_register_static_paths( + [ + StaticPathConfig( + "/auth/authorize", + mock_html_path, + cache_headers=False, + ) + ] + ) + + @pytest.mark.asyncio async def test_welcome_page_registration(hass: HomeAssistant, hass_client): """Test that welcome page is present.""" @@ -87,7 +104,7 @@ async def test_welcome_rejects_invalid_encoded_redirect_uri( @pytest.mark.asyncio -async def test_welcome_sets_strict_state_cookie_flags(hass: HomeAssistant, hass_client): +async def test_welcome_sets_secure_state_cookie_flags(hass: HomeAssistant, hass_client): """Welcome should set secure cookie flags for the OIDC state cookie.""" await setup(hass) @@ -105,7 +122,7 @@ async def test_welcome_sets_strict_state_cookie_flags(hass: HomeAssistant, hass_ set_cookie = resp.headers.get("Set-Cookie", "") assert "Path=/auth/" in set_cookie - assert "SameSite=Strict" in set_cookie + assert "SameSite=Lax" in set_cookie assert "HttpOnly" in set_cookie assert "Max-Age=300" in set_cookie @@ -557,25 +574,14 @@ async def test_frontend_injection(hass: HomeAssistant, hass_client): """Test that frontend injection works.""" # Because there is no frontend in the test setup, - # we'll have to fake /auth/authorize for the changes to register - await async_setup_component(hass, HTTP_DOMAIN, {}) - - mock_html_path = os.path.join(os.path.dirname(__file__), "mocks", "auth_page.html") - await hass.http.async_register_static_paths( - [ - StaticPathConfig( - "/auth/authorize", - mock_html_path, - cache_headers=False, - ) - ] - ) + # we'll have to fake /auth/authorize for the changes to register. + await setup_mock_authorize_route(hass) await setup(hass) client = await hass_client() resp = await client.get("/auth/authorize", allow_redirects=False) - assert resp.status == 200 + assert resp.status == 200 # 200 because there is no redirect_uri text = await resp.text() assert "