Fixes for known bugs in v1.0.0-rc1 (#241)
* Fix #238 for same-site cookies * Redirect in Python + bump to rc2
This commit is contained in:
committed by
GitHub
parent
c7672f65d9
commit
baf3ac6b5a
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 = "<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>")
|
||||
|
||||
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")
|
||||
|
||||
@@ -16,8 +16,7 @@
|
||||
"requirements": [
|
||||
"aiofiles",
|
||||
"jinja2",
|
||||
"bcrypt",
|
||||
"joserfc"
|
||||
],
|
||||
"version": "1.0.0-rc1"
|
||||
"version": "1.0.0-rc2"
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
})();
|
||||
// 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)
|
||||
@@ -9,7 +9,6 @@ license = "MIT"
|
||||
dependencies = [
|
||||
"aiofiles~=25.1",
|
||||
"jinja2~=3.1",
|
||||
"bcrypt~=5.0",
|
||||
"joserfc~=1.6.0",
|
||||
]
|
||||
readme = "README.md"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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 "<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()])
|
||||
|
||||
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 (
|
||||
@@ -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",
|
||||
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
|
||||
|
||||
|
||||
@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
2
uv.lock
generated
@@ -958,7 +958,6 @@ version = "1.0.0"
|
||||
source = { editable = "." }
|
||||
dependencies = [
|
||||
{ name = "aiofiles" },
|
||||
{ name = "bcrypt" },
|
||||
{ name = "jinja2" },
|
||||
{ name = "joserfc" },
|
||||
]
|
||||
@@ -977,7 +976,6 @@ dev = [
|
||||
[package.metadata]
|
||||
requires-dist = [
|
||||
{ name = "aiofiles", specifier = "~=25.1" },
|
||||
{ name = "bcrypt", specifier = "~=5.0" },
|
||||
{ name = "jinja2", specifier = "~=3.1" },
|
||||
{ name = "joserfc", specifier = "~=1.6.0" },
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user