fix(housekeeping): require a caller on reads; writes unchanged (T-1)
api.schweitz.net is internet-facing with no forward-auth, and every
GET under /housekeeping/* (health, devices, areas, scenes, scripts,
automations, history) was unauthenticated. It answers 500 today only
because Home Assistant's token is dead; once that token is restored it
would serve the house's device list to anyone.
Reads now go through require_caller("office", "tatlock", "user").
tatlock is in the list because tatlock's Housekeeper
(tatlock/src/agents/housekeeper/client.py) is the only caller of these
routes today and already sends Bearer CORE_API_KEY when it is set.
Writes are untouched — control_device, activate_scene, run_script and
toggle_automation still depend on get_admin_user. Whether tatlock may
write is a separate decision, not this one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,9 +13,15 @@ from src.shared.base import BaseController
|
||||
from src.shared.clients import get_homeassistant_client
|
||||
from src.shared.logging import get_logger
|
||||
from src.domains.auth.oidc import get_admin_user
|
||||
from src.domains.auth.callers import Caller, require_caller
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
# api.schweitz.net carries no forward-auth, so these reads were otherwise
|
||||
# open to the internet (T-1 / D-68). Writes are untouched — see controller
|
||||
# comments at each `get_admin_user` dependency below.
|
||||
require_housekeeping_reader = require_caller("office", "tatlock", "user")
|
||||
|
||||
|
||||
# Pydantic Schemas
|
||||
class Device(BaseModel):
|
||||
@@ -188,7 +194,7 @@ class HousekeepingController(BaseController):
|
||||
response_model=HealthResponse,
|
||||
summary="Home automation health check"
|
||||
)
|
||||
async def get_health():
|
||||
async def get_health(caller: Caller = Depends(require_housekeeping_reader)):
|
||||
"""Check Home Assistant connection health"""
|
||||
ha = get_homeassistant_client()
|
||||
return await ha.health_check()
|
||||
@@ -200,7 +206,8 @@ class HousekeepingController(BaseController):
|
||||
)
|
||||
async def list_devices(
|
||||
domain: Optional[str] = Query(None, description="Filter by domain (light, switch, climate, etc.)"),
|
||||
area: Optional[str] = Query(None, description="Filter by area/room name")
|
||||
area: Optional[str] = Query(None, description="Filter by area/room name"),
|
||||
caller: Caller = Depends(require_housekeeping_reader),
|
||||
):
|
||||
"""List all available devices with optional filtering"""
|
||||
ha = get_homeassistant_client()
|
||||
@@ -256,7 +263,10 @@ class HousekeepingController(BaseController):
|
||||
response_model=DeviceDetailResponse,
|
||||
responses={404: {"model": ErrorResponse}}
|
||||
)
|
||||
async def get_device(entity_id: str):
|
||||
async def get_device(
|
||||
entity_id: str,
|
||||
caller: Caller = Depends(require_housekeeping_reader),
|
||||
):
|
||||
"""Get detailed state of a specific device"""
|
||||
ha = get_homeassistant_client()
|
||||
|
||||
@@ -296,7 +306,7 @@ class HousekeepingController(BaseController):
|
||||
response_model=AreaListResponse,
|
||||
summary="List areas/rooms"
|
||||
)
|
||||
async def list_areas():
|
||||
async def list_areas(caller: Caller = Depends(require_housekeeping_reader)):
|
||||
"""List all configured areas/rooms in Home Assistant"""
|
||||
ha = get_homeassistant_client()
|
||||
|
||||
@@ -389,7 +399,7 @@ class HousekeepingController(BaseController):
|
||||
response_model=SceneListResponse,
|
||||
summary="List available scenes"
|
||||
)
|
||||
async def list_scenes():
|
||||
async def list_scenes(caller: Caller = Depends(require_housekeeping_reader)):
|
||||
"""List all available scenes in Home Assistant"""
|
||||
ha = get_homeassistant_client()
|
||||
|
||||
@@ -459,7 +469,7 @@ class HousekeepingController(BaseController):
|
||||
response_model=ScriptListResponse,
|
||||
summary="List available scripts"
|
||||
)
|
||||
async def list_scripts():
|
||||
async def list_scripts(caller: Caller = Depends(require_housekeeping_reader)):
|
||||
"""List all available scripts/sequences in Home Assistant"""
|
||||
ha = get_homeassistant_client()
|
||||
|
||||
@@ -531,7 +541,7 @@ class HousekeepingController(BaseController):
|
||||
response_model=AutomationListResponse,
|
||||
summary="List automations"
|
||||
)
|
||||
async def list_automations():
|
||||
async def list_automations(caller: Caller = Depends(require_housekeeping_reader)):
|
||||
"""List all automations with their enabled/disabled status"""
|
||||
ha = get_homeassistant_client()
|
||||
|
||||
@@ -609,7 +619,8 @@ class HousekeepingController(BaseController):
|
||||
)
|
||||
async def get_history(
|
||||
entity_id: str = Query(..., description="Entity ID to get history for"),
|
||||
hours: int = Query(24, ge=1, le=168, description="Hours of history (1-168)")
|
||||
hours: int = Query(24, ge=1, le=168, description="Hours of history (1-168)"),
|
||||
caller: Caller = Depends(require_housekeeping_reader),
|
||||
):
|
||||
"""Get state history for a device"""
|
||||
ha = get_homeassistant_client()
|
||||
|
||||
@@ -3,6 +3,7 @@ import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
from unittest.mock import patch, AsyncMock
|
||||
|
||||
from src.domains.auth.oidc import oidc_config
|
||||
from src.main import app
|
||||
|
||||
|
||||
@@ -12,6 +13,51 @@ def client():
|
||||
return TestClient(app)
|
||||
|
||||
|
||||
class TestHousekeepingReadsAreLocked:
|
||||
"""
|
||||
T-1 / D-68: api.schweitz.net carries no forward-auth, so an
|
||||
unauthenticated read used to reach Home Assistant directly. These
|
||||
assert the caller gate in front of the GET routes, independent of
|
||||
whatever Home Assistant itself returns.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def oidc_enabled(self):
|
||||
original = (oidc_config.enabled, oidc_config.issuers, oidc_config.audiences)
|
||||
oidc_config.configure(enabled=True, issuers=[], audiences=[])
|
||||
yield
|
||||
oidc_config.enabled, oidc_config.issuers, oidc_config.audiences = original
|
||||
|
||||
def test_no_auth_is_401_on_devices(self, client):
|
||||
response = client.get("/housekeeping/devices")
|
||||
assert response.status_code == 401
|
||||
|
||||
def test_no_auth_is_401_on_health(self, client):
|
||||
response = client.get("/housekeeping/health")
|
||||
assert response.status_code == 401
|
||||
|
||||
@patch("src.domains.housekeeping.controller.get_homeassistant_client")
|
||||
def test_tatlock_key_passes_auth_on_devices(self, mock_get_ha, client):
|
||||
"""The one caller this lock exists to keep working (dossier §2)."""
|
||||
from src.shared.config import get_settings
|
||||
|
||||
settings = get_settings()
|
||||
original = settings.tatlock_service_key
|
||||
settings.tatlock_service_key = "test-tatlock-key"
|
||||
try:
|
||||
mock_client = AsyncMock()
|
||||
mock_client.get_states.return_value = []
|
||||
mock_get_ha.return_value = mock_client
|
||||
|
||||
response = client.get(
|
||||
"/housekeeping/devices",
|
||||
headers={"Authorization": "Bearer test-tatlock-key"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
finally:
|
||||
settings.tatlock_service_key = original
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_ha_client():
|
||||
"""Create a mock Home Assistant client."""
|
||||
|
||||
Reference in New Issue
Block a user