mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
feat(oidc): refuse API writes to the env-managed provider
Startup rewrites this row from BAMBUDDY_OIDC_* on every boot, so an edit through the UI would be accepted and then silently reverted at the next restart -- the operator would watch their change vanish with nothing explaining why. A 409 says so instead. Covers all four mutating routes, including the two icon ones: the icon comes from BAMBUDDY_OIDC_ICON_URL and would be restored the same way. Extracted as one helper rather than four copies of the same check, so a fifth route cannot be added with the guard silently missing. Locking it is safe because BAMBUDDY_LOCAL_LOGIN (#1589) remains the documented recovery path if the provider itself becomes unusable. A test pins that UI-created providers stay editable -- the lock must not leak onto them. Refs #2593
This commit is contained in:
@@ -1404,6 +1404,18 @@ async def create_oidc_provider(
|
||||
return _build_provider_response(provider)
|
||||
|
||||
|
||||
def _refuse_if_env_managed(provider: OIDCProvider) -> None:
|
||||
"""Startup rewrites this provider from BAMBUDDY_OIDC_* on every boot, so an
|
||||
edit here would be accepted and then silently reverted at the next restart.
|
||||
BAMBUDDY_LOCAL_LOGIN (#1589) remains the recovery path if it becomes
|
||||
unusable, so refusing outright cannot lock anyone out."""
|
||||
if provider.is_env_managed:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail="This OIDC provider is managed by environment variables and cannot be modified.",
|
||||
)
|
||||
|
||||
|
||||
@router.put("/oidc/providers/{provider_id}", response_model=OIDCProviderResponse)
|
||||
async def update_oidc_provider(
|
||||
provider_id: int,
|
||||
@@ -1426,6 +1438,7 @@ async def update_oidc_provider(
|
||||
provider = result2.scalar_one_or_none()
|
||||
if not provider:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found")
|
||||
_refuse_if_env_managed(provider)
|
||||
|
||||
if body.default_group_id is not None:
|
||||
grp_chk = await db.execute(select(Group).where(Group.id == body.default_group_id))
|
||||
@@ -1503,6 +1516,7 @@ async def delete_oidc_provider(
|
||||
provider = result2.scalar_one_or_none()
|
||||
if not provider:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found")
|
||||
_refuse_if_env_managed(provider)
|
||||
|
||||
await db.delete(provider)
|
||||
await db.commit()
|
||||
@@ -1571,6 +1585,7 @@ async def delete_oidc_provider_icon(
|
||||
provider = result.scalar_one_or_none()
|
||||
if provider is None:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found")
|
||||
_refuse_if_env_managed(provider)
|
||||
|
||||
# Setting deferred columns is safe — no read happens, just a write.
|
||||
provider.icon_url = None
|
||||
@@ -1603,6 +1618,7 @@ async def refresh_oidc_provider_icon(
|
||||
provider = result.scalar_one_or_none()
|
||||
if provider is None:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found")
|
||||
_refuse_if_env_managed(provider)
|
||||
if not provider.icon_url:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
"""The env-managed provider is read-only through the API (#2593).
|
||||
|
||||
Startup rewrites this row from BAMBUDDY_OIDC_* on every boot, so a UI edit
|
||||
would silently disappear at the next restart -- the operator would see their
|
||||
change accepted and then reverted, with nothing explaining why. Refusing the
|
||||
write is the honest answer.
|
||||
|
||||
Locking it out is safe because BAMBUDDY_LOCAL_LOGIN (#1589) is the documented
|
||||
recovery path if the provider itself becomes unusable.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
|
||||
from backend.app.models.oidc_provider import OIDCProvider
|
||||
from backend.tests.integration.test_mfa_api import _auth_header, _setup_and_login
|
||||
|
||||
|
||||
async def _env_managed_provider(db_session) -> int:
|
||||
provider = OIDCProvider(
|
||||
name="Env Keycloak",
|
||||
issuer_url="https://sso.example.com/realms/main",
|
||||
client_id="bambuddy",
|
||||
icon_url="https://sso.example.com/logo.png",
|
||||
is_env_managed=True,
|
||||
)
|
||||
provider.client_secret = "s3cr3t"
|
||||
db_session.add(provider)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(provider)
|
||||
return provider.id
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_put_is_refused(async_client: AsyncClient, db_session):
|
||||
provider_id = await _env_managed_provider(db_session)
|
||||
token = await _setup_and_login(async_client, "envlockput", "envlockput123")
|
||||
|
||||
response = await async_client.put(
|
||||
f"/api/v1/auth/oidc/providers/{provider_id}",
|
||||
json={"name": "hijacked"},
|
||||
headers=_auth_header(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_delete_is_refused(async_client: AsyncClient, db_session):
|
||||
provider_id = await _env_managed_provider(db_session)
|
||||
token = await _setup_and_login(async_client, "envlockdel", "envlockdel123")
|
||||
|
||||
response = await async_client.delete(
|
||||
f"/api/v1/auth/oidc/providers/{provider_id}",
|
||||
headers=_auth_header(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_icon_delete_is_refused(async_client: AsyncClient, db_session):
|
||||
"""The icon is part of the env config too -- BAMBUDDY_OIDC_ICON_URL."""
|
||||
provider_id = await _env_managed_provider(db_session)
|
||||
token = await _setup_and_login(async_client, "envlockicondel", "envlockicondel123")
|
||||
|
||||
response = await async_client.delete(
|
||||
f"/api/v1/auth/oidc/providers/{provider_id}/icon",
|
||||
headers=_auth_header(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_icon_refresh_is_refused(async_client: AsyncClient, db_session):
|
||||
provider_id = await _env_managed_provider(db_session)
|
||||
token = await _setup_and_login(async_client, "envlockiconref", "envlockiconref123")
|
||||
|
||||
response = await async_client.post(
|
||||
f"/api/v1/auth/oidc/providers/{provider_id}/icon/refresh",
|
||||
headers=_auth_header(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_a_ui_provider_is_still_editable(async_client: AsyncClient):
|
||||
"""The lock must not leak onto providers the operator created themselves --
|
||||
they coexist with the env one and stay fully editable."""
|
||||
token = await _setup_and_login(async_client, "envlockui", "envlockui123")
|
||||
created = await async_client.post(
|
||||
"/api/v1/auth/oidc/providers",
|
||||
json={
|
||||
"name": "UI provider",
|
||||
"issuer_url": "https://other.example.com",
|
||||
"client_id": "ui",
|
||||
"client_secret": "ui-secret",
|
||||
"scopes": "openid",
|
||||
"is_enabled": True,
|
||||
"auto_create_users": False,
|
||||
},
|
||||
headers=_auth_header(token),
|
||||
)
|
||||
provider_id = created.json()["id"]
|
||||
|
||||
response = await async_client.put(
|
||||
f"/api/v1/auth/oidc/providers/{provider_id}",
|
||||
json={"name": "Renamed"},
|
||||
headers=_auth_header(token),
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["name"] == "Renamed"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_the_response_says_which_provider_is_env_managed(async_client: AsyncClient, db_session):
|
||||
"""The frontend needs this to render the lock; without it the UI would show
|
||||
editable fields whose writes the API then refuses."""
|
||||
await _env_managed_provider(db_session)
|
||||
token = await _setup_and_login(async_client, "envlockflag", "envlockflag123")
|
||||
|
||||
response = await async_client.get("/api/v1/auth/oidc/providers/all", headers=_auth_header(token))
|
||||
|
||||
assert response.status_code == 200
|
||||
providers = response.json()
|
||||
assert any(p["is_env_managed"] for p in providers)
|
||||
Reference in New Issue
Block a user