1 Commits

Author SHA1 Message Date
Christiaan Goossens
baf3ac6b5a Fixes for known bugs in v1.0.0-rc1 (#241)
* Fix #238 for same-site cookies

* Redirect in Python + bump to rc2
2026-04-14 09:43:58 +02:00
10 changed files with 190 additions and 101 deletions

View File

@@ -157,6 +157,6 @@ async def _setup_oidc_provider(hass: HomeAssistant, my_config: dict, display_nam
_LOGGER.info("Registered OIDC views") _LOGGER.info("Registered OIDC views")
# Inject OIDC code into the frontend for /auth/authorize for automatic redirect # Inject OIDC code into the frontend for /auth/authorize for automatic redirect
await OIDCInjectedAuthPage.inject(hass) await OIDCInjectedAuthPage.inject(hass, force_https)
return True return True

View File

@@ -8,9 +8,7 @@ from typing import Any, Dict
DEFAULT_TITLE = "OpenID Connect (SSO)" DEFAULT_TITLE = "OpenID Connect (SSO)"
DOMAIN = "auth_oidc" DOMAIN = "auth_oidc"
REPO_ROOT_URL = ( REPO_ROOT_URL = "https://github.com/christiaangoossens/hass-oidc-auth/tree/v1.0.0-rc1"
"https://github.com/christiaangoossens/hass-oidc-auth/tree/v1.0.0-rc1"
)
## === ## ===
## Config keys ## Config keys

View File

@@ -1,12 +1,18 @@
"""Injected authorization page, replacing the original""" """Injected authorization page, replacing the original"""
import base64
import logging import logging
from functools import partial from functools import partial
from homeassistant.components.http import HomeAssistantView, StaticPathConfig from urllib.parse import quote, unquote
from homeassistant.core import HomeAssistant
from aiohttp import web from aiohttp import web
from aiofiles import open as async_open 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" PATH = "/auth/authorize"
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
@@ -18,7 +24,7 @@ async def read_file(path: str) -> str:
return await f.read() 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.""" """Inject new frontend code into /auth/authorize."""
router = hass.http.app.router router = hass.http.app.router
frontend_path = None frontend_path = None
@@ -61,7 +67,7 @@ async def frontend_injection(hass: HomeAssistant) -> None:
frontend_code = await read_file(frontend_path) frontend_code = await read_file(frontend_path)
# Inject JS and register that route # Inject JS and register that route
injection_js = "<script src='/auth/oidc/static/injection.js?v=4'></script>" injection_js = "<script src='/auth/oidc/static/injection.js?v=6'></script>"
frontend_code = frontend_code.replace("</body>", f"{injection_js}</body>") frontend_code = frontend_code.replace("</body>", f"{injection_js}</body>")
await hass.http.async_register_static_paths( 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 # 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") _LOGGER.info("Performed OIDC frontend injection")
@@ -91,18 +97,46 @@ class OIDCInjectedAuthPage(HomeAssistantView):
url = PATH url = PATH
name = "auth:oidc:authorize_page" 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.""" """Initialize the injected auth page."""
self.html = html self.html = html
self.force_https = force_https
@staticmethod @staticmethod
async def inject(hass: HomeAssistant) -> None: async def inject(hass: HomeAssistant, force_https: bool) -> None:
"""Inject the OIDC auth page into the frontend.""" """Inject the OIDC auth page into the frontend."""
try: try:
await frontend_injection(hass) await frontend_injection(hass, force_https)
except Exception as e: # pylint: disable=broad-except except Exception as e: # pylint: disable=broad-except
_LOGGER.error("Failed to inject OIDC auth page: %s", e) _LOGGER.error("Failed to inject OIDC auth page: %s", e)
async def get(self, _) -> web.Response: @staticmethod
"""Return the screen""" 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") return web.Response(text=self.html, content_type="text/html")

View File

@@ -16,8 +16,7 @@
"requirements": [ "requirements": [
"aiofiles", "aiofiles",
"jinja2", "jinja2",
"bcrypt",
"joserfc" "joserfc"
], ],
"version": "1.0.0-rc1" "version": "1.0.0-rc2"
} }

View File

@@ -6,7 +6,6 @@ import logging
from typing import Dict, Optional from typing import Dict, Optional
import asyncio import asyncio
import bcrypt
from homeassistant.auth import EVENT_USER_ADDED from homeassistant.auth import EVENT_USER_ADDED
from homeassistant.auth.providers import ( from homeassistant.auth.providers import (
AUTH_PROVIDERS, AUTH_PROVIDERS,
@@ -236,7 +235,7 @@ class OpenIDAuthProvider(AuthProvider):
# Keep cookie lifetime aligned with state lifetime in storage (5 minutes). # Keep cookie lifetime aligned with state lifetime in storage (5 minutes).
"set-cookie": f"{COOKIE_NAME}=" "set-cookie": f"{COOKIE_NAME}="
+ state_id + state_id
+ "; Path=/auth/; SameSite=Strict; HttpOnly; Max-Age=300" + "; Path=/auth/; SameSite=Lax; HttpOnly; Max-Age=300"
+ secure_flag, + secure_flag,
} }
@@ -367,14 +366,6 @@ class OpenIdLoginFlow(LoginFlow):
"""Handler for the login flow.""" """Handler for the login flow."""
async def _finalize_user(self, state_id: str) -> AuthFlowResult: 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) sub = await self._auth_provider.async_get_subject(state_id)
if sub: if sub:
return await self.async_finish( return await self.async_finish(
@@ -396,11 +387,10 @@ class OpenIdLoginFlow(LoginFlow):
state_cookie = req.cookies.get(COOKIE_NAME) state_cookie = req.cookies.get(COOKIE_NAME)
if state_cookie: if state_cookie:
_LOGGER.debug("State cookie found on login: %s", state_cookie)
try: try:
return await self._finalize_user(state_cookie) return await self._finalize_user(state_cookie)
except InvalidAuthError: except InvalidAuthError:
pass return self.async_abort(reason="oidc_cookie_invalid")
# If no cookie is found, abort. # If no cookie is found, abort.
# User should either be redirected or start manually on the welcome # User should either be redirected or start manually on the welcome

View File

@@ -1,58 +1,19 @@
/** /**
* OIDC Frontend Redirect injection script * hass-oidc-auth - UX script to automatically select the Home Assistant auth provider when the "Login aborted" message is shown.
* This script is injected because the 'hass-oidc-auth' custom component is active.
*/ */
function attempt_oidc_redirect() { let authFlowElement = null
// Get URL parameters
const urlParams = new URLSearchParams(window.location.search);
// Check if we have skip_oidc_redirect directly here function update() {
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 // Find ha-auth-flow
const authFlowElement = document.querySelector('ha-auth-flow'); authFlowElement = document.querySelector('ha-auth-flow');
if (!authFlowElement) { if (!authFlowElement) {
console.warn("[OIDC] ha-auth-flow element not found. Not automatically selecting HA provider.");
return; return;
} }
// Check if the text "Login aborted" is present on the page // Check if the text "Login aborted" is present on the page
if (!authFlowElement.innerText.includes('Login aborted')) { if (!authFlowElement.innerText.includes('Login aborted')) {
console.warn("[OIDC] 'Login aborted' text not found. Not automatically selecting HA provider.");
return; return;
} }
@@ -60,7 +21,6 @@ function click_alternative_provider_instead() {
const authProviderElement = document.querySelector('ha-pick-auth-provider'); const authProviderElement = document.querySelector('ha-pick-auth-provider');
if (!authProviderElement) { if (!authProviderElement) {
console.warn("[OIDC] ha-pick-auth-provider not found. Not automatically selecting HA provider.");
return; return;
} }
@@ -72,11 +32,30 @@ function click_alternative_provider_instead() {
} }
firstListItem.click(); firstListItem.click();
}, 500);
} }
// Run OIDC injection upon load // Hide the content until ready
(() => { let ready = false
attempt_oidc_redirect(); document.querySelector(".content").style.display = "none"
click_alternative_provider_instead();
})(); 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)

View File

@@ -9,7 +9,6 @@ license = "MIT"
dependencies = [ dependencies = [
"aiofiles~=25.1", "aiofiles~=25.1",
"jinja2~=3.1", "jinja2~=3.1",
"bcrypt~=5.0",
"joserfc~=1.6.0", "joserfc~=1.6.0",
] ]
readme = "README.md" readme = "README.md"

View File

@@ -93,7 +93,7 @@ async def test_provider_cookie_header_sets_secure_when_requested(hass: HomeAssis
provider = hass.auth.get_auth_providers(DOMAIN)[0] provider = hass.auth.get_auth_providers(DOMAIN)[0]
cookie_header = provider.get_cookie_header("state-id", secure=True)["set-cookie"] 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 "HttpOnly" in cookie_header
assert "Secure" 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({}) 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["type"] == FlowResultType.ABORT
assert result["reason"] == "no_oidc_cookie_found" assert result["reason"] == "no_oidc_cookie_found"

View File

@@ -2,6 +2,7 @@
import base64 import base64
import os import os
from urllib.parse import parse_qs, quote, unquote, urlparse
from unittest.mock import AsyncMock, MagicMock, patch from unittest.mock import AsyncMock, MagicMock, patch
from auth_oidc.config.const import DISCOVERY_URL, CLIENT_ID from auth_oidc.config.const import DISCOVERY_URL, CLIENT_ID
import pytest import pytest
@@ -45,6 +46,22 @@ async def setup(
assert result 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 @pytest.mark.asyncio
async def test_welcome_page_registration(hass: HomeAssistant, hass_client): async def test_welcome_page_registration(hass: HomeAssistant, hass_client):
"""Test that welcome page is present.""" """Test that welcome page is present."""
@@ -87,7 +104,7 @@ async def test_welcome_rejects_invalid_encoded_redirect_uri(
@pytest.mark.asyncio @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.""" """Welcome should set secure cookie flags for the OIDC state cookie."""
await setup(hass) 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", "") set_cookie = resp.headers.get("Set-Cookie", "")
assert "Path=/auth/" in 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 "HttpOnly" in set_cookie
assert "Max-Age=300" 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.""" """Test that frontend injection works."""
# Because there is no frontend in the test setup, # Because there is no frontend in the test setup,
# we'll have to fake /auth/authorize for the changes to register # we'll have to fake /auth/authorize for the changes to register.
await async_setup_component(hass, HTTP_DOMAIN, {}) await setup_mock_authorize_route(hass)
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,
)
]
)
await setup(hass) await setup(hass)
client = await hass_client() client = await hass_client()
resp = await client.get("/auth/authorize", allow_redirects=False) 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() text = await resp.text()
assert "<script src='/auth/oidc/static/injection.js" in text assert "<script src='/auth/oidc/static/injection.js" in text
@@ -606,7 +612,7 @@ async def test_frontend_injection_logs_and_returns_when_route_handler_is_unexpec
return iter([FakeRoute()]) return iter([FakeRoute()])
with patch.object(hass.http.app.router, "resources", return_value=[FakeResource()]): with patch.object(hass.http.app.router, "resources", return_value=[FakeResource()]):
await frontend_injection(hass) await frontend_injection(hass, force_https=False)
assert "Unexpected route handler type" in caplog.text assert "Unexpected route handler type" in caplog.text
assert ( assert (
@@ -625,6 +631,61 @@ async def test_injected_auth_page_inject_logs_errors(hass: HomeAssistant, caplog
"custom_components.auth_oidc.endpoints.injected_auth_page.frontend_injection", "custom_components.auth_oidc.endpoints.injected_auth_page.frontend_injection",
side_effect=RuntimeError("boom"), side_effect=RuntimeError("boom"),
): ):
await OIDCInjectedAuthPage.inject(hass) await OIDCInjectedAuthPage.inject(hass, force_https=False)
assert "Failed to inject OIDC auth page: boom" in caplog.text assert "Failed to inject OIDC auth page: boom" in caplog.text
@pytest.mark.asyncio
async def test_injected_auth_page_redirects_to_welcome_when_not_skipped(
hass: HomeAssistant, hass_client
):
"""Injected auth page should redirect into OIDC when skip flags are absent."""
await setup_mock_authorize_route(hass)
await setup(hass)
client = await hass_client()
encoded_redirect_uri = quote(create_redirect_uri(WEB_CLIENT_ID), safe="")
resp = await client.get(
f"/auth/authorize?redirect_uri={encoded_redirect_uri}",
allow_redirects=False,
)
assert resp.status == 302
location = resp.headers["Location"]
parsed_location = urlparse(location)
assert parsed_location.path == "/auth/oidc/welcome"
query = parse_qs(parsed_location.query)
assert "redirect_uri" in query
original_url = base64.b64decode(unquote(query["redirect_uri"][0]), validate=True)
original_url = original_url.decode("utf-8")
assert "/auth/authorize?redirect_uri=" in original_url
@pytest.mark.asyncio
@pytest.mark.parametrize(
"request_target",
[
"/auth/authorize?skip_oidc_redirect=true",
"/auth/authorize?redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fauthorize%3Fskip_oidc_redirect%3Dtrue",
],
)
async def test_injected_auth_page_returns_original_html_when_skipped(
hass: HomeAssistant,
hass_client,
request_target: str,
):
"""Injected auth page should render HTML when redirect suppression is requested."""
await setup_mock_authorize_route(hass)
await setup(hass)
client = await hass_client()
response = await client.get(request_target, allow_redirects=False)
assert response.status == 200
assert "<script src='/auth/oidc/static/injection.js" in await response.text()

2
uv.lock generated
View File

@@ -958,7 +958,6 @@ version = "1.0.0"
source = { editable = "." } source = { editable = "." }
dependencies = [ dependencies = [
{ name = "aiofiles" }, { name = "aiofiles" },
{ name = "bcrypt" },
{ name = "jinja2" }, { name = "jinja2" },
{ name = "joserfc" }, { name = "joserfc" },
] ]
@@ -977,7 +976,6 @@ dev = [
[package.metadata] [package.metadata]
requires-dist = [ requires-dist = [
{ name = "aiofiles", specifier = "~=25.1" }, { name = "aiofiles", specifier = "~=25.1" },
{ name = "bcrypt", specifier = "~=5.0" },
{ name = "jinja2", specifier = "~=3.1" }, { name = "jinja2", specifier = "~=3.1" },
{ name = "joserfc", specifier = "~=1.6.0" }, { name = "joserfc", specifier = "~=1.6.0" },
] ]