mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-21 10:15:47 +00:00
fix(auth): let deployments close local self-registration (#4311)
* fix(auth): let deployments close local self-registration The OIDC provisioning policy (allowed_email_domains, require_verified_email, auto_create_users) is enforced only in the SSO callback via get_or_provision_oidc_user. POST /api/v1/auth/register creates a local account without consulting any of it, and nothing can turn that path off, so a deployment declaring an email-domain allowlist can still be joined by any address through local registration. Add auth.local.allow_registration (default true, so existing deployments are unchanged) and gate /register on it before the account is created. Report the flag from /setup-status so the login page stops offering a signup entry the Gateway will reject. /initialize is deliberately not gated: it is the bootstrap path, guarded by admin_count == 0, and closing it would leave a fresh install unable to create its first admin. An unreadable config.yaml falls back to the pre-gate default (open) rather than making these two endpoints a hard dependency on the file. * docs(auth): align registration-gate fallback wording with the FileNotFoundError catch
This commit is contained in:
@@ -21,6 +21,7 @@ class AuthErrorCode(StrEnum):
|
||||
PROVIDER_NOT_FOUND = "provider_not_found"
|
||||
NOT_AUTHENTICATED = "not_authenticated"
|
||||
SYSTEM_ALREADY_INITIALIZED = "system_already_initialized"
|
||||
REGISTRATION_DISABLED = "registration_disabled"
|
||||
|
||||
|
||||
class TokenError(StrEnum):
|
||||
|
||||
@@ -315,13 +315,46 @@ async def login_local(
|
||||
)
|
||||
|
||||
|
||||
def _local_registration_enabled() -> bool:
|
||||
"""Whether visitors may self-register a local account.
|
||||
|
||||
Local registration bypasses the OIDC provisioning policy entirely
|
||||
(allowed_email_domains, require_verified_email, auto_create_users are only
|
||||
enforced in the SSO callback), so SSO-provisioned deployments need a way to
|
||||
close this path.
|
||||
|
||||
``config.yaml`` is absent in bare-app contexts that never load it (tests build the
|
||||
gateway without one). Registration was unconditionally open before this gate existed,
|
||||
so an absent config file falls back to that same default rather than turning these two
|
||||
endpoints into a hard dependency on the file. Only ``FileNotFoundError`` is caught:
|
||||
a malformed config must not silently re-open a closed deployment, so it propagates.
|
||||
|
||||
``/register`` reads this fresh on every request (``get_app_config`` reloads on file
|
||||
change); ``/setup-status`` may serve it up to 60s stale via its per-IP result cache.
|
||||
"""
|
||||
from deerflow.config.app_config import get_app_config
|
||||
|
||||
try:
|
||||
return get_app_config().auth.local.allow_registration
|
||||
except FileNotFoundError:
|
||||
return True
|
||||
|
||||
|
||||
@router.post("/register", response_model=UserResponse, status_code=status.HTTP_201_CREATED)
|
||||
async def register(request: Request, response: Response, body: RegisterRequest):
|
||||
"""Register a new user account (always 'user' role).
|
||||
|
||||
The first admin is created explicitly through /initialize. This endpoint creates regular users.
|
||||
Auto-login by setting the session cookie.
|
||||
|
||||
Returns 403 when ``auth.local.allow_registration`` is false.
|
||||
"""
|
||||
if not _local_registration_enabled():
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail=AuthErrorResponse(code=AuthErrorCode.REGISTRATION_DISABLED, message="Self-registration is disabled on this deployment").model_dump(),
|
||||
)
|
||||
|
||||
try:
|
||||
user = await get_local_provider().create_user(email=body.email, password=body.password, system_role="user")
|
||||
except ValueError:
|
||||
@@ -465,7 +498,7 @@ async def setup_status(request: Request):
|
||||
|
||||
async def _compute_setup_status() -> dict:
|
||||
admin_count = await get_local_provider().count_admin_users()
|
||||
return {"needs_setup": admin_count == 0}
|
||||
return {"needs_setup": admin_count == 0, "registration_enabled": _local_registration_enabled()}
|
||||
|
||||
task = asyncio.create_task(_compute_setup_status())
|
||||
_SETUP_STATUS_INFLIGHT[client_ip] = task
|
||||
|
||||
@@ -67,7 +67,22 @@ class OIDCAuthConfig(BaseModel):
|
||||
)
|
||||
|
||||
|
||||
class LocalAuthConfig(BaseModel):
|
||||
"""Configuration for the built-in email/password authentication provider."""
|
||||
|
||||
allow_registration: bool = Field(
|
||||
default=True,
|
||||
description=(
|
||||
"Allow visitors to self-register a local account via POST /api/v1/auth/register. "
|
||||
"Set to false when accounts are provisioned exclusively through SSO — the OIDC "
|
||||
"provisioning policy (allowed_email_domains, require_verified_email, auto_create_users) "
|
||||
"does not apply to local registration."
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
class AuthAppConfig(BaseModel):
|
||||
"""Authentication configuration section for the DeerFlow app config."""
|
||||
|
||||
oidc: OIDCAuthConfig = Field(default_factory=OIDCAuthConfig, description="OIDC SSO authentication settings")
|
||||
local: LocalAuthConfig = Field(default_factory=LocalAuthConfig, description="Built-in email/password authentication settings")
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
"""Local self-registration gate (``auth.local.allow_registration``).
|
||||
|
||||
The OIDC provisioning policy (``allowed_email_domains``, ``require_verified_email``,
|
||||
``auto_create_users``) is enforced only in the SSO callback via
|
||||
``get_or_provision_oidc_user``. ``POST /api/v1/auth/register`` creates a local account
|
||||
without consulting any of it, so an SSO-provisioned deployment that declares a domain
|
||||
allowlist can still be joined by any address through the local path. These tests pin the
|
||||
gate that lets such a deployment close it.
|
||||
"""
|
||||
|
||||
import secrets
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.gateway.auth.config import AuthConfig, set_auth_config
|
||||
from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig
|
||||
|
||||
_TEST_SECRET = "test-secret-key-for-registration-gate-tests-only"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _persistence_engine(tmp_path):
|
||||
"""Per-test SQLite engine + clean ``deps._cached_*`` (mirrors test_auth_type_system).
|
||||
|
||||
These tests drive the real ``/register`` handler, which reaches
|
||||
``SQLiteUserRepository`` → ``get_session_factory``. A fresh DB per test also means
|
||||
the admin slot is empty for the /initialize case.
|
||||
"""
|
||||
import asyncio
|
||||
|
||||
from app.gateway import deps
|
||||
from deerflow.persistence.engine import close_engine, init_engine
|
||||
|
||||
asyncio.run(init_engine("sqlite", url=f"sqlite+aiosqlite:///{tmp_path}/registration_gate.db", sqlite_dir=str(tmp_path)))
|
||||
deps._cached_local_provider = None
|
||||
deps._cached_repo = None
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
deps._cached_local_provider = None
|
||||
deps._cached_repo = None
|
||||
asyncio.run(close_engine())
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client(monkeypatch):
|
||||
"""TestClient whose gate reads a config we control.
|
||||
|
||||
Overrides only ``auth.local`` on a deep copy of the real config: the app lifespan
|
||||
reads other sections (``database`` above all) to initialise the engine, and a
|
||||
narrower stub would abort startup and leak an uninitialised engine into the
|
||||
globals ``app.gateway.deps`` caches for the rest of the session.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
from deerflow.config.app_config import AppConfig
|
||||
|
||||
# config.yaml is gitignored, so tests cannot rely on it existing. The committed
|
||||
# example is a valid config and gives every other section (database above all)
|
||||
# a real value for the app lifespan.
|
||||
baseline = AppConfig.from_file(str(Path(__file__).resolve().parents[2] / "config.example.yaml"))
|
||||
|
||||
def _make(*, allow_registration: bool) -> TestClient:
|
||||
set_auth_config(AuthConfig(jwt_secret=_TEST_SECRET))
|
||||
patched = baseline.model_copy(deep=True)
|
||||
patched.auth = AuthAppConfig(local=LocalAuthConfig(allow_registration=allow_registration), oidc=baseline.auth.oidc)
|
||||
monkeypatch.setattr("deerflow.config.app_config.get_app_config", lambda: patched)
|
||||
# setup-status memoizes per client IP; drop it so each direction is read fresh.
|
||||
from app.gateway.routers import auth as auth_router
|
||||
|
||||
auth_router._SETUP_STATUS_CACHE.clear()
|
||||
|
||||
from app.gateway.app import create_app
|
||||
|
||||
return TestClient(create_app())
|
||||
|
||||
return _make
|
||||
|
||||
|
||||
def _unique_email(prefix: str) -> str:
|
||||
return f"{prefix}-{secrets.token_hex(4)}@test.com"
|
||||
|
||||
|
||||
def test_register_succeeds_when_registration_allowed(client):
|
||||
"""Default (allow_registration=True) keeps self-registration working."""
|
||||
resp = client(allow_registration=True).post(
|
||||
"/api/v1/auth/register",
|
||||
json={"email": _unique_email("gate-allowed"), "password": "Tr0ub4dor3a!"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
|
||||
|
||||
def test_register_rejected_when_registration_disabled(client):
|
||||
"""allow_registration=False closes the local self-registration path."""
|
||||
resp = client(allow_registration=False).post(
|
||||
"/api/v1/auth/register",
|
||||
json={"email": _unique_email("gate-denied"), "password": "Tr0ub4dor3a!"},
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()["detail"]["code"] == "registration_disabled"
|
||||
|
||||
|
||||
def test_register_rejected_before_the_account_is_created(client):
|
||||
"""The gate runs ahead of user creation — a denied email stays unregistered.
|
||||
|
||||
Guards against a gate placed after ``create_user``, which would 403 the caller
|
||||
while still persisting the account.
|
||||
"""
|
||||
email = _unique_email("gate-not-persisted")
|
||||
denied = client(allow_registration=False)
|
||||
assert denied.post("/api/v1/auth/register", json={"email": email, "password": "Tr0ub4dor3a!"}).status_code == 403
|
||||
|
||||
# Re-open registration: the same address must still be free, i.e. the denied
|
||||
# attempt created nothing. A leaked account would surface as 400 email_already_exists.
|
||||
resp = client(allow_registration=True).post(
|
||||
"/api/v1/auth/register",
|
||||
json={"email": email, "password": "Tr0ub4dor3a!"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
|
||||
|
||||
def test_registration_defaults_to_allowed():
|
||||
"""The default must stay True — this change is meant to add an opt-in, not flip behaviour.
|
||||
|
||||
Flipping the default would silently close self-registration on every existing
|
||||
deployment that never sets the key.
|
||||
"""
|
||||
assert LocalAuthConfig().allow_registration is True
|
||||
assert AuthAppConfig().local.allow_registration is True
|
||||
|
||||
|
||||
def test_gate_falls_back_to_open_when_config_is_absent(monkeypatch):
|
||||
"""No config.yaml → the pre-gate default (open), not a 500.
|
||||
|
||||
Bare-app contexts never load a config file. Before this gate, /register and
|
||||
/setup-status did not read the app config at all; the fallback keeps that true.
|
||||
"""
|
||||
from app.gateway.routers import auth as auth_router
|
||||
|
||||
def _raise() -> None:
|
||||
raise FileNotFoundError("`config.yaml` file not found in the project root")
|
||||
|
||||
monkeypatch.setattr("deerflow.config.app_config.get_app_config", _raise)
|
||||
assert auth_router._local_registration_enabled() is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize("allowed", [True, False])
|
||||
def test_setup_status_reports_registration_state(client, allowed):
|
||||
"""The login page needs the flag to decide whether to offer a signup entry."""
|
||||
resp = client(allow_registration=allowed).get("/api/v1/auth/setup-status")
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["registration_enabled"] is allowed
|
||||
|
||||
|
||||
def test_initialize_is_not_gated_by_the_registration_flag(client):
|
||||
"""Closing self-registration must never block first-boot admin creation.
|
||||
|
||||
``config.example.yaml`` promises that turning the flag off cannot lock an operator
|
||||
out of a fresh install; the per-test engine gives us a genuinely empty admin slot.
|
||||
"""
|
||||
resp = client(allow_registration=False).post(
|
||||
"/api/v1/auth/initialize",
|
||||
json={"email": _unique_email("gate-initialize"), "password": "Tr0ub4dor3a!"},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
+18
-1
@@ -15,7 +15,7 @@
|
||||
# ============================================================================
|
||||
# Bump this number when the config schema changes.
|
||||
# Run `make config-upgrade` to merge new fields into your local config.yaml.
|
||||
config_version: 28
|
||||
config_version: 29
|
||||
|
||||
# ============================================================================
|
||||
# Logging
|
||||
@@ -2003,7 +2003,24 @@ authorization:
|
||||
# 2. Add Valid Redirect URI: http://localhost:8001/api/v1/auth/callback/keycloak
|
||||
# 3. Add Web Origin: http://localhost:8001 (or your frontend origin)
|
||||
|
||||
# ============================================================================
|
||||
# Local (email/password) authentication
|
||||
# ============================================================================
|
||||
# Self-registration via POST /api/v1/auth/register is open by default: anyone
|
||||
# who can reach the Gateway may create a regular user account.
|
||||
#
|
||||
# The OIDC provisioning policy below (allowed_email_domains, require_verified_email,
|
||||
# auto_create_users) is enforced only in the SSO callback. It does NOT constrain local
|
||||
# registration, so on an SSO-provisioned deployment leaving this open lets a visitor
|
||||
# create an account outside that policy. Set allow_registration: false there.
|
||||
#
|
||||
# The first admin account is always created through /initialize regardless of this
|
||||
# setting, so turning it off cannot lock you out of a fresh install.
|
||||
|
||||
# auth:
|
||||
# local:
|
||||
# allow_registration: false
|
||||
#
|
||||
# oidc:
|
||||
# enabled: true
|
||||
# # Base URL of the frontend, used for redirects after SSO callback.
|
||||
|
||||
@@ -124,7 +124,7 @@ they resolve from the `secrets` map):
|
||||
|
||||
```yaml
|
||||
config: |
|
||||
config_version: 28
|
||||
config_version: 29
|
||||
models:
|
||||
- name: gpt-4
|
||||
use: langchain_openai:ChatOpenAI
|
||||
|
||||
@@ -240,7 +240,7 @@ ingress:
|
||||
# -- DeerFlow config.yaml content. Secrets MUST stay as $VAR references — never
|
||||
# inline literal secret values here. The default enables provisioner sandbox.
|
||||
config: |
|
||||
config_version: 28
|
||||
config_version: 29
|
||||
log_level: info
|
||||
|
||||
models: []
|
||||
|
||||
@@ -2,6 +2,7 @@ import { parseAuthError } from "./types";
|
||||
|
||||
export type SetupStatusResponse = {
|
||||
needs_setup?: boolean;
|
||||
registration_enabled?: boolean;
|
||||
};
|
||||
|
||||
export type SetupStatusCheck = {
|
||||
@@ -30,5 +31,11 @@ export function isSystemAlreadyInitializedError(data: unknown): boolean {
|
||||
}
|
||||
|
||||
export function canCreateRegularAccount(check: SetupStatusCheck): boolean {
|
||||
return check.checked && check.status?.needs_setup !== true;
|
||||
// registration_enabled is absent on older Gateways; treat that as allowed so
|
||||
// the signup entry only disappears when the backend actively closes it.
|
||||
return (
|
||||
check.checked &&
|
||||
check.status?.needs_setup !== true &&
|
||||
check.status?.registration_enabled !== false
|
||||
);
|
||||
}
|
||||
|
||||
@@ -56,6 +56,28 @@ describe("auth setup helpers", () => {
|
||||
expect(canCreateRegularAccount({ checked: true, status: null })).toBe(true);
|
||||
});
|
||||
|
||||
test("regular sign-up follows the gateway's registration_enabled flag", () => {
|
||||
expect(
|
||||
canCreateRegularAccount({
|
||||
checked: true,
|
||||
status: { needs_setup: false, registration_enabled: false },
|
||||
}),
|
||||
).toBe(false);
|
||||
expect(
|
||||
canCreateRegularAccount({
|
||||
checked: true,
|
||||
status: { needs_setup: false, registration_enabled: true },
|
||||
}),
|
||||
).toBe(true);
|
||||
// Older Gateways omit the field; absent must not hide the signup entry.
|
||||
expect(
|
||||
canCreateRegularAccount({
|
||||
checked: true,
|
||||
status: { needs_setup: false },
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test("detects already-initialized setup conflicts", () => {
|
||||
expect(
|
||||
isSystemAlreadyInitializedError({
|
||||
|
||||
Reference in New Issue
Block a user