fix(security): require a configured session secret in production
session_secret defaulted to a random per-process value, which silently invalidates all sessions on restart and rotates the management client secret. Add _resolve_session_secret(): use the configured secret; allow a generated one only in debug or for a localhost issuer; otherwise fail startup. The management client secret is now tied to the resolved session secret. Refs: porchlight-wvx Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c175633980
commit
cf2754f302
3 changed files with 40 additions and 5 deletions
|
|
@ -82,8 +82,8 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
|
||||||
}
|
}
|
||||||
oidc_server.keyjar.add_symmetric(client_id, client_cfg.client_secret)
|
oidc_server.keyjar.add_symmetric(client_id, client_cfg.client_secret)
|
||||||
|
|
||||||
# Register management client
|
# Register management client (stable secret tied to the session secret)
|
||||||
manage_secret = settings.session_secret or secrets.token_hex(32)
|
manage_secret = app.state.session_secret
|
||||||
oidc_server.context.cdb[settings.manage_client_id] = {
|
oidc_server.context.cdb[settings.manage_client_id] = {
|
||||||
"client_id": settings.manage_client_id,
|
"client_id": settings.manage_client_id,
|
||||||
"client_secret": manage_secret,
|
"client_secret": manage_secret,
|
||||||
|
|
@ -101,6 +101,21 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
|
||||||
raise NotImplementedError("MongoDB backend not yet implemented")
|
raise NotImplementedError("MongoDB backend not yet implemented")
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_session_secret(settings: Settings) -> str:
|
||||||
|
"""Return the session signing secret, requiring one in production.
|
||||||
|
|
||||||
|
A random per-process secret silently invalidates sessions on restart and
|
||||||
|
rotates the management client secret, so it is only acceptable for local
|
||||||
|
development (debug or a localhost issuer).
|
||||||
|
"""
|
||||||
|
if settings.session_secret:
|
||||||
|
return settings.session_secret
|
||||||
|
host = urlparse(settings.issuer).hostname or ""
|
||||||
|
if settings.debug or host in ("localhost", "127.0.0.1", "::1"):
|
||||||
|
return secrets.token_hex(32)
|
||||||
|
raise RuntimeError("OIDC_OP_SESSION_SECRET must be set in production (non-debug, non-localhost issuer).")
|
||||||
|
|
||||||
|
|
||||||
def create_app(settings: Settings | None = None) -> FastAPI:
|
def create_app(settings: Settings | None = None) -> FastAPI:
|
||||||
if settings is None:
|
if settings is None:
|
||||||
settings = Settings()
|
settings = Settings()
|
||||||
|
|
@ -116,7 +131,8 @@ def create_app(settings: Settings | None = None) -> FastAPI:
|
||||||
app.state.settings = settings
|
app.state.settings = settings
|
||||||
|
|
||||||
# Session middleware
|
# Session middleware
|
||||||
session_secret = settings.session_secret or secrets.token_hex(32)
|
session_secret = _resolve_session_secret(settings)
|
||||||
|
app.state.session_secret = session_secret
|
||||||
app.add_middleware(
|
app.add_middleware(
|
||||||
CSRFMiddleware, # ty: ignore[invalid-argument-type]
|
CSRFMiddleware, # ty: ignore[invalid-argument-type]
|
||||||
exempt_paths={"/token", "/userinfo"},
|
exempt_paths={"/token", "/userinfo"},
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,10 @@
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
from httpx import AsyncClient
|
from httpx import AsyncClient
|
||||||
|
|
||||||
|
from porchlight.app import create_app
|
||||||
|
from porchlight.config import Settings
|
||||||
from porchlight.dependencies import (
|
from porchlight.dependencies import (
|
||||||
get_credential_repo,
|
get_credential_repo,
|
||||||
get_magic_link_repo,
|
get_magic_link_repo,
|
||||||
|
|
@ -56,3 +59,19 @@ async def test_dependency_functions() -> None:
|
||||||
assert get_user_repo(request) == "user_repo_sentinel"
|
assert get_user_repo(request) == "user_repo_sentinel"
|
||||||
assert get_credential_repo(request) == "credential_repo_sentinel"
|
assert get_credential_repo(request) == "credential_repo_sentinel"
|
||||||
assert get_magic_link_repo(request) == "magic_link_repo_sentinel"
|
assert get_magic_link_repo(request) == "magic_link_repo_sentinel"
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_app_requires_session_secret_in_production() -> None:
|
||||||
|
settings = Settings(issuer="https://op.example.com", sqlite_path=":memory:", debug=False)
|
||||||
|
with pytest.raises(RuntimeError, match="SESSION_SECRET"):
|
||||||
|
create_app(settings)
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_app_allows_missing_secret_on_localhost() -> None:
|
||||||
|
settings = Settings(issuer="http://localhost:8000", sqlite_path=":memory:")
|
||||||
|
assert create_app(settings) is not None
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_app_allows_missing_secret_in_debug() -> None:
|
||||||
|
settings = Settings(issuer="https://op.example.com", sqlite_path=":memory:", debug=True)
|
||||||
|
assert create_app(settings) is not None
|
||||||
|
|
|
||||||
|
|
@ -19,7 +19,7 @@ scope = ["openid", "profile"]
|
||||||
toml_file = tmp_path / "test.toml"
|
toml_file = tmp_path / "test.toml"
|
||||||
toml_file.write_text(toml_content)
|
toml_file.write_text(toml_content)
|
||||||
|
|
||||||
settings = Settings(_toml_file=str(toml_file))
|
settings = Settings(_toml_file=str(toml_file), session_secret="x" * 32)
|
||||||
app = create_app(settings)
|
app = create_app(settings)
|
||||||
|
|
||||||
async with (
|
async with (
|
||||||
|
|
@ -41,7 +41,7 @@ scope = ["openid", "profile"]
|
||||||
|
|
||||||
async def test_manage_app_always_registered() -> None:
|
async def test_manage_app_always_registered() -> None:
|
||||||
"""The internal manage-app client is always registered, even without config file clients."""
|
"""The internal manage-app client is always registered, even without config file clients."""
|
||||||
settings = Settings(issuer="https://test.example.com")
|
settings = Settings(issuer="https://test.example.com", session_secret="x" * 32)
|
||||||
app = create_app(settings)
|
app = create_app(settings)
|
||||||
|
|
||||||
async with (
|
async with (
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue