From d8cba6e4ebd50e919aba90293a5effdca5282109 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:07:16 +0100 Subject: [PATCH 1/9] refactor(exceptions): take the error bases from gateway.exceptions The five status-carrying bases are defined in `exceptions/_base.py` and reach callers through the `gateway.exceptions` package root. `services/tenancy/errors.py` re-exported them, so eleven modules named the tenancy module for a class it does not define. Splitting that module by domain (#1193) leaves no module there to name, and the package root also avoids running `services/tenancy/__init__.py`, which pulls in the tenancy service tree for a base class. No error class moves and no status changes. --- src/gateway/api/routes/auth_oauth.py | 3 ++- src/gateway/api/routes/auth_webauthn.py | 3 ++- src/gateway/main.py | 2 +- src/gateway/services/budgets/_organization_surface.py | 2 +- src/gateway/services/organization_pricing_service.py | 2 +- src/gateway/services/playground_service.py | 7 ++----- src/gateway/services/tenancy/organization_model_access.py | 2 +- src/gateway/services/tenancy/provisioning_service.py | 7 ++----- tests/integration/test_organization_budgets.py | 3 ++- tests/integration/test_organization_pricing_routes.py | 2 +- tests/unit/test_organization_pricing_resolution.py | 6 ++---- 11 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/gateway/api/routes/auth_oauth.py b/src/gateway/api/routes/auth_oauth.py index 8b95eb671c..136af8aca9 100644 --- a/src/gateway/api/routes/auth_oauth.py +++ b/src/gateway/api/routes/auth_oauth.py @@ -54,6 +54,7 @@ # wording it differently would tell a person the doors closed for three reasons. from gateway.api.routes.auth_session import MAINTENANCE_MODE_REFUSAL from gateway.core.config import OAUTH_PROVIDERS, GatewayConfig +from gateway.exceptions import TenancyError from gateway.log_config import logger from gateway.services.dashboard_session_service import ( apply_session_cookie, @@ -71,7 +72,7 @@ provider_label, require_configured, ) -from gateway.services.tenancy.errors import OAuthNotConfiguredError, TenancyError +from gateway.services.tenancy.errors import OAuthNotConfiguredError from gateway.services.tenancy.organization_domain_service import OrganizationDomainService router = APIRouter(prefix=OAUTH_ROUTE_PREFIX, tags=["auth"]) diff --git a/src/gateway/api/routes/auth_webauthn.py b/src/gateway/api/routes/auth_webauthn.py index e03152a22b..aa3279bb22 100644 --- a/src/gateway/api/routes/auth_webauthn.py +++ b/src/gateway/api/routes/auth_webauthn.py @@ -44,6 +44,7 @@ # differently would tell a person the two doors closed for different reasons. from gateway.api.routes.auth_session import MAINTENANCE_MODE_REFUSAL from gateway.core.config import GatewayConfig +from gateway.exceptions import TenancyError from gateway.log_config import logger from gateway.models.tenancy import ( MAX_WEBAUTHN_CREDENTIAL_NAME, @@ -58,7 +59,7 @@ ) from gateway.services.maintenance_mode_service import is_maintenance_mode from gateway.services.tenancy import webauthn_service -from gateway.services.tenancy.errors import PasskeysNotConfiguredError, TenancyError +from gateway.services.tenancy.errors import PasskeysNotConfiguredError from gateway.services.tenancy.organization_domain_service import OrganizationDomainService router = APIRouter(prefix="/auth/webauthn", tags=["auth"]) diff --git a/src/gateway/main.py b/src/gateway/main.py index 9ebe113694..e77b868750 100644 --- a/src/gateway/main.py +++ b/src/gateway/main.py @@ -22,6 +22,7 @@ from gateway.core.database import create_session, dispose_db, init_db from gateway.core.feature import Worker from gateway.dashboard import DASHBOARD_PACKAGE_PATH, get_dashboard_build_id, get_dashboard_dir +from gateway.exceptions import TenancyError from gateway.inflight import InFlightMiddleware, InFlightRegistry from gateway.log_config import logger from gateway.ports.api_key_format_port import ApiKeyFormatPort @@ -80,7 +81,6 @@ ) from gateway.services.secret_box import validate_secret_key from gateway.services.selector_index_service import run_selector_index_refresher -from gateway.services.tenancy.errors import TenancyError from gateway.services.tenancy.org_provider_key_service import ( load_org_provider_keys_at_startup, reset_org_provider_cache, diff --git a/src/gateway/services/budgets/_organization_surface.py b/src/gateway/services/budgets/_organization_surface.py index f8ee1ee037..d4d7b67b03 100644 --- a/src/gateway/services/budgets/_organization_surface.py +++ b/src/gateway/services/budgets/_organization_surface.py @@ -14,6 +14,7 @@ from datetime import UTC, datetime from typing import Any +from gateway.exceptions import TenancyValidationError from gateway.exceptions.budget_exceptions import ( BudgetStillReferencedError, OrganizationBudgetHeldElsewhereError, @@ -41,7 +42,6 @@ from gateway.services.budgets._periods import period_window from gateway.services.budgets._retiming import cadence_of from gateway.services.budgets._scopes import ScopeOwnership, lock_workspace_for_scope -from gateway.services.tenancy.errors import TenancyValidationError from gateway.services.tenancy.organization_service import OrganizationService _MAX_LIST_LIMIT = 1000 diff --git a/src/gateway/services/organization_pricing_service.py b/src/gateway/services/organization_pricing_service.py index 35b02abc43..108f333f0c 100644 --- a/src/gateway/services/organization_pricing_service.py +++ b/src/gateway/services/organization_pricing_service.py @@ -46,6 +46,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig +from gateway.exceptions import TenancyValidationError from gateway.models.money import to_usd, to_usd_or_none from gateway.models.pricing import API_ORIGIN, ModelPricing, OrganizationModelPricing, PriceSource from gateway.models.tenancy import User as TenancyUser @@ -62,7 +63,6 @@ OrganizationPricingManagedModelError, OrganizationPricingNotFoundError, OrganizationPricingOverlapError, - TenancyValidationError, ) from gateway.services.tenancy.org_provider_key_service import OrgProviderKeyService from gateway.services.tenancy.organization_service import OrganizationService diff --git a/src/gateway/services/playground_service.py b/src/gateway/services/playground_service.py index 362affaf8c..f174a6b15c 100644 --- a/src/gateway/services/playground_service.py +++ b/src/gateway/services/playground_service.py @@ -25,6 +25,7 @@ from sqlmodel import col from gateway.core.config import GatewayConfig +from gateway.exceptions import TenancyConflictError, TenancyForbiddenError from gateway.models.playground import ( MAX_FAVORITE_MODELS, MAX_SAVED_COMPARISONS, @@ -48,11 +49,7 @@ from gateway.repositories.users_repository import get_or_create_attribution_user from gateway.services.tenancy import OrganizationService from gateway.services.tenancy.authorization import resolve_workspace_in_organization -from gateway.services.tenancy.errors import ( - TenancyConflictError, - TenancyForbiddenError, - WorkspaceNotFoundError, -) +from gateway.services.tenancy.errors import WorkspaceNotFoundError from gateway.services.workspace_scope import organization_default_workspace_id from gateway.types.session_principal import SessionPrincipal diff --git a/src/gateway/services/tenancy/organization_model_access.py b/src/gateway/services/tenancy/organization_model_access.py index 686ce51ebd..ac898d821f 100644 --- a/src/gateway/services/tenancy/organization_model_access.py +++ b/src/gateway/services/tenancy/organization_model_access.py @@ -38,6 +38,7 @@ from sqlmodel import col from gateway.core.config import GatewayConfig +from gateway.exceptions import TenancyForbiddenError, TenancyNotFoundError from gateway.models.provider_keys import ( OrgProviderKey, ) @@ -50,7 +51,6 @@ ) from gateway.services.provider_kwargs import provider_key from gateway.services.tenancy.authorization import VisibleWorkspaceScope, resolve_visible_workspace_scope -from gateway.services.tenancy.errors import TenancyForbiddenError, TenancyNotFoundError from gateway.services.tenancy.org_provider_key_service import OrgProviderKeyService, has_credential, key_is_usable from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.workspace_scope import lookup_default_workspace_id, organization_for_workspace_id diff --git a/src/gateway/services/tenancy/provisioning_service.py b/src/gateway/services/tenancy/provisioning_service.py index 820c016bbc..fe21b8a1a4 100644 --- a/src/gateway/services/tenancy/provisioning_service.py +++ b/src/gateway/services/tenancy/provisioning_service.py @@ -30,6 +30,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from sqlmodel import col +from gateway.exceptions import TenancyError, TenancyNotFoundError from gateway.log_config import logger from gateway.models.platform import RuntimeSetting from gateway.models.tenancy import Organization, User @@ -41,11 +42,7 @@ WorkspaceRepository, ) from gateway.repositories.users_repository import get_or_create_attribution_user -from gateway.services.tenancy.errors import ( - ForeignTenancyError, - TenancyError, - TenancyNotFoundError, -) +from gateway.services.tenancy.errors import ForeignTenancyError from gateway.services.tenancy.membership_listener import MembershipListener # Stored in runtime_settings, and deliberately not a SETTABLE_KEY, so diff --git a/tests/integration/test_organization_budgets.py b/tests/integration/test_organization_budgets.py index fc86075982..9fed0f8a6d 100644 --- a/tests/integration/test_organization_budgets.py +++ b/tests/integration/test_organization_budgets.py @@ -30,6 +30,7 @@ from gateway.core.config import API_ROOT from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions import TenancyValidationError from gateway.exceptions.budget_exceptions import ( OrganizationBudgetHeldElsewhereError, OrganizationBudgetInUseError, @@ -59,7 +60,7 @@ ) from gateway.services.api_keys import ApiKeyService from gateway.services.budgets import BudgetService -from gateway.services.tenancy.errors import NotAuthorizedError, TenancyValidationError +from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.organization_service import OrganizationService _BUDGETS = f"{API_ROOT}/organizations/me/budgets" diff --git a/tests/integration/test_organization_pricing_routes.py b/tests/integration/test_organization_pricing_routes.py index 4f293f70e8..a9c9be192b 100644 --- a/tests/integration/test_organization_pricing_routes.py +++ b/tests/integration/test_organization_pricing_routes.py @@ -23,6 +23,7 @@ from sqlmodel import col from gateway.core.config import API_ROOT, GatewayConfig +from gateway.exceptions import TenancyValidationError from gateway.models.api_keys import APIKey from gateway.models.pricing import ModelPricing, OrganizationModelPricing from gateway.models.tenancy import DashboardSession, Organization, OrganizationMember, User, Workspace @@ -48,7 +49,6 @@ OrganizationPricingManagedModelError, OrganizationPricingNotFoundError, OrganizationPricingOverlapError, - TenancyValidationError, ) from gateway.services.tenancy.org_provider_key_service import refresh_org_provider_cache, reset_org_provider_cache from gateway.services.workspace_scope import ( diff --git a/tests/unit/test_organization_pricing_resolution.py b/tests/unit/test_organization_pricing_resolution.py index c924b5a7a0..c7fbb524ae 100644 --- a/tests/unit/test_organization_pricing_resolution.py +++ b/tests/unit/test_organization_pricing_resolution.py @@ -24,6 +24,7 @@ import gateway.models # noqa: F401 (registers every table on the shared metadata) from gateway.core.config import GatewayConfig +from gateway.exceptions import TenancyValidationError from gateway.models.pricing import ModelPricing, OrganizationModelPricing from gateway.models.tenancy import Organization from gateway.services.external_usage_service import _load_pricing_index, _resolve_pricing @@ -32,10 +33,7 @@ validate_period, ) from gateway.services.pricing_service import find_model_pricing, price_tool_calls -from gateway.services.tenancy.errors import ( - OrganizationPricingOverlapError, - TenancyValidationError, -) +from gateway.services.tenancy.errors import OrganizationPricingOverlapError T = TypeVar("T") From ce4373d861afea82401127d7dd61146bef9e50fe Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:08:55 +0100 Subject: [PATCH 2/9] refactor(exceptions): move the secret box error into shared_exceptions.py `SecretBoxUnavailableTenancyError` reports an absent `OTARI_SECRET_KEY`, which is a deployment's condition rather than one domain's: provider keys, workspace MCP servers and both guardrail surfaces raise it. Splitting the tenancy errors by domain (#1193) leaves it no domain module, and putting it in one would make the other three import that domain for it. Pure move: the class, its status and its message are unchanged. Its docstring no longer points at the old module's docstring for why the class carries a status. --- src/gateway/exceptions/shared_exceptions.py | 32 +++++++++++++++++++ src/gateway/services/tenancy/errors.py | 21 ------------ .../tenancy/org_provider_key_service.py | 2 +- ...ganization_guardrail_definition_service.py | 2 +- .../tenancy/organization_guardrail_service.py | 2 +- .../tenancy/workspace_mcp_server_service.py | 2 +- 6 files changed, 36 insertions(+), 25 deletions(-) create mode 100644 src/gateway/exceptions/shared_exceptions.py diff --git a/src/gateway/exceptions/shared_exceptions.py b/src/gateway/exceptions/shared_exceptions.py new file mode 100644 index 0000000000..6ac95f962e --- /dev/null +++ b/src/gateway/exceptions/shared_exceptions.py @@ -0,0 +1,32 @@ +"""Errors that several domains raise, each carrying the status it renders as. + +A condition that belongs to the deployment rather than to one domain has no +domain module to sit in, and lives here. +""" + +from gateway.exceptions import TenancyError + + +class SecretBoxUnavailableTenancyError(TenancyError): + """`OTARI_SECRET_KEY` is not configured, so a secret cannot be stored. + + Wraps `services.secret_box.SecretBoxUnavailableError` in the status-carrying + family so the route stays thin: the underlying error carries no key + material, and neither does this one. A 500, not the 400 a + `TenancyValidationError` would carry: the caller sent a well-formed + request, and a missing secret key is a deployment configuration gap the + caller cannot fix. Blaming the client here would also keep the condition + out of 5xx error-rate alerting, which is exactly the audience that can. + + ``stored`` names what could not be stored, so the message points at the + surface the caller was using. It defaults to the provider credentials this + error was written for; workspace MCP servers pass their own. + """ + + def __init__(self, stored: str = "provider credentials") -> None: + super().__init__(f"OTARI_SECRET_KEY is not set; it is required to store {stored}") + + +__all__ = [ + "SecretBoxUnavailableTenancyError", +] diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/services/tenancy/errors.py index b7001074c5..8373a06d74 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/services/tenancy/errors.py @@ -724,26 +724,6 @@ def __init__(self) -> None: super().__init__("A provider key override cannot be both pinned as default and disabled") -class SecretBoxUnavailableTenancyError(TenancyError): - """`OTARI_SECRET_KEY` is not configured, so a secret cannot be stored. - - Wraps `services.secret_box.SecretBoxUnavailableError` as a tenancy error so - the route stays thin (see the module docstring): the underlying error - carries no key material, and neither does this one. A 500, not the 400 a - `TenancyValidationError` would carry: the caller sent a well-formed - request, and a missing secret key is a deployment configuration gap the - caller cannot fix. Blaming the client here would also keep the condition - out of 5xx error-rate alerting, which is exactly the audience that can. - - ``stored`` names what could not be stored, so the message points at the - surface the caller was using. It defaults to the provider credentials this - error was written for; workspace MCP servers pass their own. - """ - - def __init__(self, stored: str = "provider credentials") -> None: - super().__init__(f"OTARI_SECRET_KEY is not set; it is required to store {stored}") - - # The two below are pricing errors in a tenancy module, because the status # mapping is what decides where an error class lives here: one handler is # registered for ``TenancyError`` (see `gateway.main`), so an organization-scoped @@ -1324,7 +1304,6 @@ class SandboxImageNotAllowedError(TenancyValidationError): "ResetTokenInvalidError", "SandboxImageNotAllowedError", "SandboxToolsUnrunnableError", - "SecretBoxUnavailableTenancyError", "SignInAddressRequiredError", "TenancyConflictError", "TenancyError", diff --git a/src/gateway/services/tenancy/org_provider_key_service.py b/src/gateway/services/tenancy/org_provider_key_service.py index 5f0bd2f8aa..501ec0d06d 100644 --- a/src/gateway/services/tenancy/org_provider_key_service.py +++ b/src/gateway/services/tenancy/org_provider_key_service.py @@ -53,6 +53,7 @@ from gateway.core.config import PROVIDER_TYPE_ALIASES from gateway.core.database import create_session +from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.provider_keys import ( OrgProviderKey, @@ -97,7 +98,6 @@ OrgProviderKeyNotFoundError, OrgProviderKeyUnknownProviderError, OrgProviderKeyUnsafeApiBaseError, - SecretBoxUnavailableTenancyError, WorkspaceProviderKeyOverrideConflictError, ) from gateway.services.tenancy.organization_service import OrganizationService diff --git a/src/gateway/services/tenancy/organization_guardrail_definition_service.py b/src/gateway/services/tenancy/organization_guardrail_definition_service.py index 574abb91dd..ccc0521b64 100644 --- a/src/gateway/services/tenancy/organization_guardrail_definition_service.py +++ b/src/gateway/services/tenancy/organization_guardrail_definition_service.py @@ -60,6 +60,7 @@ from pydantic.json_schema import SkipJsonSchema from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.guardrails import OrganizationGuardrailDefinition from gateway.models.secret_fields import REDACTED_VALUE, restore_redacted_values @@ -88,7 +89,6 @@ OrganizationGuardrailDefinitionUnsafeUrlError, OrganizationGuardrailNotBuildableError, OrganizationGuardrailNotDefinableError, - SecretBoxUnavailableTenancyError, ) from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.url_safety import UnsafeURLError, validate_mcp_url diff --git a/src/gateway/services/tenancy/organization_guardrail_service.py b/src/gateway/services/tenancy/organization_guardrail_service.py index c318dd362b..2bda291fef 100644 --- a/src/gateway/services/tenancy/organization_guardrail_service.py +++ b/src/gateway/services/tenancy/organization_guardrail_service.py @@ -59,6 +59,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.guardrails import ( GuardrailConfig, @@ -87,7 +88,6 @@ OrganizationGuardrailSingleBackendError, OrganizationGuardrailTestsItsDefinitionError, OrganizationGuardrailUnsafeUrlError, - SecretBoxUnavailableTenancyError, WorkspaceNotFoundError, ) from gateway.services.tenancy.organization_service import OrganizationService diff --git a/src/gateway/services/tenancy/workspace_mcp_server_service.py b/src/gateway/services/tenancy/workspace_mcp_server_service.py index 3f06b5af91..d5423337ae 100644 --- a/src/gateway/services/tenancy/workspace_mcp_server_service.py +++ b/src/gateway/services/tenancy/workspace_mcp_server_service.py @@ -49,6 +49,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.models.mcp import McpServerConfig, ResolvedMcpServer from gateway.models.tenancy import User from gateway.models.tools import WorkspaceMcpServer @@ -60,7 +61,6 @@ ) from gateway.services.tenancy import authorization from gateway.services.tenancy.errors import ( - SecretBoxUnavailableTenancyError, WorkspaceMcpServerAlreadyExistsError, WorkspaceMcpServerLimitReachedError, WorkspaceMcpServerNotFoundError, From 052b7c2352f22d4a4702073f7c37cd7135c3e46c Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:16:22 +0100 Subject: [PATCH 3/9] refactor(exceptions): move the pricing errors into pricing_exceptions.py The three organization pricing errors sat in the tenancy error module because it was the only module whose classes the registered handler renders (#1193). `exceptions/` is now that home, so they move to their own domain's module. Pure move: the classes, their statuses and their messages are unchanged. The comment explaining why pricing errors lived under tenancy is dropped, since it no longer describes where they are. --- src/gateway/exceptions/pricing_exceptions.py | 65 +++++++++++++++++++ .../services/organization_pricing_service.py | 10 +-- src/gateway/services/tenancy/errors.py | 63 ------------------ .../test_organization_pricing_routes.py | 12 ++-- .../test_organization_pricing_resolution.py | 2 +- 5 files changed, 77 insertions(+), 75 deletions(-) create mode 100644 src/gateway/exceptions/pricing_exceptions.py diff --git a/src/gateway/exceptions/pricing_exceptions.py b/src/gateway/exceptions/pricing_exceptions.py new file mode 100644 index 0000000000..0dfe58acbc --- /dev/null +++ b/src/gateway/exceptions/pricing_exceptions.py @@ -0,0 +1,65 @@ +"""Errors an organization's pricing overrides raise, and the HTTP status each carries.""" + +from gateway.exceptions import TenancyConflictError, TenancyForbiddenError, TenancyNotFoundError + + +class OrganizationPricingNotFoundError(TenancyNotFoundError): + """No pricing override with this id in the caller's organization. + + One status for "never existed" and "belongs to another organization", the same + rule the base class states: a distinguishable 404 would tell one tenant which + ids exist in another. + """ + + def __init__(self, pricing_id: object): + super().__init__(f"Pricing override {pricing_id} not found") + + +class OrganizationPricingOverlapError(TenancyConflictError): + """A period that would overlap one this organization already priced. + + ``model_pricing`` lets a later row shadow an earlier one, because a catalog is + re-imported wholesale. An override is a commitment for a period, so two + periods covering one instant is an unanswerable question rather than a newest + wins rule, and it is refused with both periods named. + """ + + def __init__(self, model_key: str, existing_period: str): + super().__init__( + f"An override for '{model_key}' already covers part of that period ({existing_period}). " + "Change this period, or edit the existing override instead." + ) + + +class OrganizationPricingManagedModelError(TenancyForbiddenError): + """An override aimed at a model the deployment, not the organization, pays for. + + An override is how an organization records what it pays for a model it + supplies the provider key for. Two cases raise this: a model addressed through + a ``config.providers`` instance, and a bare ``provider:model`` key that the + bound ``ModelProviderPort`` would serve on a deployment-owned hosted credential + because a workspace lacks a usable BYO key. Both mean the deployment holds the + upstream credential and settles the upstream bill, so the rate is the + deployment price list's and a tenant-set rate would decide what that + deployment charges itself. A zero is the sharp end of it, since cost is also + what a budget counts down. + + Withheld from an organization manager, not from everyone: a deployment + operator is the party that pays, so the standalone deployment whose one + administrator is also its only tenant keeps setting its own rates exactly as + before (otari-ai#2095). + """ + + def __init__(self, model_key: str): + super().__init__( + f"'{model_key}' resolves on a credential this deployment, not your organization, supplies, " + "so its rate is set on the deployment price list rather than per organization. An override " + "applies to a model your organization supplies its own provider key for." + ) + + +__all__ = [ + "OrganizationPricingManagedModelError", + "OrganizationPricingNotFoundError", + "OrganizationPricingOverlapError", +] diff --git a/src/gateway/services/organization_pricing_service.py b/src/gateway/services/organization_pricing_service.py index 108f333f0c..483c901c2c 100644 --- a/src/gateway/services/organization_pricing_service.py +++ b/src/gateway/services/organization_pricing_service.py @@ -47,6 +47,11 @@ from gateway.core.config import GatewayConfig from gateway.exceptions import TenancyValidationError +from gateway.exceptions.pricing_exceptions import ( + OrganizationPricingManagedModelError, + OrganizationPricingNotFoundError, + OrganizationPricingOverlapError, +) from gateway.models.money import to_usd, to_usd_or_none from gateway.models.pricing import API_ORIGIN, ModelPricing, OrganizationModelPricing, PriceSource from gateway.models.tenancy import User as TenancyUser @@ -59,11 +64,6 @@ ) from gateway.services.provider_kwargs import is_deployment_instance_key, split_selector from gateway.services.tenancy.deployment_user_service import DeploymentUserService -from gateway.services.tenancy.errors import ( - OrganizationPricingManagedModelError, - OrganizationPricingNotFoundError, - OrganizationPricingOverlapError, -) from gateway.services.tenancy.org_provider_key_service import OrgProviderKeyService from gateway.services.tenancy.organization_service import OrganizationService diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/services/tenancy/errors.py index 8373a06d74..5f29eca2d2 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/services/tenancy/errors.py @@ -724,66 +724,6 @@ def __init__(self) -> None: super().__init__("A provider key override cannot be both pinned as default and disabled") -# The two below are pricing errors in a tenancy module, because the status -# mapping is what decides where an error class lives here: one handler is -# registered for ``TenancyError`` (see `gateway.main`), so an organization-scoped -# error that wants a status rather than a 500 has to descend from it. Naming them -# for what they are keeps that visible. -class OrganizationPricingNotFoundError(TenancyNotFoundError): - """No pricing override with this id in the caller's organization. - - One status for "never existed" and "belongs to another organization", the same - rule the base class states: a distinguishable 404 would tell one tenant which - ids exist in another. - """ - - def __init__(self, pricing_id: object): - super().__init__(f"Pricing override {pricing_id} not found") - - -class OrganizationPricingOverlapError(TenancyConflictError): - """A period that would overlap one this organization already priced. - - ``model_pricing`` lets a later row shadow an earlier one, because a catalog is - re-imported wholesale. An override is a commitment for a period, so two - periods covering one instant is an unanswerable question rather than a newest - wins rule, and it is refused with both periods named. - """ - - def __init__(self, model_key: str, existing_period: str): - super().__init__( - f"An override for '{model_key}' already covers part of that period ({existing_period}). " - "Change this period, or edit the existing override instead." - ) - - -class OrganizationPricingManagedModelError(TenancyForbiddenError): - """An override aimed at a model the deployment, not the organization, pays for. - - An override is how an organization records what it pays for a model it - supplies the provider key for. Two cases raise this: a model addressed through - a ``config.providers`` instance, and a bare ``provider:model`` key that the - bound ``ModelProviderPort`` would serve on a deployment-owned hosted credential - because a workspace lacks a usable BYO key. Both mean the deployment holds the - upstream credential and settles the upstream bill, so the rate is the - deployment price list's and a tenant-set rate would decide what that - deployment charges itself. A zero is the sharp end of it, since cost is also - what a budget counts down. - - Withheld from an organization manager, not from everyone: a deployment - operator is the party that pays, so the standalone deployment whose one - administrator is also its only tenant keeps setting its own rates exactly as - before (otari-ai#2095). - """ - - def __init__(self, model_key: str): - super().__init__( - f"'{model_key}' resolves on a credential this deployment, not your organization, supplies, " - "so its rate is set on the deployment price list rather than per organization. An override " - "applies to a model your organization supplies its own provider key for." - ) - - class InvitationNotFoundError(TenancyNotFoundError): """No invitation matches the token or id given. @@ -1288,10 +1228,7 @@ class SandboxImageNotAllowedError(TenancyValidationError): "OAuthNotConfiguredError", "OAuthStateError", "OrganizationNotFoundError", - "OrganizationPricingManagedModelError", - "OrganizationPricingNotFoundError", "OrganizationSlugUnavailableError", - "OrganizationPricingOverlapError", "PasskeyAlreadyRegisteredError", "PasskeyCeremonyError", "PasskeyLimitReachedError", diff --git a/tests/integration/test_organization_pricing_routes.py b/tests/integration/test_organization_pricing_routes.py index a9c9be192b..a11bbff01e 100644 --- a/tests/integration/test_organization_pricing_routes.py +++ b/tests/integration/test_organization_pricing_routes.py @@ -24,6 +24,11 @@ from gateway.core.config import API_ROOT, GatewayConfig from gateway.exceptions import TenancyValidationError +from gateway.exceptions.pricing_exceptions import ( + OrganizationPricingManagedModelError, + OrganizationPricingNotFoundError, + OrganizationPricingOverlapError, +) from gateway.models.api_keys import APIKey from gateway.models.pricing import ModelPricing, OrganizationModelPricing from gateway.models.tenancy import DashboardSession, Organization, OrganizationMember, User, Workspace @@ -44,12 +49,7 @@ from gateway.services.pricing_service import find_model_pricing from gateway.services.provider_kwargs import credential_ladder_exhausted, get_provider_kwargs from gateway.services.secret_box import encrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - OrganizationPricingManagedModelError, - OrganizationPricingNotFoundError, - OrganizationPricingOverlapError, -) +from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.org_provider_key_service import refresh_org_provider_cache, reset_org_provider_cache from gateway.services.workspace_scope import ( organization_for_key_id, diff --git a/tests/unit/test_organization_pricing_resolution.py b/tests/unit/test_organization_pricing_resolution.py index c7fbb524ae..685310b5f8 100644 --- a/tests/unit/test_organization_pricing_resolution.py +++ b/tests/unit/test_organization_pricing_resolution.py @@ -25,6 +25,7 @@ import gateway.models # noqa: F401 (registers every table on the shared metadata) from gateway.core.config import GatewayConfig from gateway.exceptions import TenancyValidationError +from gateway.exceptions.pricing_exceptions import OrganizationPricingOverlapError from gateway.models.pricing import ModelPricing, OrganizationModelPricing from gateway.models.tenancy import Organization from gateway.services.external_usage_service import _load_pricing_index, _resolve_pricing @@ -33,7 +34,6 @@ validate_period, ) from gateway.services.pricing_service import find_model_pricing, price_tool_calls -from gateway.services.tenancy.errors import OrganizationPricingOverlapError T = TypeVar("T") From a3c6407adb1cee4b18c37ab900e1a6a91e2c51a5 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:17:35 +0100 Subject: [PATCH 4/9] refactor(exceptions): move the provider key errors into providers_exceptions.py The ten provider-key errors join the offered-model errors already in the providers module (#1193), key errors first, since a model is offered on a key. The module gains an `__all__`, which the split would otherwise drop for the ten names the tenancy module listed. Pure move: the classes, their statuses and their messages are unchanged. --- .../exceptions/providers_exceptions.py | 127 ++++++++++++++++++ .../providers/_org_provider_model_service.py | 2 +- src/gateway/services/tenancy/errors.py | 118 ---------------- .../tenancy/org_provider_key_service.py | 24 ++-- .../test_org_provider_key_models.py | 6 +- tests/integration/test_org_provider_keys.py | 19 ++- 6 files changed, 151 insertions(+), 145 deletions(-) diff --git a/src/gateway/exceptions/providers_exceptions.py b/src/gateway/exceptions/providers_exceptions.py index 78978613b4..5e1a1fba99 100644 --- a/src/gateway/exceptions/providers_exceptions.py +++ b/src/gateway/exceptions/providers_exceptions.py @@ -3,6 +3,114 @@ from gateway.exceptions import TenancyConflictError, TenancyNotFoundError, TenancyValidationError +class OrgProviderKeyNotFoundError(TenancyNotFoundError): + def __init__(self, key_id: object): + super().__init__(f"Provider key {key_id} not found") + + +class OrgProviderKeyNameRequiredError(TenancyValidationError): + """A key name that is absent, null, or blank once trimmed. + + ``OrgProviderKey.name`` is NOT NULL, and ``OrgProviderKeyUpdateRequest`` + types it as nullable so a client can send an explicit ``null``, the same + shape ``WorkspaceNameRequiredError`` guards against for a workspace. Left + unguarded, an explicit ``null`` reaches the database as a NOT NULL + violation, which the surrounding duplicate-name handling then reports as a + 409 naming a key called "None" rather than the 400 this is. + """ + + def __init__(self) -> None: + super().__init__("A provider key name is required") + + +class OrgProviderKeyUnknownProviderError(TenancyValidationError): + """A ``provider`` that is blank, or does not resolve to a known any-llm implementation. + + ``OrgProviderKey.provider`` is stored verbatim and is exactly the string + ``cached_org_provider_kwargs`` keys its cache on, matched against a + resolved selector's ``LLMProvider.value`` at dispatch (see + ``org_provider_key_service.refresh_org_provider_cache``). Left unguarded, + a typo, unexpected casing, or an unaliased value (``"OpenAI"``, + ``"azure-openai"``, trailing whitespace) is accepted with a 201 and then + never resolves at dispatch, with no error at either point. Mirrors + ``/v1/provider-credentials``'s ``_validate_instance`` provider_type guard, + including ``PROVIDER_TYPE_ALIASES`` so an aliased name still resolves. + """ + + def __init__(self, provider: str) -> None: + if not provider: + super().__init__("A provider is required") + else: + super().__init__(f"'{provider}' is not a known provider implementation") + + +class OrgProviderKeyUnsafeApiBaseError(TenancyValidationError): + """An ``api_base`` that resolves to an internal address, gated off. + + Wraps ``services.url_safety.UnsafeURLError`` as a tenancy error so the + route stays thin; the message is that function's own, which already + carries no more than the host it refused. + """ + + def __init__(self, message: str) -> None: + super().__init__(message) + + +class OrgProviderKeyAlreadyExistsError(TenancyConflictError): + def __init__(self, provider: str, name: str): + super().__init__(f"A '{provider}' key named '{name}' already exists in this organization") + + +class OrgProviderKeyArchivedError(TenancyValidationError): + """The key is archived, which refuses every mutation except restore.""" + + def __init__(self, key_id: object): + super().__init__(f"Provider key {key_id} is archived; restore it before changing it") + + +class OrgProviderKeyNotArchivedError(TenancyValidationError): + """Deletion requires archiving first, the same two-step every irreversible action here takes.""" + + def __init__(self, key_id: object): + super().__init__(f"Provider key {key_id} must be archived before it can be deleted") + + +class OrgDefaultProviderKeyConflictError(TenancyConflictError): + """Two concurrent 'set default' calls raced for the same (organization, provider). + + The partial unique index (``uq_org_provider_keys_org_default``) is the + actual arbiter; this is what the loser's ``IntegrityError`` is mapped to. + """ + + def __init__(self, provider: str) -> None: + super().__init__(f"Another request just changed the default '{provider}' key; retry") + + +class OrgProviderKeyDisabledForWorkspaceError(TenancyValidationError): + """A model restriction was requested for a key this workspace has disabled. + + Refused rather than stored: a restriction on a key the workspace cannot + use anyway would resurface with a stale list if the key were re-enabled + later, which `set_workspace_override_for_user` already deletes for the + opposite transition (see the repository docstring on the cascade). + """ + + def __init__(self) -> None: + super().__init__("This provider key is disabled for the workspace; enable it before restricting its models") + + +class WorkspaceProviderKeyOverrideConflictError(TenancyValidationError): + """A caller asked to pin and disable the same key in the same request. + + Sending one flag lets the other auto-resolve (pinning re-enables a + disabled key, disabling un-pins a pinned one); sending both explicitly + true is a contradiction with no safe default to pick. + """ + + def __init__(self) -> None: + super().__init__("A provider key override cannot be both pinned as default and disabled") + + class OrgProviderModelNotFoundError(TenancyNotFoundError): def __init__(self, model_id: object): super().__init__(f"Offered model {model_id} not found") @@ -57,3 +165,22 @@ class OrgProviderModelAlreadyOfferedError(TenancyConflictError): def __init__(self, provider: str, model: str) -> None: super().__init__(f"'{model}' is already offered on this '{provider}' key") + + +__all__ = [ + "OrgDefaultProviderKeyConflictError", + "OrgProviderKeyAlreadyExistsError", + "OrgProviderKeyArchivedError", + "OrgProviderKeyDisabledForWorkspaceError", + "OrgProviderKeyNameRequiredError", + "OrgProviderKeyNotArchivedError", + "OrgProviderKeyNotFoundError", + "OrgProviderKeyUnknownProviderError", + "OrgProviderKeyUnsafeApiBaseError", + "OrgProviderLastModelError", + "OrgProviderModelAlreadyOfferedError", + "OrgProviderModelNameRequiredError", + "OrgProviderModelNotFoundError", + "OrgProviderModelUnpricedError", + "WorkspaceProviderKeyOverrideConflictError", +] diff --git a/src/gateway/services/providers/_org_provider_model_service.py b/src/gateway/services/providers/_org_provider_model_service.py index b6201ccd08..e253df2a85 100644 --- a/src/gateway/services/providers/_org_provider_model_service.py +++ b/src/gateway/services/providers/_org_provider_model_service.py @@ -46,6 +46,7 @@ from gateway.core.metered_pricing import quantize_rate from gateway.core.unit_of_work import UnitOfWork from gateway.exceptions.providers_exceptions import ( + OrgProviderKeyNotFoundError, OrgProviderLastModelError, OrgProviderModelAlreadyOfferedError, OrgProviderModelNameRequiredError, @@ -74,7 +75,6 @@ from gateway.services.organization_pricing_service import EffectiveRate, OrganizationPricingService from gateway.services.pricing_service import default_model_pricing, normalize_effective_at from gateway.services.secret_box import SecretBoxUnavailableError, SecretDecryptionError, decrypt_secret -from gateway.services.tenancy.errors import OrgProviderKeyNotFoundError from gateway.services.tenancy.org_provider_key_service import OrgProviderKeyService from gateway.services.tenancy.organization_service import OrganizationService diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/services/tenancy/errors.py index 5f29eca2d2..c1fc8d3351 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/services/tenancy/errors.py @@ -616,114 +616,6 @@ def __init__(self) -> None: super().__init__("An organization keeps at least one workspace; create another before deleting this one") -class OrgProviderKeyNotFoundError(TenancyNotFoundError): - def __init__(self, key_id: object): - super().__init__(f"Provider key {key_id} not found") - - -class OrgProviderKeyNameRequiredError(TenancyValidationError): - """A key name that is absent, null, or blank once trimmed. - - ``OrgProviderKey.name`` is NOT NULL, and ``OrgProviderKeyUpdateRequest`` - types it as nullable so a client can send an explicit ``null``, the same - shape ``WorkspaceNameRequiredError`` guards against for a workspace. Left - unguarded, an explicit ``null`` reaches the database as a NOT NULL - violation, which the surrounding duplicate-name handling then reports as a - 409 naming a key called "None" rather than the 400 this is. - """ - - def __init__(self) -> None: - super().__init__("A provider key name is required") - - -class OrgProviderKeyUnknownProviderError(TenancyValidationError): - """A ``provider`` that is blank, or does not resolve to a known any-llm implementation. - - ``OrgProviderKey.provider`` is stored verbatim and is exactly the string - ``cached_org_provider_kwargs`` keys its cache on, matched against a - resolved selector's ``LLMProvider.value`` at dispatch (see - ``org_provider_key_service.refresh_org_provider_cache``). Left unguarded, - a typo, unexpected casing, or an unaliased value (``"OpenAI"``, - ``"azure-openai"``, trailing whitespace) is accepted with a 201 and then - never resolves at dispatch, with no error at either point. Mirrors - ``/v1/provider-credentials``'s ``_validate_instance`` provider_type guard, - including ``PROVIDER_TYPE_ALIASES`` so an aliased name still resolves. - """ - - def __init__(self, provider: str) -> None: - if not provider: - super().__init__("A provider is required") - else: - super().__init__(f"'{provider}' is not a known provider implementation") - - -class OrgProviderKeyUnsafeApiBaseError(TenancyValidationError): - """An ``api_base`` that resolves to an internal address, gated off. - - Wraps ``services.url_safety.UnsafeURLError`` as a tenancy error so the - route stays thin; the message is that function's own, which already - carries no more than the host it refused. - """ - - def __init__(self, message: str) -> None: - super().__init__(message) - - -class OrgProviderKeyAlreadyExistsError(TenancyConflictError): - def __init__(self, provider: str, name: str): - super().__init__(f"A '{provider}' key named '{name}' already exists in this organization") - - -class OrgProviderKeyArchivedError(TenancyValidationError): - """The key is archived, which refuses every mutation except restore.""" - - def __init__(self, key_id: object): - super().__init__(f"Provider key {key_id} is archived; restore it before changing it") - - -class OrgProviderKeyNotArchivedError(TenancyValidationError): - """Deletion requires archiving first, the same two-step every irreversible action here takes.""" - - def __init__(self, key_id: object): - super().__init__(f"Provider key {key_id} must be archived before it can be deleted") - - -class OrgDefaultProviderKeyConflictError(TenancyConflictError): - """Two concurrent 'set default' calls raced for the same (organization, provider). - - The partial unique index (``uq_org_provider_keys_org_default``) is the - actual arbiter; this is what the loser's ``IntegrityError`` is mapped to. - """ - - def __init__(self, provider: str) -> None: - super().__init__(f"Another request just changed the default '{provider}' key; retry") - - -class OrgProviderKeyDisabledForWorkspaceError(TenancyValidationError): - """A model restriction was requested for a key this workspace has disabled. - - Refused rather than stored: a restriction on a key the workspace cannot - use anyway would resurface with a stale list if the key were re-enabled - later, which `set_workspace_override_for_user` already deletes for the - opposite transition (see the repository docstring on the cascade). - """ - - def __init__(self) -> None: - super().__init__("This provider key is disabled for the workspace; enable it before restricting its models") - - -class WorkspaceProviderKeyOverrideConflictError(TenancyValidationError): - """A caller asked to pin and disable the same key in the same request. - - Sending one flag lets the other auto-resolve (pinning re-enables a - disabled key, disabling un-pins a pinned one); sending both explicitly - true is a contradiction with no safe default to pick. - """ - - def __init__(self) -> None: - super().__init__("A provider key override cannot be both pinned as default and disabled") - - class InvitationNotFoundError(TenancyNotFoundError): """No invitation matches the token or id given. @@ -1200,15 +1092,6 @@ class SandboxImageNotAllowedError(TenancyValidationError): "MembershipUpdateError", "NotAnOrganizationMemberError", "NotAuthorizedError", - "OrgDefaultProviderKeyConflictError", - "OrgProviderKeyAlreadyExistsError", - "OrgProviderKeyArchivedError", - "OrgProviderKeyDisabledForWorkspaceError", - "OrgProviderKeyNameRequiredError", - "OrgProviderKeyNotArchivedError", - "OrgProviderKeyNotFoundError", - "OrgProviderKeyUnknownProviderError", - "OrgProviderKeyUnsafeApiBaseError", "OrganizationGuardrailAlreadyExistsError", "OrganizationGuardrailCredentialNeedsUrlError", "OrganizationGuardrailLimitReachedError", @@ -1264,6 +1147,5 @@ class SandboxImageNotAllowedError(TenancyValidationError): "WorkspaceMemberNotFoundError", "WorkspaceNameRequiredError", "WorkspaceNotFoundError", - "WorkspaceProviderKeyOverrideConflictError", "WorkspaceWebSearchDomainsExcludedError", ] diff --git a/src/gateway/services/tenancy/org_provider_key_service.py b/src/gateway/services/tenancy/org_provider_key_service.py index 501ec0d06d..0f0ea83b57 100644 --- a/src/gateway/services/tenancy/org_provider_key_service.py +++ b/src/gateway/services/tenancy/org_provider_key_service.py @@ -53,6 +53,18 @@ from gateway.core.config import PROVIDER_TYPE_ALIASES from gateway.core.database import create_session +from gateway.exceptions.providers_exceptions import ( + OrgDefaultProviderKeyConflictError, + OrgProviderKeyAlreadyExistsError, + OrgProviderKeyArchivedError, + OrgProviderKeyDisabledForWorkspaceError, + OrgProviderKeyNameRequiredError, + OrgProviderKeyNotArchivedError, + OrgProviderKeyNotFoundError, + OrgProviderKeyUnknownProviderError, + OrgProviderKeyUnsafeApiBaseError, + WorkspaceProviderKeyOverrideConflictError, +) from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.provider_keys import ( @@ -88,18 +100,6 @@ encrypt_secret, ) from gateway.services.tenancy import authorization -from gateway.services.tenancy.errors import ( - OrgDefaultProviderKeyConflictError, - OrgProviderKeyAlreadyExistsError, - OrgProviderKeyArchivedError, - OrgProviderKeyDisabledForWorkspaceError, - OrgProviderKeyNameRequiredError, - OrgProviderKeyNotArchivedError, - OrgProviderKeyNotFoundError, - OrgProviderKeyUnknownProviderError, - OrgProviderKeyUnsafeApiBaseError, - WorkspaceProviderKeyOverrideConflictError, -) from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.url_safety import UnsafeURLError, validate_provider_api_base diff --git a/tests/integration/test_org_provider_key_models.py b/tests/integration/test_org_provider_key_models.py index 8f9fade7d2..1cdfd94a46 100644 --- a/tests/integration/test_org_provider_key_models.py +++ b/tests/integration/test_org_provider_key_models.py @@ -25,6 +25,7 @@ from gateway.core.config import GatewayConfig from gateway.core.unit_of_work import UnitOfWork from gateway.exceptions.providers_exceptions import ( + OrgProviderKeyNotFoundError, OrgProviderLastModelError, OrgProviderModelAlreadyOfferedError, OrgProviderModelNameRequiredError, @@ -56,10 +57,7 @@ from gateway.services.providers import OrgProviderModelService from gateway.services.secret_box import generate_secret_key from gateway.services.tenancy import OrgProviderKeyService -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - OrgProviderKeyNotFoundError, -) +from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.org_provider_key_service import ( cached_org_model_restriction, refresh_org_provider_cache, diff --git a/tests/integration/test_org_provider_keys.py b/tests/integration/test_org_provider_keys.py index 5d91af82e0..31b6821b2c 100644 --- a/tests/integration/test_org_provider_keys.py +++ b/tests/integration/test_org_provider_keys.py @@ -18,6 +18,14 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig +from gateway.exceptions.providers_exceptions import ( + OrgProviderKeyAlreadyExistsError, + OrgProviderKeyArchivedError, + OrgProviderKeyDisabledForWorkspaceError, + OrgProviderKeyNotArchivedError, + OrgProviderKeyNotFoundError, + WorkspaceProviderKeyOverrideConflictError, +) from gateway.models.provider_keys import ( OrgProviderKey, ) @@ -39,16 +47,7 @@ from gateway.services.provider_kwargs import resolve_provider_selector from gateway.services.secret_box import encrypt_secret, generate_secret_key from gateway.services.tenancy import OrgProviderKeyService -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - OrgProviderKeyAlreadyExistsError, - OrgProviderKeyArchivedError, - OrgProviderKeyDisabledForWorkspaceError, - OrgProviderKeyNotArchivedError, - OrgProviderKeyNotFoundError, - WorkspaceNotFoundError, - WorkspaceProviderKeyOverrideConflictError, -) +from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.org_provider_key_service import ( cached_org_model_restriction, refresh_org_provider_cache, From 8434c1fc3186410b8a7a09826ca8e70acae01414 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:18:09 +0100 Subject: [PATCH 5/9] refactor(exceptions): move the tools errors into tools_exceptions.py Workspace MCP servers, workspace web search and the sandbox code-execution policy are all the tools domain, so their seven errors leave the tenancy module for their own (#1193). Pure move: the classes, their statuses and their messages are unchanged. --- src/gateway/api/routes/_pipeline.py | 5 +- src/gateway/exceptions/tools_exceptions.py | 98 +++++++++++++++++++ src/gateway/services/tenancy/errors.py | 91 ----------------- ...workspace_code_execution_policy_service.py | 2 +- .../tenancy/workspace_mcp_server_service.py | 12 +-- .../tenancy/workspace_web_search_service.py | 2 +- .../test_workspace_code_execution_policy.py | 8 +- .../integration/test_workspace_mcp_servers.py | 15 ++- tests/unit/test_pipeline_settlement.py | 2 +- tests/unit/test_web_search_narrowing.py | 2 +- 10 files changed, 118 insertions(+), 119 deletions(-) create mode 100644 src/gateway/exceptions/tools_exceptions.py diff --git a/src/gateway/api/routes/_pipeline.py b/src/gateway/api/routes/_pipeline.py index 4351734346..5f008da944 100644 --- a/src/gateway/api/routes/_pipeline.py +++ b/src/gateway/api/routes/_pipeline.py @@ -124,6 +124,7 @@ cache_write_1h_tokens_of, cache_write_tokens_of, ) +from gateway.exceptions.tools_exceptions import WorkspaceMcpServerNotFoundError, WorkspaceWebSearchDomainsExcludedError from gateway.inflight import track_request from gateway.log_config import logger from gateway.metrics import REGISTRY, Histogram @@ -198,10 +199,6 @@ SandboxUnavailableError, ) from gateway.services.secret_box import SecretBoxUnavailableError, SecretDecryptionError -from gateway.services.tenancy.errors import ( - WorkspaceMcpServerNotFoundError, - WorkspaceWebSearchDomainsExcludedError, -) from gateway.services.tenancy.org_provider_key_service import cached_org_model_restriction from gateway.services.tenancy.organization_guardrail_runner import handle as guardrail_handle from gateway.services.tenancy.organization_guardrail_service import ( diff --git a/src/gateway/exceptions/tools_exceptions.py b/src/gateway/exceptions/tools_exceptions.py new file mode 100644 index 0000000000..1f5a54ebe5 --- /dev/null +++ b/src/gateway/exceptions/tools_exceptions.py @@ -0,0 +1,98 @@ +"""Errors that the tools domain may raise, each carrying the status it renders as.""" + +from gateway.exceptions import TenancyConflictError, TenancyForbiddenError, TenancyNotFoundError, TenancyValidationError + + +class WorkspaceMcpServerNotFoundError(TenancyNotFoundError): + def __init__(self, mcp_server_id: object): + super().__init__(f"MCP server {mcp_server_id} not found") + + +class WorkspaceMcpServerAlreadyExistsError(TenancyConflictError): + """A workspace already has an MCP server under this name. + + Refused rather than collapsed onto the existing row: the name is what the + tool loop labels a server's tools with, so silently reusing it would point + a caller's request at a different endpoint than the one they just + configured. + """ + + def __init__(self, workspace_id: object, name: object): + super().__init__(f"Workspace {workspace_id} already has an MCP server named '{name}'") + + +class WorkspaceMcpServerUnsafeUrlError(TenancyValidationError): + """The URL failed the same SSRF and TLS checks a request-body MCP server faces. + + Carries the reason from `services.url_safety.UnsafeURLError` verbatim: it + names the host and the range it resolved into, which is what an operator + needs to fix the entry, and it is the operator's own URL either way (this + surface is management-gated, not a caller-supplied endpoint). + """ + + def __init__(self, reason: str): + super().__init__(reason) + + +class WorkspaceMcpServerLimitReachedError(TenancyValidationError): + """The workspace already holds as many MCP servers as it may. + + A resolved request opens a session to every server it names, so the cap + bounds the fan-out one workspace can ask a gateway process for. + """ + + def __init__(self, workspace_id: object, limit: int): + super().__init__(f"Workspace {workspace_id} already has the maximum of {limit} MCP servers") + + +class WorkspaceWebSearchDomainsExcludedError(TenancyForbiddenError): + """A request's search allow-list shares no domain with its workspace's. + + The two lists are intersected rather than overridden, so this is the empty + intersection: every domain the request asked for is one the workspace does + not permit. Refused rather than run, because an empty effective allow-list + is read by ``_build_web_retrieval_backend`` as *no* allow-list (an empty list + is falsy), which would turn the narrowest possible policy into no policy at + all. + """ + + def __init__(self) -> None: + super().__init__("The requested search domains are not permitted for this workspace") + + +class SandboxToolsUnrunnableError(TenancyValidationError): + """A code-execution policy's tool list names nothing this deployment serves. + + The list intersects what the sandbox backend offers, so this one resolves to + an empty set and every request would answer 403. Refused at the write rather + than stored, because a policy that reads as a refinement and behaves as a + refusal is the failure the surface exists to prevent, and the operator's only + signal would be users reporting 403s days later. + """ + + def __init__(self, served: tuple[str, ...]): + super().__init__( + "A code-execution tool list must name at least one tool this deployment serves " + f"({', '.join(served)}). Use enabled=false to refuse the workspace instead." + ) + + +class SandboxImageNotAllowedError(TenancyValidationError): + """A workspace code-execution policy named a sandbox image the operator has not curated. + + A 400 rather than a 403: the caller has the role to set the policy, and the + value they sent is the thing being refused. The message names the allowed + set, which is not a disclosure worth withholding, because that set is + already reported on the policy itself so the dashboard can offer it. + """ + + +__all__ = [ + "SandboxImageNotAllowedError", + "SandboxToolsUnrunnableError", + "WorkspaceMcpServerAlreadyExistsError", + "WorkspaceMcpServerLimitReachedError", + "WorkspaceMcpServerNotFoundError", + "WorkspaceMcpServerUnsafeUrlError", + "WorkspaceWebSearchDomainsExcludedError", +] diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/services/tenancy/errors.py index c1fc8d3351..a72bc3f869 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/services/tenancy/errors.py @@ -713,63 +713,6 @@ def __init__(self) -> None: super().__init__("This workspace has already served a successful request") -class WorkspaceMcpServerNotFoundError(TenancyNotFoundError): - def __init__(self, mcp_server_id: object): - super().__init__(f"MCP server {mcp_server_id} not found") - - -class WorkspaceMcpServerAlreadyExistsError(TenancyConflictError): - """A workspace already has an MCP server under this name. - - Refused rather than collapsed onto the existing row: the name is what the - tool loop labels a server's tools with, so silently reusing it would point - a caller's request at a different endpoint than the one they just - configured. - """ - - def __init__(self, workspace_id: object, name: object): - super().__init__(f"Workspace {workspace_id} already has an MCP server named '{name}'") - - -class WorkspaceMcpServerUnsafeUrlError(TenancyValidationError): - """The URL failed the same SSRF and TLS checks a request-body MCP server faces. - - Carries the reason from `services.url_safety.UnsafeURLError` verbatim: it - names the host and the range it resolved into, which is what an operator - needs to fix the entry, and it is the operator's own URL either way (this - surface is management-gated, not a caller-supplied endpoint). - """ - - def __init__(self, reason: str): - super().__init__(reason) - - -class WorkspaceMcpServerLimitReachedError(TenancyValidationError): - """The workspace already holds as many MCP servers as it may. - - A resolved request opens a session to every server it names, so the cap - bounds the fan-out one workspace can ask a gateway process for. - """ - - def __init__(self, workspace_id: object, limit: int): - super().__init__(f"Workspace {workspace_id} already has the maximum of {limit} MCP servers") - - -class WorkspaceWebSearchDomainsExcludedError(TenancyForbiddenError): - """A request's search allow-list shares no domain with its workspace's. - - The two lists are intersected rather than overridden, so this is the empty - intersection: every domain the request asked for is one the workspace does - not permit. Refused rather than run, because an empty effective allow-list - is read by ``_build_web_retrieval_backend`` as *no* allow-list (an empty list - is falsy), which would turn the narrowest possible policy into no policy at - all. - """ - - def __init__(self) -> None: - super().__init__("The requested search domains are not permitted for this workspace") - - class OrganizationGuardrailNotFoundError(TenancyNotFoundError): def __init__(self, guardrail_id: object): super().__init__(f"Organization guardrail {guardrail_id} not found") @@ -1041,33 +984,6 @@ def __init__(self, profiles: list[str]): ) -class SandboxToolsUnrunnableError(TenancyValidationError): - """A code-execution policy's tool list names nothing this deployment serves. - - The list intersects what the sandbox backend offers, so this one resolves to - an empty set and every request would answer 403. Refused at the write rather - than stored, because a policy that reads as a refinement and behaves as a - refusal is the failure the surface exists to prevent, and the operator's only - signal would be users reporting 403s days later. - """ - - def __init__(self, served: tuple[str, ...]): - super().__init__( - "A code-execution tool list must name at least one tool this deployment serves " - f"({', '.join(served)}). Use enabled=false to refuse the workspace instead." - ) - - -class SandboxImageNotAllowedError(TenancyValidationError): - """A workspace code-execution policy named a sandbox image the operator has not curated. - - A 400 rather than a 403: the caller has the role to set the policy, and the - value they sent is the thing being refused. The message names the allowed - set, which is not a disclosure worth withholding, because that set is - already reported on the policy itself so the dashboard can offer it. - """ - - __all__ = [ "BootstrapOperatorProtectedError", "CurrentPasswordIncorrectError", @@ -1122,8 +1038,6 @@ class SandboxImageNotAllowedError(TenancyValidationError): "PasswordNotSetError", "PasswordPolicyError", "ResetTokenInvalidError", - "SandboxImageNotAllowedError", - "SandboxToolsUnrunnableError", "SignInAddressRequiredError", "TenancyConflictError", "TenancyError", @@ -1139,13 +1053,8 @@ class SandboxImageNotAllowedError(TenancyValidationError): "WorkspaceActivationUnavailableError", "WorkspaceAlreadyActivatedError", "WorkspaceInUseError", - "WorkspaceMcpServerAlreadyExistsError", - "WorkspaceMcpServerLimitReachedError", - "WorkspaceMcpServerNotFoundError", - "WorkspaceMcpServerUnsafeUrlError", "WorkspaceMemberAlreadyExistsError", "WorkspaceMemberNotFoundError", "WorkspaceNameRequiredError", "WorkspaceNotFoundError", - "WorkspaceWebSearchDomainsExcludedError", ] diff --git a/src/gateway/services/tenancy/workspace_code_execution_policy_service.py b/src/gateway/services/tenancy/workspace_code_execution_policy_service.py index f8632b74c3..cca064e788 100644 --- a/src/gateway/services/tenancy/workspace_code_execution_policy_service.py +++ b/src/gateway/services/tenancy/workspace_code_execution_policy_service.py @@ -24,6 +24,7 @@ from sqlalchemy.exc import IntegrityError, SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.tools_exceptions import SandboxImageNotAllowedError, SandboxToolsUnrunnableError from gateway.models.tenancy import User, Workspace from gateway.models.tools import CodeExecutor, WorkspaceCodeExecutionPolicy from gateway.services.mcp_loop import MAX_TOOL_ITERATIONS_CAP @@ -33,7 +34,6 @@ DEFAULT_EXEC_TIMEOUT_S, ) from gateway.services.tenancy import authorization -from gateway.services.tenancy.errors import SandboxImageNotAllowedError, SandboxToolsUnrunnableError from gateway.services.tenancy.organization_service import OrganizationService # A policy may only narrow, so a value above either ceiling would read as a diff --git a/src/gateway/services/tenancy/workspace_mcp_server_service.py b/src/gateway/services/tenancy/workspace_mcp_server_service.py index d5423337ae..a50f3a28d2 100644 --- a/src/gateway/services/tenancy/workspace_mcp_server_service.py +++ b/src/gateway/services/tenancy/workspace_mcp_server_service.py @@ -50,6 +50,12 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError +from gateway.exceptions.tools_exceptions import ( + WorkspaceMcpServerAlreadyExistsError, + WorkspaceMcpServerLimitReachedError, + WorkspaceMcpServerNotFoundError, + WorkspaceMcpServerUnsafeUrlError, +) from gateway.models.mcp import McpServerConfig, ResolvedMcpServer from gateway.models.tenancy import User from gateway.models.tools import WorkspaceMcpServer @@ -60,12 +66,6 @@ encrypt_secret, ) from gateway.services.tenancy import authorization -from gateway.services.tenancy.errors import ( - WorkspaceMcpServerAlreadyExistsError, - WorkspaceMcpServerLimitReachedError, - WorkspaceMcpServerNotFoundError, - WorkspaceMcpServerUnsafeUrlError, -) from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.url_safety import UnsafeURLError, redact_url_secrets, validate_mcp_url diff --git a/src/gateway/services/tenancy/workspace_web_search_service.py b/src/gateway/services/tenancy/workspace_web_search_service.py index 1312be38be..d454111aab 100644 --- a/src/gateway/services/tenancy/workspace_web_search_service.py +++ b/src/gateway/services/tenancy/workspace_web_search_service.py @@ -57,10 +57,10 @@ from sqlalchemy.exc import IntegrityError, SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.tools_exceptions import WorkspaceWebSearchDomainsExcludedError from gateway.models.tenancy import User, Workspace from gateway.models.tools import WorkspaceWebSearchConfig from gateway.services.tenancy import authorization -from gateway.services.tenancy.errors import WorkspaceWebSearchDomainsExcludedError from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.web_retrieval_backend import MAX_RESULTS_CAP from gateway.services.web_retrieval_policy import ( diff --git a/tests/integration/test_workspace_code_execution_policy.py b/tests/integration/test_workspace_code_execution_policy.py index 9ebbd812ce..d202973dce 100644 --- a/tests/integration/test_workspace_code_execution_policy.py +++ b/tests/integration/test_workspace_code_execution_policy.py @@ -16,6 +16,7 @@ from pydantic import ValidationError from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from gateway.exceptions.tools_exceptions import SandboxImageNotAllowedError, SandboxToolsUnrunnableError from gateway.models.tenancy import Organization, User, Workspace from gateway.models.tools import WorkspaceCodeExecutionPolicy from gateway.repositories.tenancy import ( @@ -26,12 +27,7 @@ WorkspaceRepository, ) from gateway.services.sandbox_backend import CODE_EXECUTION_TOOL_NAMES -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - SandboxImageNotAllowedError, - SandboxToolsUnrunnableError, - WorkspaceNotFoundError, -) +from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.workspace_code_execution_policy_service import ( SERVED_TOOL_NAMES, WorkspaceCodeExecutionPolicyService, diff --git a/tests/integration/test_workspace_mcp_servers.py b/tests/integration/test_workspace_mcp_servers.py index e2d6bf2453..7d12a3cc11 100644 --- a/tests/integration/test_workspace_mcp_servers.py +++ b/tests/integration/test_workspace_mcp_servers.py @@ -29,6 +29,12 @@ from gateway.api.routes.chat import ChatCompletionRequest from gateway.core.config import GatewayConfig from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions.tools_exceptions import ( + WorkspaceMcpServerAlreadyExistsError, + WorkspaceMcpServerLimitReachedError, + WorkspaceMcpServerNotFoundError, + WorkspaceMcpServerUnsafeUrlError, +) from gateway.models.mcp import MAX_MCP_SERVER_IDS, McpServerConfig from gateway.models.tenancy import Organization, User, Workspace from gateway.models.tools import WorkspaceMcpServer @@ -40,14 +46,7 @@ WorkspaceRepository, ) from gateway.services.secret_box import decrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - WorkspaceMcpServerAlreadyExistsError, - WorkspaceMcpServerLimitReachedError, - WorkspaceMcpServerNotFoundError, - WorkspaceMcpServerUnsafeUrlError, - WorkspaceNotFoundError, -) +from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.workspace_mcp_server_service import ( MAX_ALLOWED_TOOLS, MAX_MCP_SERVERS_PER_WORKSPACE, diff --git a/tests/unit/test_pipeline_settlement.py b/tests/unit/test_pipeline_settlement.py index a563a544bc..aaa4e70919 100644 --- a/tests/unit/test_pipeline_settlement.py +++ b/tests/unit/test_pipeline_settlement.py @@ -56,11 +56,11 @@ ) from gateway.api.routes._platform import ResolvedAttempt, ResolvedRoute, SettledCost from gateway.core.config import GatewayConfig +from gateway.exceptions.tools_exceptions import WorkspaceMcpServerNotFoundError from gateway.models.mcp import McpServerConfig from gateway.models.pricing import PriceSource from gateway.rate_limit import RateLimitInfo from gateway.services.budgets import ReservationHandle -from gateway.services.tenancy.errors import WorkspaceMcpServerNotFoundError from gateway.services.tenancy.workspace_web_search_service import ResolvedWebSearchConfig from gateway.services.tool_usage import ToolUsageTally diff --git a/tests/unit/test_web_search_narrowing.py b/tests/unit/test_web_search_narrowing.py index a9371e1c5e..747bc82f5f 100644 --- a/tests/unit/test_web_search_narrowing.py +++ b/tests/unit/test_web_search_narrowing.py @@ -18,7 +18,7 @@ import pytest -from gateway.services.tenancy.errors import WorkspaceWebSearchDomainsExcludedError +from gateway.exceptions.tools_exceptions import WorkspaceWebSearchDomainsExcludedError from gateway.services.tenancy.workspace_web_search_service import ( _MAX_DOMAINS, _MAX_RESULTS, From f22b795527c3420e25ed4423d8684abc358266dd Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:19:45 +0100 Subject: [PATCH 6/9] refactor(exceptions): move the guardrails errors into guardrails_exceptions.py The twenty errors of the two organization guardrail surfaces, mandates and definitions, leave the tenancy module for their own domain's (#1193). The route docstrings that pointed at the tenancy module for where a status comes from now name the domain module, pricing's included. Pure move: the classes, their statuses and their messages are unchanged. --- .../organization_guardrail_definitions.py | 4 +- .../api/routes/organization_guardrails.py | 4 +- .../api/routes/organization_pricing.py | 4 +- .../exceptions/guardrails_exceptions.py | 300 ++++++++++++++++++ src/gateway/services/tenancy/errors.py | 277 ---------------- ...ganization_guardrail_definition_service.py | 24 +- .../tenancy/organization_guardrail_service.py | 28 +- ...ganization_guardrail_definition_service.py | 22 +- .../test_organization_guardrails.py | 29 +- ...organization_guardrail_definition_rules.py | 12 +- 10 files changed, 363 insertions(+), 341 deletions(-) create mode 100644 src/gateway/exceptions/guardrails_exceptions.py diff --git a/src/gateway/api/routes/organization_guardrail_definitions.py b/src/gateway/api/routes/organization_guardrail_definitions.py index 247299116f..63d32d1e46 100644 --- a/src/gateway/api/routes/organization_guardrail_definitions.py +++ b/src/gateway/api/routes/organization_guardrail_definitions.py @@ -4,8 +4,8 @@ `gateway.services.tenancy.organization_guardrail_definition_service`: resolve the caller's identity, call the service, return its typed result. The role gate, the catalog rules and the secret handling live there, and the domain errors it raises -carry their own statuses (see `gateway.services.tenancy.errors`), so nothing here -catches them. +carry their own statuses (see `gateway.exceptions.guardrails_exceptions`), so +nothing here catches them. A separate surface from ``/api/v1/organizations/me/guardrails`` because the two say different things. A definition is *what* a check is, and one of them can be diff --git a/src/gateway/api/routes/organization_guardrails.py b/src/gateway/api/routes/organization_guardrails.py index a2f585559f..222df8b830 100644 --- a/src/gateway/api/routes/organization_guardrails.py +++ b/src/gateway/api/routes/organization_guardrails.py @@ -3,8 +3,8 @@ Thin composition over `gateway.services.tenancy.organization_guardrail_service`: resolve the caller's identity, call the service, return its typed result. The role gate and the scope rules live there, and the domain errors it raises carry -their own statuses (see `gateway.services.tenancy.errors`), so nothing here -catches them. +their own statuses (see `gateway.exceptions.guardrails_exceptions`), so nothing +here catches them. Scoped to ``/me`` for the reason `routes/organization_pricing.py` and `routes/organizations.py` are: a standalone deployment has exactly one diff --git a/src/gateway/api/routes/organization_pricing.py b/src/gateway/api/routes/organization_pricing.py index 6c87b1cefa..4f4dc314dd 100644 --- a/src/gateway/api/routes/organization_pricing.py +++ b/src/gateway/api/routes/organization_pricing.py @@ -4,8 +4,8 @@ the caller's identity, call the service, return its typed result. The overlap rule, the role gate, and the refusal to re-price a model the deployment supplies the credential for all live there, and the domain errors it raises carry their -own statuses (see `gateway.services.tenancy.errors`), so nothing here catches -them. +own statuses (see `gateway.exceptions.pricing_exceptions`), so nothing here +catches them. Scoped to ``/me`` for the same reason `routes/organizations.py` is: a request cannot name an organization at all, because a standalone deployment has exactly diff --git a/src/gateway/exceptions/guardrails_exceptions.py b/src/gateway/exceptions/guardrails_exceptions.py new file mode 100644 index 0000000000..c2f86a08d6 --- /dev/null +++ b/src/gateway/exceptions/guardrails_exceptions.py @@ -0,0 +1,300 @@ +"""Errors an organization's guardrail configuration raises, and the HTTP status each carries.""" + +from fastapi import status + +from gateway.exceptions import TenancyConflictError, TenancyError, TenancyNotFoundError, TenancyValidationError + + +class OrganizationGuardrailNotFoundError(TenancyNotFoundError): + def __init__(self, guardrail_id: object): + super().__init__(f"Organization guardrail {guardrail_id} not found") + + +class OrganizationGuardrailAlreadyExistsError(TenancyConflictError): + """The organization already mandates this guardrail profile. + + One row per profile, not per nickname: the effective guardrail set on the + request path is keyed by profile, so a second row of the same profile could + never run alongside the first. Refused at the write rather than silently + losing at admission. + """ + + def __init__(self, profile: object): + super().__init__(f"This organization already configures the guardrail profile '{profile}'") + + +class OrganizationGuardrailScopeConflictError(TenancyValidationError): + """A workspace list was sent for a guardrail that applies to every workspace. + + The two say different things about the same guardrail and the flag wins at + resolve time, so accepting both would store a list that never decides + anything while reading as though it does. The create path refuses the same + pair in its request model; an update can reach it by setting only one half, + which is why the rule also lives here. + """ + + def __init__(self) -> None: + super().__init__("workspace_ids must be empty when applies_to_all_workspaces is true") + + +class OrganizationGuardrailCredentialNeedsUrlError(TenancyValidationError): + """A credential was stored on an entry that names no endpoint of its own. + + Without a ``url`` the credential rides to whatever the deployment's + ``guardrails_url`` points at, which the shipped compose file makes a + same-host ``http://`` sidecar, so the bearer would cross the wire in clear. + Naming the endpoint is what puts it through ``validate_mcp_url``, which + refuses ``http`` once a credential is in play, so requiring one here is what + makes "a credential is sent over https" true rather than aspirational. + """ + + def __init__(self) -> None: + super().__init__( + "A guardrail credential requires the entry to name its own https url; " + "the deployment's guardrails_url is not necessarily encrypted" + ) + + +class OrganizationGuardrailUnsafeUrlError(TenancyValidationError): + """The endpoint failed the same SSRF and TLS checks a request-body guardrail faces. + + Carries the reason from `services.url_safety.UnsafeURLError` verbatim, for + the reason `WorkspaceMcpServerUnsafeUrlError` does: it names the host and the + range it resolved into, and this surface is management-gated rather than + caller-supplied. + """ + + def __init__(self, reason: str): + super().__init__(reason) + + +class OrganizationGuardrailLimitReachedError(TenancyValidationError): + """The organization already mandates as many guardrails as it may. + + Every mandated guardrail in scope for a workspace is one more sequential + call the guardrails service makes before the provider is reached, on every + request that workspace sends, so the list is latency an organization spends + rather than only rows it stores. + """ + + def __init__(self, limit: int): + super().__init__(f"This organization already configures the maximum of {limit} guardrails") + + +class OrganizationGuardrailSingleBackendError(TenancyValidationError): + """A mandate named both an endpoint of its own and a definition to build. + + Two ways to run one check, and nothing in the row decides between them, so + the pair is refused rather than resolved. ``credential`` counts as naming an + endpoint: it is only ever sent to one, so a credential beside a definition is + the same contradiction one step back. + + ``ck_organization_guardrails_single_backend`` says the url half in the + database too, which is what holds when a write path forgets. It says it as + an ``IntegrityError``, and the write path reports one of those as a profile + collision, so the answer a caller can act on has to come from here. + """ + + def __init__(self) -> None: + super().__init__( + "A guardrail mandate names either its own endpoint or a definition for Otari to " + "build, not both; clear one of them" + ) + + +class OrganizationGuardrailTestsItsDefinitionError(TenancyConflictError): + """A test asked for a mandate that runs one of the organization's definitions. + + That check is the definition's, and its own test runs it; testing it here + as well would be a second way to reach one runner. + """ + + def __init__(self) -> None: + super().__init__("This mandate runs a guardrail you configured; test that guardrail instead") + + +class OrganizationGuardrailNoEndpointError(TenancyConflictError): + """A test asked for a mandate with no endpoint, on a deployment that sets none either.""" + + def __init__(self) -> None: + super().__init__( + "This guardrail names no endpoint and the deployment has no guardrails URL, so there is " + "nothing to test against" + ) + + +class OrganizationGuardrailCheckFailedError(TenancyError): + """The guardrails service was called and the check did not come back. + + A 502 naming nothing the service said, for the reason + `OrganizationGuardrailDefinitionCheckFailedError` gives: the reason is logged. + """ + + status_code = status.HTTP_502_BAD_GATEWAY + + def __init__(self) -> None: + super().__init__("The guardrail could not be evaluated. The reason is in the gateway's log.") + + +class OrganizationGuardrailDefinitionNotFoundError(TenancyNotFoundError): + def __init__(self, definition_id: object): + super().__init__(f"Organization guardrail definition {definition_id} not found") + + +class OrganizationGuardrailDefinitionAlreadyExistsError(TenancyConflictError): + """The organization already has a definition under this name. + + One definition per name, which is what a mandate points at and what an + organization recognizes it by. Unlike the mandate's ``profile``, the name is + the organization's own label, so the same guardrail may be defined twice + under two names with different arguments. + """ + + def __init__(self, name: object): + super().__init__(f"This organization already defines a guardrail named '{name}'") + + +class OrganizationGuardrailDefinitionLimitReachedError(TenancyValidationError): + """The organization already defines as many guardrails as it may. + + A separate ceiling from the mandates', and bounding something else: each + enabled definition becomes a built vendor client held in memory by every + worker, where a mandate is one more sequential call on a request. Neither + number constrains the other, so neither limit stands in for the other. + """ + + def __init__(self, limit: int): + super().__init__(f"This organization already defines the maximum of {limit} guardrails") + + +class OrganizationGuardrailNotBuildableError(TenancyValidationError): + """The named guardrail is not one this gateway can construct and call itself. + + One answer for a name the installed any-guardrail has never heard of and for + one whose guardrail holds local model weights, because a caller can do + nothing different with either. What this gateway builds is a property of the + library it ships with, so the catalog read is where the answer comes from. + """ + + def __init__(self, guardrail_name: object): + super().__init__( + f"'{guardrail_name}' is not a guardrail this deployment can run itself; " + "the built-in guardrail catalog lists the ones it can" + ) + + +class OrganizationGuardrailNotDefinableError(TenancyValidationError): + """The guardrail exists and an organization still may not define it. + + A different refusal from :class:`OrganizationGuardrailNotBuildableError`, + which is about what is possible. This one is about who pays: a guardrail that + takes no credential of its own bills whatever the deployment's environment + holds, outside the budget the request reserved. + """ + + def __init__(self, guardrail_name: object): + super().__init__( + f"'{guardrail_name}' cannot be defined by an organization, because it would run on the " + "deployment's own credentials with nothing metering it" + ) + + +class OrganizationGuardrailDefinitionArgumentsError(TenancyValidationError): + """The submitted build arguments do not make a guardrail this gateway can construct. + + Carries the reason verbatim, the way `OrganizationGuardrailUnsafeUrlError` + does, because every reason names a parameter and this surface is + management-gated rather than caller-supplied. Every rule behind it is read + off the guardrail catalog, so the refusals and the form's fields are one + derivation; a list written here could only drift from the picker it exists to + accept. + """ + + def __init__(self, reason: str): + super().__init__(reason) + + +class OrganizationGuardrailDefinitionUnsafeUrlError(TenancyValidationError): + """A build argument names an address this gateway must not dial. + + Named after the argument it came from, because a guardrail can take several + and the caller has to know which field to fix. The reason is carried + verbatim, as `OrganizationGuardrailDefinitionArgumentsError` explains, and it + is a reason that names the host and the range it resolved into rather than + the URL: an endpoint can carry a credential in its userinfo, and this answer + goes back over the API. + """ + + def __init__(self, argument: str, reason: str): + super().__init__(f"'{argument}' is not an address this gateway may dial: {reason}") + + +class OrganizationGuardrailDefinitionNotRunningError(TenancyConflictError): + """A test asked for a guardrail this worker does not hold built. + + Disabled, failed to build, or not caught up with a write yet: the + definition's `build_state` says which, and that is where the fix is. + """ + + def __init__(self) -> None: + super().__init__( + "This guardrail is not running here, so it cannot be tested. Its status says why; fix that and try again." + ) + + +class OrganizationGuardrailDefinitionCheckFailedError(TenancyError): + """The vendor was called and the check did not come back. + + A 502, because the fault is upstream of this gateway. The message names + neither the vendor's error nor its type: a vendor library may put the + credentials it was handed into its own message, so the reason is logged and + only the log carries it. + """ + + status_code = status.HTTP_502_BAD_GATEWAY + + def __init__(self) -> None: + super().__init__("The guardrail could not be evaluated. The reason is in the gateway's log.") + + +class OrganizationGuardrailDefinitionInUseError(TenancyConflictError): + """A mandate still names the definition the caller asked to drop. + + The link is ``RESTRICT`` so that dropping a definition cannot silently stop + a guardrail running, and without this the database's refusal would reach the + caller as a 500. The profiles are named because clearing them is the + caller's next move and both surfaces belong to the same organization; the + list is read after the delete was refused, so a mandate removed in between + leaves it empty rather than making this a different answer. + """ + + def __init__(self, profiles: list[str]): + mandated = f" by {', '.join(profiles)}" if profiles else "" + super().__init__( + f"This guardrail definition is still mandated{mandated}; stop mandating it first, " + "or set enabled to false to switch it off everywhere at once" + ) + + +__all__ = [ + "OrganizationGuardrailAlreadyExistsError", + "OrganizationGuardrailCheckFailedError", + "OrganizationGuardrailCredentialNeedsUrlError", + "OrganizationGuardrailDefinitionAlreadyExistsError", + "OrganizationGuardrailDefinitionArgumentsError", + "OrganizationGuardrailDefinitionCheckFailedError", + "OrganizationGuardrailDefinitionInUseError", + "OrganizationGuardrailDefinitionLimitReachedError", + "OrganizationGuardrailDefinitionNotFoundError", + "OrganizationGuardrailDefinitionNotRunningError", + "OrganizationGuardrailDefinitionUnsafeUrlError", + "OrganizationGuardrailLimitReachedError", + "OrganizationGuardrailNoEndpointError", + "OrganizationGuardrailNotBuildableError", + "OrganizationGuardrailNotDefinableError", + "OrganizationGuardrailNotFoundError", + "OrganizationGuardrailScopeConflictError", + "OrganizationGuardrailSingleBackendError", + "OrganizationGuardrailTestsItsDefinitionError", + "OrganizationGuardrailUnsafeUrlError", +] diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/services/tenancy/errors.py index a72bc3f869..b7970ddc38 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/services/tenancy/errors.py @@ -713,277 +713,6 @@ def __init__(self) -> None: super().__init__("This workspace has already served a successful request") -class OrganizationGuardrailNotFoundError(TenancyNotFoundError): - def __init__(self, guardrail_id: object): - super().__init__(f"Organization guardrail {guardrail_id} not found") - - -class OrganizationGuardrailAlreadyExistsError(TenancyConflictError): - """The organization already mandates this guardrail profile. - - One row per profile, not per nickname: the effective guardrail set on the - request path is keyed by profile, so a second row of the same profile could - never run alongside the first. Refused at the write rather than silently - losing at admission. - """ - - def __init__(self, profile: object): - super().__init__(f"This organization already configures the guardrail profile '{profile}'") - - -class OrganizationGuardrailScopeConflictError(TenancyValidationError): - """A workspace list was sent for a guardrail that applies to every workspace. - - The two say different things about the same guardrail and the flag wins at - resolve time, so accepting both would store a list that never decides - anything while reading as though it does. The create path refuses the same - pair in its request model; an update can reach it by setting only one half, - which is why the rule also lives here. - """ - - def __init__(self) -> None: - super().__init__("workspace_ids must be empty when applies_to_all_workspaces is true") - - -class OrganizationGuardrailCredentialNeedsUrlError(TenancyValidationError): - """A credential was stored on an entry that names no endpoint of its own. - - Without a ``url`` the credential rides to whatever the deployment's - ``guardrails_url`` points at, which the shipped compose file makes a - same-host ``http://`` sidecar, so the bearer would cross the wire in clear. - Naming the endpoint is what puts it through ``validate_mcp_url``, which - refuses ``http`` once a credential is in play, so requiring one here is what - makes "a credential is sent over https" true rather than aspirational. - """ - - def __init__(self) -> None: - super().__init__( - "A guardrail credential requires the entry to name its own https url; " - "the deployment's guardrails_url is not necessarily encrypted" - ) - - -class OrganizationGuardrailUnsafeUrlError(TenancyValidationError): - """The endpoint failed the same SSRF and TLS checks a request-body guardrail faces. - - Carries the reason from `services.url_safety.UnsafeURLError` verbatim, for - the reason `WorkspaceMcpServerUnsafeUrlError` does: it names the host and the - range it resolved into, and this surface is management-gated rather than - caller-supplied. - """ - - def __init__(self, reason: str): - super().__init__(reason) - - -class OrganizationGuardrailLimitReachedError(TenancyValidationError): - """The organization already mandates as many guardrails as it may. - - Every mandated guardrail in scope for a workspace is one more sequential - call the guardrails service makes before the provider is reached, on every - request that workspace sends, so the list is latency an organization spends - rather than only rows it stores. - """ - - def __init__(self, limit: int): - super().__init__(f"This organization already configures the maximum of {limit} guardrails") - - -class OrganizationGuardrailSingleBackendError(TenancyValidationError): - """A mandate named both an endpoint of its own and a definition to build. - - Two ways to run one check, and nothing in the row decides between them, so - the pair is refused rather than resolved. ``credential`` counts as naming an - endpoint: it is only ever sent to one, so a credential beside a definition is - the same contradiction one step back. - - ``ck_organization_guardrails_single_backend`` says the url half in the - database too, which is what holds when a write path forgets. It says it as - an ``IntegrityError``, and the write path reports one of those as a profile - collision, so the answer a caller can act on has to come from here. - """ - - def __init__(self) -> None: - super().__init__( - "A guardrail mandate names either its own endpoint or a definition for Otari to " - "build, not both; clear one of them" - ) - - -class OrganizationGuardrailTestsItsDefinitionError(TenancyConflictError): - """A test asked for a mandate that runs one of the organization's definitions. - - That check is the definition's, and its own test runs it; testing it here - as well would be a second way to reach one runner. - """ - - def __init__(self) -> None: - super().__init__("This mandate runs a guardrail you configured; test that guardrail instead") - - -class OrganizationGuardrailNoEndpointError(TenancyConflictError): - """A test asked for a mandate with no endpoint, on a deployment that sets none either.""" - - def __init__(self) -> None: - super().__init__( - "This guardrail names no endpoint and the deployment has no guardrails URL, so there is " - "nothing to test against" - ) - - -class OrganizationGuardrailCheckFailedError(TenancyError): - """The guardrails service was called and the check did not come back. - - A 502 naming nothing the service said, for the reason - `OrganizationGuardrailDefinitionCheckFailedError` gives: the reason is logged. - """ - - status_code = status.HTTP_502_BAD_GATEWAY - - def __init__(self) -> None: - super().__init__("The guardrail could not be evaluated. The reason is in the gateway's log.") - - -class OrganizationGuardrailDefinitionNotFoundError(TenancyNotFoundError): - def __init__(self, definition_id: object): - super().__init__(f"Organization guardrail definition {definition_id} not found") - - -class OrganizationGuardrailDefinitionAlreadyExistsError(TenancyConflictError): - """The organization already has a definition under this name. - - One definition per name, which is what a mandate points at and what an - organization recognizes it by. Unlike the mandate's ``profile``, the name is - the organization's own label, so the same guardrail may be defined twice - under two names with different arguments. - """ - - def __init__(self, name: object): - super().__init__(f"This organization already defines a guardrail named '{name}'") - - -class OrganizationGuardrailDefinitionLimitReachedError(TenancyValidationError): - """The organization already defines as many guardrails as it may. - - A separate ceiling from the mandates', and bounding something else: each - enabled definition becomes a built vendor client held in memory by every - worker, where a mandate is one more sequential call on a request. Neither - number constrains the other, so neither limit stands in for the other. - """ - - def __init__(self, limit: int): - super().__init__(f"This organization already defines the maximum of {limit} guardrails") - - -class OrganizationGuardrailNotBuildableError(TenancyValidationError): - """The named guardrail is not one this gateway can construct and call itself. - - One answer for a name the installed any-guardrail has never heard of and for - one whose guardrail holds local model weights, because a caller can do - nothing different with either. What this gateway builds is a property of the - library it ships with, so the catalog read is where the answer comes from. - """ - - def __init__(self, guardrail_name: object): - super().__init__( - f"'{guardrail_name}' is not a guardrail this deployment can run itself; " - "the built-in guardrail catalog lists the ones it can" - ) - - -class OrganizationGuardrailNotDefinableError(TenancyValidationError): - """The guardrail exists and an organization still may not define it. - - A different refusal from :class:`OrganizationGuardrailNotBuildableError`, - which is about what is possible. This one is about who pays: a guardrail that - takes no credential of its own bills whatever the deployment's environment - holds, outside the budget the request reserved. - """ - - def __init__(self, guardrail_name: object): - super().__init__( - f"'{guardrail_name}' cannot be defined by an organization, because it would run on the " - "deployment's own credentials with nothing metering it" - ) - - -class OrganizationGuardrailDefinitionArgumentsError(TenancyValidationError): - """The submitted build arguments do not make a guardrail this gateway can construct. - - Carries the reason verbatim, the way `OrganizationGuardrailUnsafeUrlError` - does, because every reason names a parameter and this surface is - management-gated rather than caller-supplied. Every rule behind it is read - off the guardrail catalog, so the refusals and the form's fields are one - derivation; a list written here could only drift from the picker it exists to - accept. - """ - - def __init__(self, reason: str): - super().__init__(reason) - - -class OrganizationGuardrailDefinitionUnsafeUrlError(TenancyValidationError): - """A build argument names an address this gateway must not dial. - - Named after the argument it came from, because a guardrail can take several - and the caller has to know which field to fix. The reason is carried - verbatim, as `OrganizationGuardrailDefinitionArgumentsError` explains, and it - is a reason that names the host and the range it resolved into rather than - the URL: an endpoint can carry a credential in its userinfo, and this answer - goes back over the API. - """ - - def __init__(self, argument: str, reason: str): - super().__init__(f"'{argument}' is not an address this gateway may dial: {reason}") - - -class OrganizationGuardrailDefinitionNotRunningError(TenancyConflictError): - """A test asked for a guardrail this worker does not hold built. - - Disabled, failed to build, or not caught up with a write yet: the - definition's `build_state` says which, and that is where the fix is. - """ - - def __init__(self) -> None: - super().__init__( - "This guardrail is not running here, so it cannot be tested. Its status says why; fix that and try again." - ) - - -class OrganizationGuardrailDefinitionCheckFailedError(TenancyError): - """The vendor was called and the check did not come back. - - A 502, because the fault is upstream of this gateway. The message names - neither the vendor's error nor its type: a vendor library may put the - credentials it was handed into its own message, so the reason is logged and - only the log carries it. - """ - - status_code = status.HTTP_502_BAD_GATEWAY - - def __init__(self) -> None: - super().__init__("The guardrail could not be evaluated. The reason is in the gateway's log.") - - -class OrganizationGuardrailDefinitionInUseError(TenancyConflictError): - """A mandate still names the definition the caller asked to drop. - - The link is ``RESTRICT`` so that dropping a definition cannot silently stop - a guardrail running, and without this the database's refusal would reach the - caller as a 500. The profiles are named because clearing them is the - caller's next move and both surfaces belong to the same organization; the - list is read after the delete was refused, so a mandate removed in between - leaves it empty rather than making this a different answer. - """ - - def __init__(self, profiles: list[str]): - mandated = f" by {', '.join(profiles)}" if profiles else "" - super().__init__( - f"This guardrail definition is still mandated{mandated}; stop mandating it first, " - "or set enabled to false to switch it off everywhere at once" - ) - - __all__ = [ "BootstrapOperatorProtectedError", "CurrentPasswordIncorrectError", @@ -1008,12 +737,6 @@ def __init__(self, profiles: list[str]): "MembershipUpdateError", "NotAnOrganizationMemberError", "NotAuthorizedError", - "OrganizationGuardrailAlreadyExistsError", - "OrganizationGuardrailCredentialNeedsUrlError", - "OrganizationGuardrailLimitReachedError", - "OrganizationGuardrailNotFoundError", - "OrganizationGuardrailScopeConflictError", - "OrganizationGuardrailUnsafeUrlError", "OrganizationMemberAlreadyExistsError", "OrganizationDomainAlreadyClaimedError", "OrganizationDomainClaimedHereError", diff --git a/src/gateway/services/tenancy/organization_guardrail_definition_service.py b/src/gateway/services/tenancy/organization_guardrail_definition_service.py index ccc0521b64..b09462263b 100644 --- a/src/gateway/services/tenancy/organization_guardrail_definition_service.py +++ b/src/gateway/services/tenancy/organization_guardrail_definition_service.py @@ -60,6 +60,18 @@ from pydantic.json_schema import SkipJsonSchema from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions.guardrails_exceptions import ( + OrganizationGuardrailDefinitionAlreadyExistsError, + OrganizationGuardrailDefinitionArgumentsError, + OrganizationGuardrailDefinitionCheckFailedError, + OrganizationGuardrailDefinitionInUseError, + OrganizationGuardrailDefinitionLimitReachedError, + OrganizationGuardrailDefinitionNotFoundError, + OrganizationGuardrailDefinitionNotRunningError, + OrganizationGuardrailDefinitionUnsafeUrlError, + OrganizationGuardrailNotBuildableError, + OrganizationGuardrailNotDefinableError, +) from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.guardrails import OrganizationGuardrailDefinition @@ -78,18 +90,6 @@ decrypt_secret, encrypt_secret, ) -from gateway.services.tenancy.errors import ( - OrganizationGuardrailDefinitionAlreadyExistsError, - OrganizationGuardrailDefinitionArgumentsError, - OrganizationGuardrailDefinitionCheckFailedError, - OrganizationGuardrailDefinitionInUseError, - OrganizationGuardrailDefinitionLimitReachedError, - OrganizationGuardrailDefinitionNotFoundError, - OrganizationGuardrailDefinitionNotRunningError, - OrganizationGuardrailDefinitionUnsafeUrlError, - OrganizationGuardrailNotBuildableError, - OrganizationGuardrailNotDefinableError, -) from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.url_safety import UnsafeURLError, validate_mcp_url diff --git a/src/gateway/services/tenancy/organization_guardrail_service.py b/src/gateway/services/tenancy/organization_guardrail_service.py index 2bda291fef..ed8833fbb6 100644 --- a/src/gateway/services/tenancy/organization_guardrail_service.py +++ b/src/gateway/services/tenancy/organization_guardrail_service.py @@ -59,6 +59,19 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.guardrails_exceptions import ( + OrganizationGuardrailAlreadyExistsError, + OrganizationGuardrailCheckFailedError, + OrganizationGuardrailCredentialNeedsUrlError, + OrganizationGuardrailDefinitionNotFoundError, + OrganizationGuardrailLimitReachedError, + OrganizationGuardrailNoEndpointError, + OrganizationGuardrailNotFoundError, + OrganizationGuardrailScopeConflictError, + OrganizationGuardrailSingleBackendError, + OrganizationGuardrailTestsItsDefinitionError, + OrganizationGuardrailUnsafeUrlError, +) from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.guardrails import ( @@ -76,20 +89,7 @@ decrypt_secret, encrypt_secret, ) -from gateway.services.tenancy.errors import ( - OrganizationGuardrailAlreadyExistsError, - OrganizationGuardrailCheckFailedError, - OrganizationGuardrailCredentialNeedsUrlError, - OrganizationGuardrailDefinitionNotFoundError, - OrganizationGuardrailLimitReachedError, - OrganizationGuardrailNoEndpointError, - OrganizationGuardrailNotFoundError, - OrganizationGuardrailScopeConflictError, - OrganizationGuardrailSingleBackendError, - OrganizationGuardrailTestsItsDefinitionError, - OrganizationGuardrailUnsafeUrlError, - WorkspaceNotFoundError, -) +from gateway.services.tenancy.errors import WorkspaceNotFoundError from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.url_safety import UnsafeURLError, validate_mcp_url diff --git a/tests/integration/test_organization_guardrail_definition_service.py b/tests/integration/test_organization_guardrail_definition_service.py index de603e3f56..ad7182003c 100644 --- a/tests/integration/test_organization_guardrail_definition_service.py +++ b/tests/integration/test_organization_guardrail_definition_service.py @@ -24,6 +24,16 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions.guardrails_exceptions import ( + OrganizationGuardrailDefinitionAlreadyExistsError, + OrganizationGuardrailDefinitionArgumentsError, + OrganizationGuardrailDefinitionInUseError, + OrganizationGuardrailDefinitionLimitReachedError, + OrganizationGuardrailDefinitionNotFoundError, + OrganizationGuardrailDefinitionUnsafeUrlError, + OrganizationGuardrailNotBuildableError, + OrganizationGuardrailNotDefinableError, +) from gateway.models.guardrails import OrganizationGuardrail, OrganizationGuardrailDefinition from gateway.models.tenancy import Organization, User from gateway.repositories.tenancy import ( @@ -34,17 +44,7 @@ ) from gateway.services.secret_box import decrypt_secret, generate_secret_key from gateway.services.tenancy import organization_guardrail_runner as runner -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - OrganizationGuardrailDefinitionAlreadyExistsError, - OrganizationGuardrailDefinitionArgumentsError, - OrganizationGuardrailDefinitionInUseError, - OrganizationGuardrailDefinitionLimitReachedError, - OrganizationGuardrailDefinitionNotFoundError, - OrganizationGuardrailDefinitionUnsafeUrlError, - OrganizationGuardrailNotBuildableError, - OrganizationGuardrailNotDefinableError, -) +from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.organization_guardrail_definition_service import ( MAX_DEFINITIONS_PER_ORGANIZATION, OrganizationGuardrailDefinitionCreate, diff --git a/tests/integration/test_organization_guardrails.py b/tests/integration/test_organization_guardrails.py index fba78ad7cd..98b84a6962 100644 --- a/tests/integration/test_organization_guardrails.py +++ b/tests/integration/test_organization_guardrails.py @@ -24,6 +24,19 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.guardrails_exceptions import ( + OrganizationGuardrailAlreadyExistsError, + OrganizationGuardrailCheckFailedError, + OrganizationGuardrailCredentialNeedsUrlError, + OrganizationGuardrailDefinitionNotFoundError, + OrganizationGuardrailLimitReachedError, + OrganizationGuardrailNoEndpointError, + OrganizationGuardrailNotFoundError, + OrganizationGuardrailScopeConflictError, + OrganizationGuardrailSingleBackendError, + OrganizationGuardrailTestsItsDefinitionError, + OrganizationGuardrailUnsafeUrlError, +) from gateway.models.guardrails import ( OrganizationGuardrail, OrganizationGuardrailDefinition, @@ -38,21 +51,7 @@ WorkspaceRepository, ) from gateway.services.secret_box import decrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - OrganizationGuardrailAlreadyExistsError, - OrganizationGuardrailCheckFailedError, - OrganizationGuardrailCredentialNeedsUrlError, - OrganizationGuardrailDefinitionNotFoundError, - OrganizationGuardrailLimitReachedError, - OrganizationGuardrailNoEndpointError, - OrganizationGuardrailNotFoundError, - OrganizationGuardrailScopeConflictError, - OrganizationGuardrailSingleBackendError, - OrganizationGuardrailTestsItsDefinitionError, - OrganizationGuardrailUnsafeUrlError, - WorkspaceNotFoundError, -) +from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.organization_guardrail_service import ( MAX_GUARDRAILS_PER_ORGANIZATION, OrganizationGuardrailCreate, diff --git a/tests/unit/test_organization_guardrail_definition_rules.py b/tests/unit/test_organization_guardrail_definition_rules.py index 1526d903ed..cad127ea60 100644 --- a/tests/unit/test_organization_guardrail_definition_rules.py +++ b/tests/unit/test_organization_guardrail_definition_rules.py @@ -18,6 +18,12 @@ import pytest from any_guardrail.parameters import RequirementGroup +from gateway.exceptions.guardrails_exceptions import ( + OrganizationGuardrailDefinitionArgumentsError, + OrganizationGuardrailDefinitionUnsafeUrlError, + OrganizationGuardrailNotBuildableError, + OrganizationGuardrailNotDefinableError, +) from gateway.models.guardrails import OrganizationGuardrailDefinition from gateway.services.guardrail_catalog import ( BuiltInGuardrailSpec, @@ -26,12 +32,6 @@ builtin_guardrail_spec, ) from gateway.services.secret_box import encrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import ( - OrganizationGuardrailDefinitionArgumentsError, - OrganizationGuardrailDefinitionUnsafeUrlError, - OrganizationGuardrailNotBuildableError, - OrganizationGuardrailNotDefinableError, -) from gateway.services.tenancy.organization_guardrail_definition_service import ( OrganizationGuardrailDefinitionUpdate, _arguments_after, From d22905e7e2a3d6f5259278b69c738949b6f84207 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:20:34 +0100 Subject: [PATCH 7/9] refactor(exceptions): move the identity errors into identity_exceptions.py Passwords, passkeys, OAuth, email verification and reset, the profile, and deployment-wide account administration are the identity domain, so their thirty errors leave the tenancy module for their own (#1193). Pure move: the classes, their statuses and their messages are unchanged. One cross-reference in `deployment_user_service` names the new module. --- .../adapters/identity_provider_adapter.py | 8 +- src/gateway/api/routes/auth_oauth.py | 2 +- src/gateway/api/routes/auth_password.py | 2 +- src/gateway/api/routes/auth_session.py | 2 +- src/gateway/api/routes/auth_webauthn.py | 2 +- src/gateway/exceptions/identity_exceptions.py | 461 ++++++++++++++++++ src/gateway/services/oauth_service.py | 6 +- .../tenancy/deployment_user_service.py | 16 +- src/gateway/services/tenancy/email_address.py | 2 +- src/gateway/services/tenancy/errors.py | 446 ----------------- .../services/tenancy/password_policy.py | 2 +- src/gateway/services/tenancy/user_service.py | 22 +- .../services/tenancy/webauthn_service.py | 18 +- .../test_deployment_user_administration.py | 14 +- tests/integration/test_tenancy_races.py | 8 +- tests/unit/test_oauth_service.py | 2 +- tests/unit/test_webauthn_challenge.py | 2 +- 17 files changed, 514 insertions(+), 501 deletions(-) create mode 100644 src/gateway/exceptions/identity_exceptions.py diff --git a/src/gateway/adapters/identity_provider_adapter.py b/src/gateway/adapters/identity_provider_adapter.py index 2a5d407578..999904df4e 100644 --- a/src/gateway/adapters/identity_provider_adapter.py +++ b/src/gateway/adapters/identity_provider_adapter.py @@ -18,14 +18,14 @@ from sqlalchemy.ext.asyncio import AsyncSession -from gateway.models.tenancy import User -from gateway.repositories.tenancy import UserRepository -from gateway.services.tenancy.email_address import validated_email -from gateway.services.tenancy.errors import ( +from gateway.exceptions.identity_exceptions import ( InvalidEmailError, OAuthEmailNotVerifiedError, OAuthIdentityUnknownError, ) +from gateway.models.tenancy import User +from gateway.repositories.tenancy import UserRepository +from gateway.services.tenancy.email_address import validated_email class RosterIdentityProviderAdapter: diff --git a/src/gateway/api/routes/auth_oauth.py b/src/gateway/api/routes/auth_oauth.py index 136af8aca9..f96e88415a 100644 --- a/src/gateway/api/routes/auth_oauth.py +++ b/src/gateway/api/routes/auth_oauth.py @@ -55,6 +55,7 @@ from gateway.api.routes.auth_session import MAINTENANCE_MODE_REFUSAL from gateway.core.config import OAUTH_PROVIDERS, GatewayConfig from gateway.exceptions import TenancyError +from gateway.exceptions.identity_exceptions import OAuthNotConfiguredError from gateway.log_config import logger from gateway.services.dashboard_session_service import ( apply_session_cookie, @@ -72,7 +73,6 @@ provider_label, require_configured, ) -from gateway.services.tenancy.errors import OAuthNotConfiguredError from gateway.services.tenancy.organization_domain_service import OrganizationDomainService router = APIRouter(prefix=OAUTH_ROUTE_PREFIX, tags=["auth"]) diff --git a/src/gateway/api/routes/auth_password.py b/src/gateway/api/routes/auth_password.py index e06810b31a..eeae467d98 100644 --- a/src/gateway/api/routes/auth_password.py +++ b/src/gateway/api/routes/auth_password.py @@ -51,10 +51,10 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.api.deps import CurrentIdentity, get_db, verify_master_key +from gateway.exceptions.identity_exceptions import CurrentPasswordRequiredError, EmailChangeNotSupportedError from gateway.services.dashboard_session_service import SESSION_COOKIE_NAME, hash_session_token from gateway.services.password_service import MIN_PASSWORD_LENGTH from gateway.services.tenancy.email_address import MAX_EMAIL_LENGTH, validated_email -from gateway.services.tenancy.errors import CurrentPasswordRequiredError, EmailChangeNotSupportedError from gateway.services.tenancy.provisioning_service import load_bootstrap_identity from gateway.services.tenancy.user_service import set_password, update_password diff --git a/src/gateway/api/routes/auth_session.py b/src/gateway/api/routes/auth_session.py index 11b4881734..65ac286bcc 100644 --- a/src/gateway/api/routes/auth_session.py +++ b/src/gateway/api/routes/auth_session.py @@ -50,6 +50,7 @@ from gateway.api.deps import get_config, get_db, is_valid_master_key, record_auth_failure from gateway.core.config import GatewayConfig +from gateway.exceptions.identity_exceptions import EmailNotVerifiedError, InvalidCredentialsError from gateway.log_config import logger from gateway.models.tenancy import User as TenancyUser from gateway.rate_limit import RateLimiter @@ -65,7 +66,6 @@ from gateway.services.maintenance_mode_service import is_maintenance_mode from gateway.services.password_service import MAX_PASSWORD_BYTES from gateway.services.tenancy.email_address import MAX_EMAIL_LENGTH -from gateway.services.tenancy.errors import EmailNotVerifiedError, InvalidCredentialsError from gateway.services.tenancy.organization_domain_service import OrganizationDomainService from gateway.services.tenancy.provisioning_service import ensure_bootstrap_identity from gateway.services.tenancy.user_service import authenticate, operator_has_password diff --git a/src/gateway/api/routes/auth_webauthn.py b/src/gateway/api/routes/auth_webauthn.py index aa3279bb22..ed3c07a960 100644 --- a/src/gateway/api/routes/auth_webauthn.py +++ b/src/gateway/api/routes/auth_webauthn.py @@ -45,6 +45,7 @@ from gateway.api.routes.auth_session import MAINTENANCE_MODE_REFUSAL from gateway.core.config import GatewayConfig from gateway.exceptions import TenancyError +from gateway.exceptions.identity_exceptions import PasskeysNotConfiguredError from gateway.log_config import logger from gateway.models.tenancy import ( MAX_WEBAUTHN_CREDENTIAL_NAME, @@ -59,7 +60,6 @@ ) from gateway.services.maintenance_mode_service import is_maintenance_mode from gateway.services.tenancy import webauthn_service -from gateway.services.tenancy.errors import PasskeysNotConfiguredError from gateway.services.tenancy.organization_domain_service import OrganizationDomainService router = APIRouter(prefix="/auth/webauthn", tags=["auth"]) diff --git a/src/gateway/exceptions/identity_exceptions.py b/src/gateway/exceptions/identity_exceptions.py new file mode 100644 index 0000000000..545499cedc --- /dev/null +++ b/src/gateway/exceptions/identity_exceptions.py @@ -0,0 +1,461 @@ +"""Errors the sign-in and account surfaces raise, and the HTTP status each carries.""" + +from fastapi import status + +from gateway.exceptions import ( + TenancyConflictError, + TenancyError, + TenancyForbiddenError, + TenancyNotFoundError, + TenancyValidationError, +) + + +class DeploymentAdministrationUnavailableError(TenancyNotFoundError): + """The caller is not an operator of this deployment, so the surface is not there. + + A 404 and not the 403 the condition really is, because a surface that answers + 403 confirms it exists to anyone who asks, which for a deployment-wide + account list is a thing worth not confirming. A caller who is not an operator + gets the answer an unentitled deployment gets, which is the one that tells + them least. The hosted backends' own admin surface refuses the same way + (mozilla-ai/otari-ai#1842). + """ + + def __init__(self) -> None: + super().__init__("Not found") + + +class DeploymentUserNotFoundError(TenancyNotFoundError): + def __init__(self, user_id: object): + super().__init__(f"User {user_id} not found") + + +class DeploymentUserSelfChangeError(TenancyValidationError): + """An operator aiming one of these flags at their own account. + + Deactivating yourself ends the session you are holding, and clearing your own + superuser flag takes away the page you did it from. Neither is recoverable + from the dashboard, so both are refused rather than confirmed: an operator + who really means to stand down has another operator do it, or the deployment + still has its bootstrap identity. + """ + + +class BootstrapOperatorProtectedError(TenancyValidationError): + """A change aimed at the identity ``tenancy_bootstrap_user_id`` names. + + That identity is what master-key sign-in mints a session for, and + ``resolve_dashboard_session`` refuses a deactivated one, so deactivating it + turns the deployment's fallback credential into a session that dies on + arrival. Its superuser flag is protected for the matching reason: the marker + is the gate that survives a cleared flag, and the two together are what keep + a deployment from being locked out of its own administration. + """ + + +class EmptyDeploymentUserUpdateError(TenancyValidationError): + """A change request that names neither flag. + + Refused rather than answered as a no-op: the two fields are optional so that + deactivating an account and changing what it may administer stay separate + decisions, and a body that set neither is a decision that got lost on the way. + """ + + def __init__(self) -> None: + super().__init__("Set is_active or is_superuser; a change naming neither does nothing") + + +class InvalidCredentialsError(TenancyError): + """A password sign-in that did not succeed, without saying which part failed. + + 401 rather than 403: nothing is known about the caller, so the answer is + "authenticate", not "you may not". Declared on this class rather than on a + shared 401 base, because it is the only 401 the tenancy family raises today + and a base class with one subclass is an abstraction with no second user. + + **A deliberate departure from the platform's port.** ``user_service`` there + distinguishes ``UserNotFoundError``, ``OAuthAccountPasswordLoginError``, + ``IncorrectPasswordError`` and ``EmailNotVerifiedError`` at the sign-in + endpoint. Each of those answers "does this address hold an account here, and + how does it sign in", which is a question an unauthenticated caller may ask + an unlimited number of times. A self-hosted deployment's roster is small + enough that enumerating it is worth doing, so the four collapse into one + message here, and `gateway.services.password_service` makes them cost the + same wall-clock time as well. + + The distinctions survive where a caller has already authenticated: + ``CurrentPasswordIncorrectError`` and ``PasswordNotSetError`` below say + exactly what went wrong, because by then the caller is not being told + anything about somebody else's account. + """ + + status_code = status.HTTP_401_UNAUTHORIZED + + def __init__(self) -> None: + super().__init__("Incorrect email or password") + + +class PasskeysNotConfiguredError(TenancyValidationError): + """This deployment has no relying-party ID, so it cannot run a ceremony. + + 503 rather than the 400 its base carries: nothing is wrong with the request, + the deployment is not set up to answer it, and that is the same shape + `api.routes.mail` gives an unconfigured mailer. The message names the + setting, because an operator who reached this endpoint meant to offer + passkeys and needs to know which line is missing rather than that something + was refused. + """ + + status_code = status.HTTP_503_SERVICE_UNAVAILABLE + + def __init__(self) -> None: + super().__init__( + "Passkeys are unavailable on this deployment: it does not know its own address. " + "Set public_base_url (or webauthn_rp_id) and restart." + ) + + +class PasskeyNotFoundError(TenancyNotFoundError): + def __init__(self, credential_id: object): + super().__init__(f"Passkey {credential_id} not found") + + +class PasskeyNameTakenError(TenancyConflictError): + def __init__(self, name: str): + super().__init__(f"You already have a passkey named '{name}'") + + +class PasskeyAlreadyRegisteredError(TenancyConflictError): + """This authenticator already has a row, possibly on another identity. + + Said plainly rather than hidden, and the wording does not reveal *whose*. + A caller performing this ceremony is signed in and holds the authenticator + that just answered, so telling them it is already known here costs nothing; + telling them which identity holds it would be somebody else's business. + """ + + def __init__(self) -> None: + super().__init__("That passkey is already registered on this deployment") + + +class PasskeyLimitReachedError(TenancyValidationError): + """This identity already holds as many passkeys as it may. + + A ceiling on a table an authenticated caller writes in a loop they control, + not a policy about how many devices a person should have; see + ``MAX_PASSKEYS_PER_IDENTITY``. The message says the number, because the only + useful action is to delete one and the caller cannot count what they cannot + see. + """ + + def __init__(self, limit: int): + super().__init__( + f"You already have {limit} passkeys, which is the most one identity may hold. Delete one first." + ) + + +class PasskeyCeremonyError(TenancyValidationError): + """A registration or authentication ceremony did not verify. + + One error for every way the ceremony can fail (an unknown or expired + challenge, a mismatched origin or relying-party ID, a signature that does + not check out, an authenticator answering somebody else's challenge), + carrying the library's reason in the log and a fixed sentence to the caller. + + Undifferentiated on purpose, for the reason ``InvalidCredentialsError`` + gives: the sign-in half of this is reachable unauthenticated, and each + distinct refusal would answer a question about which credentials this + deployment holds. The registration half is authenticated and could afford + to say more, but a caller there cannot act on the distinction either: every + branch means "try the ceremony again". + """ + + def __init__(self) -> None: + super().__init__("That passkey could not be verified. Try again.") + + +class PasskeySignInFailedError(TenancyError): + """A passkey sign-in that did not succeed, without saying which part failed. + + 401 for the reason ``InvalidCredentialsError`` is: nothing is known about + the caller. Separate from that class only because the message names the + credential the caller actually used, and being told to check an email and + password after tapping a passkey is a dead end. + """ + + status_code = status.HTTP_401_UNAUTHORIZED + + def __init__(self) -> None: + super().__init__("That passkey did not sign you in") + + +class OAuthNotConfiguredError(TenancyValidationError): + """This deployment configures no client credentials for the named provider. + + 503 rather than the 400 its base carries, for the reason + ``PasskeysNotConfiguredError`` gives: nothing is wrong with the request, the + deployment is not set up to answer it. The message names the settings, + because the only caller who reaches this meant to offer that provider and + needs to know which two lines are missing. + + Reachable at all only because the dashboard hides a provider this deployment + does not configure: the affordance is absent rather than disabled + (``GET /v1/bootstrap``'s ``oauth_providers``), so this answers a bookmark or + a hand-made request rather than a button somebody was offered. + """ + + status_code = status.HTTP_503_SERVICE_UNAVAILABLE + + def __init__(self, provider: str) -> None: + super().__init__( + f"{provider} sign-in is not configured on this deployment. " + f"Set oauth_{provider}_client_id and oauth_{provider}_client_secret, and public_base_url, then restart." + ) + + +class OAuthExchangeError(TenancyValidationError): + """The authorization code did not exchange for an identity. + + One error for every way the exchange can fail (a code already spent, a code + that expired, a redirect URI the provider does not recognize, the provider + being unreachable), carrying a fixed sentence to the caller. + + The provider's own reason is deliberately not interpolated into the message. + apron-auth's exchange errors carry the provider's RFC 6749 ``error`` and + ``error_description`` verbatim, and this message reaches both the HTTP + client and the log aggregator (CWE-532). Chaining keeps that payload on the + traceback, which is where debugging needs it and where the error-detail + boundary in ``gateway.main`` leaves it. + """ + + def __init__(self, provider: str) -> None: + super().__init__(f"{provider} did not complete the sign-in. Try again.") + + +class OAuthStateError(TenancyValidationError): + """The callback did not carry a ``state`` this deployment is still waiting on. + + Covers every way that can be true (never minted here, already spent, + expired, or minted for a different provider), because they are one answer to + the caller: start the sign-in again. Which of them it was stays in the log. + + Deliberately indistinguishable from the outside. Telling an attacker that a + state existed but had expired, as against never existing at all, tells them + their guess was in the right shape. + """ + + def __init__(self) -> None: + super().__init__("That sign-in did not start here, or it expired. Start again.") + + +class OAuthEmailNotVerifiedError(TenancyError): + """The provider returned an address it will not vouch for. + + 401, and named rather than collapsed into the refusal below, for the reason + ``InvalidCredentialsError``'s own docstring carves out: the distinctions + survive where a caller has already authenticated. By the time this is + raised the provider has confirmed the person at the browser holds that + account, so naming the reason tells them about themselves and nobody else, + and it is the one refusal here they can act on without an operator. + + An address the provider simply did not speak to (apron-auth reports + ``email_verified`` as tri-state, and silence is not an assertion) lands + here too. That is the point: an unasserted address is treated as + unverified rather than laundered into a verified identity. + """ + + status_code = status.HTTP_401_UNAUTHORIZED + + def __init__(self, provider: str) -> None: + super().__init__( + f"{provider} did not confirm that address is yours, so it cannot sign you in here. " + f"Verify your email address with {provider} and try again." + ) + + +class OAuthIdentityUnknownError(TenancyError): + """No account on this deployment signs in as that external identity. + + 401, and the message says what to do, because the caller has already proven + to the provider that they hold the address: the enumeration risk + ``InvalidCredentialsError`` exists to close is a caller asking about + *other people's* addresses, and nobody can complete a consent screen for an + address they do not control. + + A deactivated identity is refused here rather than in an error of its own. + Deactivating somebody has to close every road in, and "your account is + switched off" and "there is no account" are the same instruction to the same + person: talk to whoever administers this gateway. Saying which would let + somebody an operator has already shut out keep confirming their account is + still on file. + + This is the base build's roster policy speaking, not a property of OAuth. + Otari's signup claims an identity an operator already added and never + creates one from nothing (``user_service.create_user_for_signup``), and + social sign-in does not get to be the exception that lets any holder of a + Google account in. An overlay that provisions on first sight binds its own + ``IdentityProviderPort`` adapter and never raises this. + """ + + status_code = status.HTTP_401_UNAUTHORIZED + + def __init__(self, provider: str) -> None: + super().__init__( + f"That {provider} account is not registered on this gateway. " + "Ask whoever administers it to add your email address, then sign in again." + ) + + +class CurrentPasswordIncorrectError(TenancyValidationError): + """The current password given with a password change does not match. + + 400 and not 401, as the platform's own note says: a 401 on this route reads + to a browser client as "your session died", and it would sign the caller out + of a form they filled in correctly except for one field. + """ + + def __init__(self) -> None: + super().__init__("Current password is incorrect") + + +class PasswordNotSetError(TenancyValidationError): + """A password change on an identity that has no password to change.""" + + def __init__(self) -> None: + super().__init__("This identity has no password set; set one instead of changing it") + + +class UnmodifiedPasswordError(TenancyValidationError): + """The new password is the one already stored.""" + + def __init__(self) -> None: + super().__init__("The new password cannot be the same as the current one") + + +class CurrentPasswordRequiredError(TenancyValidationError): + """A password change from a session, with no current password supplied.""" + + def __init__(self) -> None: + super().__init__("The current password is required to change it") + + +class EmailChangeNotSupportedError(TenancyValidationError): + """An address change attempted through the password endpoint. + + Supplying an address is part of claiming an identity that has none. Changing + one that already exists is a different operation with its own requirements + (it invalidates a sign-in handle, and the new address has to be verified), + and it belongs to the verification flow rather than being smuggled in + alongside a password. + """ + + def __init__(self) -> None: + super().__init__("This identity already has an email address; changing it is not supported yet") + + +class PasswordPolicyError(TenancyValidationError): + """A password that is too short, or longer than bcrypt will hash.""" + + +class SignInAddressRequiredError(TenancyValidationError): + """Setting a password on an identity that has no address to sign in with. + + The operator identity first boot provisions is a label rather than a sign-in + address (`gateway.services.tenancy.provisioning_service`), so claiming it + supplies one. Refused rather than defaulted: a synthesized address would be + a credential handle nobody knows. + """ + + def __init__(self) -> None: + super().__init__("This identity has no email address; supply one to sign in with a password") + + +class EmailAlreadyInUseError(TenancyConflictError): + """Another identity already holds the address being claimed.""" + + def __init__(self, email: str) -> None: + super().__init__(f"'{email}' already belongs to another identity") + + +class InvalidEmailError(TenancyValidationError): + """An address that could not be a claim handle. + + Deliberately a shape check and nothing more. The address is not delivered to + by this edition, and ownership of it is proven by the claim flow that + arrives with sign-in, so anything stricter here would be theater. + """ + + def __init__(self, email: str): + super().__init__(f"'{email}' is not a valid email address") + + +class VerificationTokenInvalidError(TenancyValidationError): + """A verification token that is unknown, expired, or already consumed. + + One message for all three, the same reasoning ``InvitationNotFoundError`` + gives an unknown-or-foreign invitation: distinguishing "expired" from + "already used" from "never existed" would let a caller narrow down which + is true of a token they do not hold. + """ + + def __init__(self) -> None: + super().__init__("This verification link is invalid, expired, or already used") + + +class ResetTokenInvalidError(TenancyValidationError): + """A password-reset token that is unknown, expired, or already consumed. + + Same collapse as ``VerificationTokenInvalidError``, for the same reason. + """ + + def __init__(self) -> None: + super().__init__("This password reset link is invalid, expired, or already used") + + +class EmailNotVerifiedError(TenancyForbiddenError): + """A password sign-in on an identity that has not verified its address. + + Raised only after the password itself has already checked out, which is + why it is allowed to say what is actually wrong: the distinction the + module docstring on ``authenticate`` promises survives once a caller has + proven something, the same way ``CurrentPasswordIncorrectError`` and + ``PasswordNotSetError`` do. + """ + + def __init__(self) -> None: + super().__init__("Verify your email before signing in; request a new verification email if yours expired") + + +__all__ = [ + "BootstrapOperatorProtectedError", + "CurrentPasswordIncorrectError", + "CurrentPasswordRequiredError", + "DeploymentAdministrationUnavailableError", + "DeploymentUserNotFoundError", + "DeploymentUserSelfChangeError", + "EmailAlreadyInUseError", + "EmailChangeNotSupportedError", + "EmailNotVerifiedError", + "EmptyDeploymentUserUpdateError", + "InvalidCredentialsError", + "InvalidEmailError", + "OAuthEmailNotVerifiedError", + "OAuthExchangeError", + "OAuthIdentityUnknownError", + "OAuthNotConfiguredError", + "OAuthStateError", + "PasskeyAlreadyRegisteredError", + "PasskeyCeremonyError", + "PasskeyLimitReachedError", + "PasskeyNameTakenError", + "PasskeyNotFoundError", + "PasskeySignInFailedError", + "PasskeysNotConfiguredError", + "PasswordNotSetError", + "PasswordPolicyError", + "ResetTokenInvalidError", + "SignInAddressRequiredError", + "UnmodifiedPasswordError", + "VerificationTokenInvalidError", +] diff --git a/src/gateway/services/oauth_service.py b/src/gateway/services/oauth_service.py index 41481c48c3..5e36aaafc1 100644 --- a/src/gateway/services/oauth_service.py +++ b/src/gateway/services/oauth_service.py @@ -59,13 +59,9 @@ from sqlmodel import col from gateway.core.config import API_ROOT, OAUTH_PROVIDERS, GatewayConfig +from gateway.exceptions.identity_exceptions import OAuthExchangeError, OAuthNotConfiguredError, OAuthStateError from gateway.log_config import logger from gateway.models.tenancy import OAUTH_STATE_TTL_SECONDS, OAuthPendingState -from gateway.services.tenancy.errors import ( - OAuthExchangeError, - OAuthNotConfiguredError, - OAuthStateError, -) # Which scopes each provider is asked for, and what it calls itself. # diff --git a/src/gateway/services/tenancy/deployment_user_service.py b/src/gateway/services/tenancy/deployment_user_service.py index ed2f2240eb..5e05232981 100644 --- a/src/gateway/services/tenancy/deployment_user_service.py +++ b/src/gateway/services/tenancy/deployment_user_service.py @@ -21,7 +21,7 @@ operator whose flag was cleared, by hand or by another operator, and it is the reason this surface cannot lock a deployment out of itself. Everyone else is refused with 404 rather than 403; see -:class:`~gateway.services.tenancy.errors.DeploymentAdministrationUnavailableError` +:class:`~gateway.exceptions.identity_exceptions.DeploymentAdministrationUnavailableError` for why the status is the one it is. **What it refuses.** Two lockout guards, both narrow and both directional, so a @@ -45,6 +45,13 @@ from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.identity_exceptions import ( + BootstrapOperatorProtectedError, + DeploymentAdministrationUnavailableError, + DeploymentUserNotFoundError, + DeploymentUserSelfChangeError, + EmptyDeploymentUserUpdateError, +) from gateway.models.tenancy import ( DeploymentUserOrganizationPublic, DeploymentUserPublic, @@ -56,13 +63,6 @@ ) from gateway.repositories.tenancy import OrganizationMemberRepository, UserRepository from gateway.services.dashboard_session_service import revoke_user_dashboard_sessions -from gateway.services.tenancy.errors import ( - BootstrapOperatorProtectedError, - DeploymentAdministrationUnavailableError, - DeploymentUserNotFoundError, - DeploymentUserSelfChangeError, - EmptyDeploymentUserUpdateError, -) from gateway.services.tenancy.provisioning_service import load_bootstrap_identity diff --git a/src/gateway/services/tenancy/email_address.py b/src/gateway/services/tenancy/email_address.py index 2606f34313..9f9bb6fe54 100644 --- a/src/gateway/services/tenancy/email_address.py +++ b/src/gateway/services/tenancy/email_address.py @@ -11,7 +11,7 @@ """ from gateway.core.addresses import normalized_address -from gateway.services.tenancy.errors import InvalidEmailError +from gateway.exceptions.identity_exceptions import InvalidEmailError # ``user.email`` is ``varchar(255)``, so this is the column's width and not a # policy. Request schemas carry it too, but theirs bounds the *raw* value and diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/services/tenancy/errors.py index b7970ddc38..a44067564f 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/services/tenancy/errors.py @@ -55,61 +55,6 @@ def __init__(self, message: str = "Not enough privileges to perform this action" super().__init__(message) -class DeploymentAdministrationUnavailableError(TenancyNotFoundError): - """The caller is not an operator of this deployment, so the surface is not there. - - A 404 and not the 403 the condition really is, because a surface that answers - 403 confirms it exists to anyone who asks, which for a deployment-wide - account list is a thing worth not confirming. A caller who is not an operator - gets the answer an unentitled deployment gets, which is the one that tells - them least. The hosted backends' own admin surface refuses the same way - (mozilla-ai/otari-ai#1842). - """ - - def __init__(self) -> None: - super().__init__("Not found") - - -class DeploymentUserNotFoundError(TenancyNotFoundError): - def __init__(self, user_id: object): - super().__init__(f"User {user_id} not found") - - -class DeploymentUserSelfChangeError(TenancyValidationError): - """An operator aiming one of these flags at their own account. - - Deactivating yourself ends the session you are holding, and clearing your own - superuser flag takes away the page you did it from. Neither is recoverable - from the dashboard, so both are refused rather than confirmed: an operator - who really means to stand down has another operator do it, or the deployment - still has its bootstrap identity. - """ - - -class BootstrapOperatorProtectedError(TenancyValidationError): - """A change aimed at the identity ``tenancy_bootstrap_user_id`` names. - - That identity is what master-key sign-in mints a session for, and - ``resolve_dashboard_session`` refuses a deactivated one, so deactivating it - turns the deployment's fallback credential into a session that dies on - arrival. Its superuser flag is protected for the matching reason: the marker - is the gate that survives a cleared flag, and the two together are what keep - a deployment from being locked out of its own administration. - """ - - -class EmptyDeploymentUserUpdateError(TenancyValidationError): - """A change request that names neither flag. - - Refused rather than answered as a no-op: the two fields are optional so that - deactivating an account and changing what it may administer stay separate - decisions, and a body that set neither is a decision that got lost on the way. - """ - - def __init__(self) -> None: - super().__init__("Set is_active or is_superuser; a change naming neither does nothing") - - class MembershipUpdateError(TenancyValidationError): """A membership change the organization's own rules refuse (e.g. the last owner).""" @@ -215,330 +160,6 @@ def __init__(self, identifier: object): super().__init__(f"{identifier} already has a pending invitation") -class InvalidCredentialsError(TenancyError): - """A password sign-in that did not succeed, without saying which part failed. - - 401 rather than 403: nothing is known about the caller, so the answer is - "authenticate", not "you may not". Declared on this class rather than on a - shared 401 base, because it is the only 401 the tenancy family raises today - and a base class with one subclass is an abstraction with no second user. - - **A deliberate departure from the platform's port.** ``user_service`` there - distinguishes ``UserNotFoundError``, ``OAuthAccountPasswordLoginError``, - ``IncorrectPasswordError`` and ``EmailNotVerifiedError`` at the sign-in - endpoint. Each of those answers "does this address hold an account here, and - how does it sign in", which is a question an unauthenticated caller may ask - an unlimited number of times. A self-hosted deployment's roster is small - enough that enumerating it is worth doing, so the four collapse into one - message here, and `gateway.services.password_service` makes them cost the - same wall-clock time as well. - - The distinctions survive where a caller has already authenticated: - ``CurrentPasswordIncorrectError`` and ``PasswordNotSetError`` below say - exactly what went wrong, because by then the caller is not being told - anything about somebody else's account. - """ - - status_code = status.HTTP_401_UNAUTHORIZED - - def __init__(self) -> None: - super().__init__("Incorrect email or password") - - -class PasskeysNotConfiguredError(TenancyValidationError): - """This deployment has no relying-party ID, so it cannot run a ceremony. - - 503 rather than the 400 its base carries: nothing is wrong with the request, - the deployment is not set up to answer it, and that is the same shape - `api.routes.mail` gives an unconfigured mailer. The message names the - setting, because an operator who reached this endpoint meant to offer - passkeys and needs to know which line is missing rather than that something - was refused. - """ - - status_code = status.HTTP_503_SERVICE_UNAVAILABLE - - def __init__(self) -> None: - super().__init__( - "Passkeys are unavailable on this deployment: it does not know its own address. " - "Set public_base_url (or webauthn_rp_id) and restart." - ) - - -class PasskeyNotFoundError(TenancyNotFoundError): - def __init__(self, credential_id: object): - super().__init__(f"Passkey {credential_id} not found") - - -class PasskeyNameTakenError(TenancyConflictError): - def __init__(self, name: str): - super().__init__(f"You already have a passkey named '{name}'") - - -class PasskeyAlreadyRegisteredError(TenancyConflictError): - """This authenticator already has a row, possibly on another identity. - - Said plainly rather than hidden, and the wording does not reveal *whose*. - A caller performing this ceremony is signed in and holds the authenticator - that just answered, so telling them it is already known here costs nothing; - telling them which identity holds it would be somebody else's business. - """ - - def __init__(self) -> None: - super().__init__("That passkey is already registered on this deployment") - - -class PasskeyLimitReachedError(TenancyValidationError): - """This identity already holds as many passkeys as it may. - - A ceiling on a table an authenticated caller writes in a loop they control, - not a policy about how many devices a person should have; see - ``MAX_PASSKEYS_PER_IDENTITY``. The message says the number, because the only - useful action is to delete one and the caller cannot count what they cannot - see. - """ - - def __init__(self, limit: int): - super().__init__( - f"You already have {limit} passkeys, which is the most one identity may hold. Delete one first." - ) - - -class PasskeyCeremonyError(TenancyValidationError): - """A registration or authentication ceremony did not verify. - - One error for every way the ceremony can fail (an unknown or expired - challenge, a mismatched origin or relying-party ID, a signature that does - not check out, an authenticator answering somebody else's challenge), - carrying the library's reason in the log and a fixed sentence to the caller. - - Undifferentiated on purpose, for the reason ``InvalidCredentialsError`` - gives: the sign-in half of this is reachable unauthenticated, and each - distinct refusal would answer a question about which credentials this - deployment holds. The registration half is authenticated and could afford - to say more, but a caller there cannot act on the distinction either: every - branch means "try the ceremony again". - """ - - def __init__(self) -> None: - super().__init__("That passkey could not be verified. Try again.") - - -class PasskeySignInFailedError(TenancyError): - """A passkey sign-in that did not succeed, without saying which part failed. - - 401 for the reason ``InvalidCredentialsError`` is: nothing is known about - the caller. Separate from that class only because the message names the - credential the caller actually used, and being told to check an email and - password after tapping a passkey is a dead end. - """ - - status_code = status.HTTP_401_UNAUTHORIZED - - def __init__(self) -> None: - super().__init__("That passkey did not sign you in") - - -class OAuthNotConfiguredError(TenancyValidationError): - """This deployment configures no client credentials for the named provider. - - 503 rather than the 400 its base carries, for the reason - ``PasskeysNotConfiguredError`` gives: nothing is wrong with the request, the - deployment is not set up to answer it. The message names the settings, - because the only caller who reaches this meant to offer that provider and - needs to know which two lines are missing. - - Reachable at all only because the dashboard hides a provider this deployment - does not configure: the affordance is absent rather than disabled - (``GET /v1/bootstrap``'s ``oauth_providers``), so this answers a bookmark or - a hand-made request rather than a button somebody was offered. - """ - - status_code = status.HTTP_503_SERVICE_UNAVAILABLE - - def __init__(self, provider: str) -> None: - super().__init__( - f"{provider} sign-in is not configured on this deployment. " - f"Set oauth_{provider}_client_id and oauth_{provider}_client_secret, and public_base_url, then restart." - ) - - -class OAuthExchangeError(TenancyValidationError): - """The authorization code did not exchange for an identity. - - One error for every way the exchange can fail (a code already spent, a code - that expired, a redirect URI the provider does not recognize, the provider - being unreachable), carrying a fixed sentence to the caller. - - The provider's own reason is deliberately not interpolated into the message. - apron-auth's exchange errors carry the provider's RFC 6749 ``error`` and - ``error_description`` verbatim, and this message reaches both the HTTP - client and the log aggregator (CWE-532). Chaining keeps that payload on the - traceback, which is where debugging needs it and where the error-detail - boundary in ``gateway.main`` leaves it. - """ - - def __init__(self, provider: str) -> None: - super().__init__(f"{provider} did not complete the sign-in. Try again.") - - -class OAuthStateError(TenancyValidationError): - """The callback did not carry a ``state`` this deployment is still waiting on. - - Covers every way that can be true (never minted here, already spent, - expired, or minted for a different provider), because they are one answer to - the caller: start the sign-in again. Which of them it was stays in the log. - - Deliberately indistinguishable from the outside. Telling an attacker that a - state existed but had expired, as against never existing at all, tells them - their guess was in the right shape. - """ - - def __init__(self) -> None: - super().__init__("That sign-in did not start here, or it expired. Start again.") - - -class OAuthEmailNotVerifiedError(TenancyError): - """The provider returned an address it will not vouch for. - - 401, and named rather than collapsed into the refusal below, for the reason - ``InvalidCredentialsError``'s own docstring carves out: the distinctions - survive where a caller has already authenticated. By the time this is - raised the provider has confirmed the person at the browser holds that - account, so naming the reason tells them about themselves and nobody else, - and it is the one refusal here they can act on without an operator. - - An address the provider simply did not speak to (apron-auth reports - ``email_verified`` as tri-state, and silence is not an assertion) lands - here too. That is the point: an unasserted address is treated as - unverified rather than laundered into a verified identity. - """ - - status_code = status.HTTP_401_UNAUTHORIZED - - def __init__(self, provider: str) -> None: - super().__init__( - f"{provider} did not confirm that address is yours, so it cannot sign you in here. " - f"Verify your email address with {provider} and try again." - ) - - -class OAuthIdentityUnknownError(TenancyError): - """No account on this deployment signs in as that external identity. - - 401, and the message says what to do, because the caller has already proven - to the provider that they hold the address: the enumeration risk - ``InvalidCredentialsError`` exists to close is a caller asking about - *other people's* addresses, and nobody can complete a consent screen for an - address they do not control. - - A deactivated identity is refused here rather than in an error of its own. - Deactivating somebody has to close every road in, and "your account is - switched off" and "there is no account" are the same instruction to the same - person: talk to whoever administers this gateway. Saying which would let - somebody an operator has already shut out keep confirming their account is - still on file. - - This is the base build's roster policy speaking, not a property of OAuth. - Otari's signup claims an identity an operator already added and never - creates one from nothing (``user_service.create_user_for_signup``), and - social sign-in does not get to be the exception that lets any holder of a - Google account in. An overlay that provisions on first sight binds its own - ``IdentityProviderPort`` adapter and never raises this. - """ - - status_code = status.HTTP_401_UNAUTHORIZED - - def __init__(self, provider: str) -> None: - super().__init__( - f"That {provider} account is not registered on this gateway. " - "Ask whoever administers it to add your email address, then sign in again." - ) - - -class CurrentPasswordIncorrectError(TenancyValidationError): - """The current password given with a password change does not match. - - 400 and not 401, as the platform's own note says: a 401 on this route reads - to a browser client as "your session died", and it would sign the caller out - of a form they filled in correctly except for one field. - """ - - def __init__(self) -> None: - super().__init__("Current password is incorrect") - - -class PasswordNotSetError(TenancyValidationError): - """A password change on an identity that has no password to change.""" - - def __init__(self) -> None: - super().__init__("This identity has no password set; set one instead of changing it") - - -class UnmodifiedPasswordError(TenancyValidationError): - """The new password is the one already stored.""" - - def __init__(self) -> None: - super().__init__("The new password cannot be the same as the current one") - - -class CurrentPasswordRequiredError(TenancyValidationError): - """A password change from a session, with no current password supplied.""" - - def __init__(self) -> None: - super().__init__("The current password is required to change it") - - -class EmailChangeNotSupportedError(TenancyValidationError): - """An address change attempted through the password endpoint. - - Supplying an address is part of claiming an identity that has none. Changing - one that already exists is a different operation with its own requirements - (it invalidates a sign-in handle, and the new address has to be verified), - and it belongs to the verification flow rather than being smuggled in - alongside a password. - """ - - def __init__(self) -> None: - super().__init__("This identity already has an email address; changing it is not supported yet") - - -class PasswordPolicyError(TenancyValidationError): - """A password that is too short, or longer than bcrypt will hash.""" - - -class SignInAddressRequiredError(TenancyValidationError): - """Setting a password on an identity that has no address to sign in with. - - The operator identity first boot provisions is a label rather than a sign-in - address (`gateway.services.tenancy.provisioning_service`), so claiming it - supplies one. Refused rather than defaulted: a synthesized address would be - a credential handle nobody knows. - """ - - def __init__(self) -> None: - super().__init__("This identity has no email address; supply one to sign in with a password") - - -class EmailAlreadyInUseError(TenancyConflictError): - """Another identity already holds the address being claimed.""" - - def __init__(self, email: str) -> None: - super().__init__(f"'{email}' already belongs to another identity") - - -class InvalidEmailError(TenancyValidationError): - """An address that could not be a claim handle. - - Deliberately a shape check and nothing more. The address is not delivered to - by this edition, and ownership of it is proven by the claim flow that - arrives with sign-in, so anything stricter here would be theater. - """ - - def __init__(self, email: str): - super().__init__(f"'{email}' is not a valid email address") - - class WorkspaceNameRequiredError(TenancyValidationError): """A workspace name that is absent, null, or blank once trimmed. @@ -655,43 +276,6 @@ def __init__(self) -> None: super().__init__("This address can already sign in; accept without a password and sign in as usual") -class VerificationTokenInvalidError(TenancyValidationError): - """A verification token that is unknown, expired, or already consumed. - - One message for all three, the same reasoning ``InvitationNotFoundError`` - gives an unknown-or-foreign invitation: distinguishing "expired" from - "already used" from "never existed" would let a caller narrow down which - is true of a token they do not hold. - """ - - def __init__(self) -> None: - super().__init__("This verification link is invalid, expired, or already used") - - -class ResetTokenInvalidError(TenancyValidationError): - """A password-reset token that is unknown, expired, or already consumed. - - Same collapse as ``VerificationTokenInvalidError``, for the same reason. - """ - - def __init__(self) -> None: - super().__init__("This password reset link is invalid, expired, or already used") - - -class EmailNotVerifiedError(TenancyForbiddenError): - """A password sign-in on an identity that has not verified its address. - - Raised only after the password itself has already checked out, which is - why it is allowed to say what is actually wrong: the distinction the - module docstring on ``authenticate`` promises survives once a caller has - proven something, the same way ``CurrentPasswordIncorrectError`` and - ``PasswordNotSetError`` do. - """ - - def __init__(self) -> None: - super().__init__("Verify your email before signing in; request a new verification email if yours expired") - - class WorkspaceActivationUnavailableError(TenancyConflictError): """The first-request setup guide is not on offer for this workspace. @@ -714,19 +298,7 @@ def __init__(self) -> None: __all__ = [ - "BootstrapOperatorProtectedError", - "CurrentPasswordIncorrectError", - "CurrentPasswordRequiredError", - "DeploymentAdministrationUnavailableError", - "DeploymentUserNotFoundError", - "DeploymentUserSelfChangeError", - "EmailAlreadyInUseError", - "EmailChangeNotSupportedError", - "EmailNotVerifiedError", - "EmptyDeploymentUserUpdateError", "ForeignTenancyError", - "InvalidCredentialsError", - "InvalidEmailError", "InvalidRoleError", "InvitationAlreadyPendingError", "InvitationAlreadyUsedError", @@ -744,34 +316,16 @@ def __init__(self) -> None: "OrganizationDomainNotVerifiedError", "OrganizationMemberNotFoundError", "OrganizationNameRequiredError", - "OAuthEmailNotVerifiedError", - "OAuthExchangeError", - "OAuthIdentityUnknownError", - "OAuthNotConfiguredError", - "OAuthStateError", "OrganizationNotFoundError", "OrganizationSlugUnavailableError", - "PasskeyAlreadyRegisteredError", - "PasskeyCeremonyError", - "PasskeyLimitReachedError", - "PasskeyNameTakenError", - "PasskeyNotFoundError", - "PasskeySignInFailedError", - "PasskeysNotConfiguredError", - "PasswordNotSetError", - "PasswordPolicyError", - "ResetTokenInvalidError", - "SignInAddressRequiredError", "TenancyConflictError", "TenancyError", "TenancyForbiddenError", "TenancyNotFoundError", "TenancyValidationError", "PublicEmailDomainError", - "UnmodifiedPasswordError", "TooManyOrganizationDomainsError", "UnregistrableDomainError", - "VerificationTokenInvalidError", "WorkspaceAlreadyExistsError", "WorkspaceActivationUnavailableError", "WorkspaceAlreadyActivatedError", diff --git a/src/gateway/services/tenancy/password_policy.py b/src/gateway/services/tenancy/password_policy.py index 5c66d26a95..6115dc72b5 100644 --- a/src/gateway/services/tenancy/password_policy.py +++ b/src/gateway/services/tenancy/password_policy.py @@ -6,8 +6,8 @@ either without a cycle. """ +from gateway.exceptions.identity_exceptions import PasswordPolicyError from gateway.services.password_service import MAX_PASSWORD_BYTES, MIN_PASSWORD_LENGTH -from gateway.services.tenancy.errors import PasswordPolicyError def validate_new_password(password: str) -> None: diff --git a/src/gateway/services/tenancy/user_service.py b/src/gateway/services/tenancy/user_service.py index d794b1fc92..8474a9f5dd 100644 --- a/src/gateway/services/tenancy/user_service.py +++ b/src/gateway/services/tenancy/user_service.py @@ -55,17 +55,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig -from gateway.models.tenancy import User -from gateway.repositories.tenancy import UserRepository -from gateway.services.dashboard_session_service import revoke_user_dashboard_sessions -from gateway.services.mail import Mailer -from gateway.services.password_service import ( - hash_password_async, - verify_absent_password_async, - verify_password_async, -) -from gateway.services.tenancy.email_address import validated_email -from gateway.services.tenancy.errors import ( +from gateway.exceptions.identity_exceptions import ( CurrentPasswordIncorrectError, EmailAlreadyInUseError, EmailNotVerifiedError, @@ -76,6 +66,16 @@ UnmodifiedPasswordError, VerificationTokenInvalidError, ) +from gateway.models.tenancy import User +from gateway.repositories.tenancy import UserRepository +from gateway.services.dashboard_session_service import revoke_user_dashboard_sessions +from gateway.services.mail import Mailer +from gateway.services.password_service import ( + hash_password_async, + verify_absent_password_async, + verify_password_async, +) +from gateway.services.tenancy.email_address import validated_email from gateway.services.tenancy.membership_listener import MembershipListener from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.tenancy.password_policy import validate_new_password diff --git a/src/gateway/services/tenancy/webauthn_service.py b/src/gateway/services/tenancy/webauthn_service.py index d140726570..ed83f2ce48 100644 --- a/src/gateway/services/tenancy/webauthn_service.py +++ b/src/gateway/services/tenancy/webauthn_service.py @@ -70,15 +70,7 @@ ) from gateway.core.config import GatewayConfig, RelyingParty -from gateway.log_config import logger -from gateway.models.tenancy import ( - WEBAUTHN_CHALLENGE_TTL_SECONDS, - User, - WebAuthnChallenge, - WebAuthnCredential, - WebAuthnCredentialPublic, -) -from gateway.services.tenancy.errors import ( +from gateway.exceptions.identity_exceptions import ( PasskeyAlreadyRegisteredError, PasskeyCeremonyError, PasskeyLimitReachedError, @@ -87,6 +79,14 @@ PasskeySignInFailedError, PasskeysNotConfiguredError, ) +from gateway.log_config import logger +from gateway.models.tenancy import ( + WEBAUTHN_CHALLENGE_TTL_SECONDS, + User, + WebAuthnChallenge, + WebAuthnCredential, + WebAuthnCredentialPublic, +) # How many passkeys one identity may hold. Generous rather than tight: the # reason to have more than one is that losing the only one is a lockout, so a diff --git a/tests/integration/test_deployment_user_administration.py b/tests/integration/test_deployment_user_administration.py index 4851f269df..fe1bf7fdd2 100644 --- a/tests/integration/test_deployment_user_administration.py +++ b/tests/integration/test_deployment_user_administration.py @@ -22,6 +22,13 @@ from sqlmodel import col from gateway.core.config import API_ROOT, GatewayConfig +from gateway.exceptions.identity_exceptions import ( + BootstrapOperatorProtectedError, + DeploymentAdministrationUnavailableError, + DeploymentUserNotFoundError, + DeploymentUserSelfChangeError, + EmptyDeploymentUserUpdateError, +) from gateway.models.platform import RuntimeSetting from gateway.models.tenancy import ( DashboardSession, @@ -37,13 +44,6 @@ hash_session_token, ) from gateway.services.tenancy.deployment_user_service import DeploymentUserService -from gateway.services.tenancy.errors import ( - BootstrapOperatorProtectedError, - DeploymentAdministrationUnavailableError, - DeploymentUserNotFoundError, - DeploymentUserSelfChangeError, - EmptyDeploymentUserUpdateError, -) from gateway.services.tenancy.provisioning_service import BOOTSTRAP_IDENTITY_KEY diff --git a/tests/integration/test_tenancy_races.py b/tests/integration/test_tenancy_races.py index dc887933f2..87ce5b93ee 100644 --- a/tests/integration/test_tenancy_races.py +++ b/tests/integration/test_tenancy_races.py @@ -27,6 +27,11 @@ from gateway.core.config import GatewayConfig from gateway.core.unit_of_work import UnitOfWork from gateway.exceptions.budget_exceptions import OrganizationScopeNotFoundError +from gateway.exceptions.identity_exceptions import ( + EmailAlreadyInUseError, + ResetTokenInvalidError, + VerificationTokenInvalidError, +) from gateway.models.api_keys import APIKey from gateway.models.budgets import SCOPE_WORKSPACE, SCOPE_WORKSPACE_MEMBER, ScopedBudget, ScopeType from gateway.models.tenancy import ( @@ -63,7 +68,6 @@ from gateway.services.password_service import verify_password_async from gateway.services.tenancy import OrganizationService, WorkspaceService, user_service from gateway.services.tenancy.errors import ( - EmailAlreadyInUseError, ForeignTenancyError, InvitationAlreadyPendingError, InvitationAlreadyUsedError, @@ -72,8 +76,6 @@ MembershipUpdateError, NotAuthorizedError, OrganizationMemberAlreadyExistsError, - ResetTokenInvalidError, - VerificationTokenInvalidError, WorkspaceAlreadyExistsError, WorkspaceMemberAlreadyExistsError, ) diff --git a/tests/unit/test_oauth_service.py b/tests/unit/test_oauth_service.py index 486a524dab..77eec464b1 100644 --- a/tests/unit/test_oauth_service.py +++ b/tests/unit/test_oauth_service.py @@ -26,9 +26,9 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import OAUTH_PROVIDERS, GatewayConfig +from gateway.exceptions.identity_exceptions import OAuthExchangeError, OAuthNotConfiguredError, OAuthStateError from gateway.log_config import logger as gateway_logger from gateway.services import oauth_service -from gateway.services.tenancy.errors import OAuthExchangeError, OAuthNotConfiguredError, OAuthStateError class FakeSession: diff --git a/tests/unit/test_webauthn_challenge.py b/tests/unit/test_webauthn_challenge.py index 66c05ef13e..25511f5f8f 100644 --- a/tests/unit/test_webauthn_challenge.py +++ b/tests/unit/test_webauthn_challenge.py @@ -19,9 +19,9 @@ from sqlmodel import SQLModel, col import gateway.models # noqa: F401 imports every model module, so create_all sees the whole schema +from gateway.exceptions.identity_exceptions import PasskeyCeremonyError from gateway.models.tenancy import Organization, User, WebAuthnChallenge from gateway.services.tenancy import webauthn_service -from gateway.services.tenancy.errors import PasskeyCeremonyError CHALLENGE = b"\x01" * 32 OTHER_CHALLENGE = b"\x02" * 32 From 3365f5dedea2e1fadd7ca236cf5522ec3f5f85ee Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:21:59 +0100 Subject: [PATCH 8/9] refactor(exceptions): move the organizations errors into organizations_exceptions.py The last thirty-one errors, organizations, workspaces, members, invitations, email-domain claims and first-boot provisioning, leave for their own domain module, and `services/tenancy/errors.py` goes with them (#1193). The design note it carried, that a class names its own status and one handler renders the family, moves to the `gateway.exceptions` package docstring, which is where every caller now reaches the family. The three remaining pointers at the old module name the package instead. `test_tenancy_errors_home.py` asserted that the bases the old module re-exported were the ones in `gateway.exceptions`. With no re-export left there is nothing for it to compare. Pure move: the classes, their statuses and their messages are unchanged. --- src/gateway/api/routes/organization_keys.py | 2 +- src/gateway/exceptions/__init__.py | 8 ++++- .../organizations_exceptions.py} | 20 ++----------- src/gateway/main.py | 8 ++--- .../services/overview/overview_service.py | 2 +- src/gateway/services/playground_service.py | 4 +-- src/gateway/services/tenancy/authorization.py | 2 +- .../tenancy/organization_domain_service.py | 18 +++++------ .../tenancy/organization_guardrail_service.py | 2 +- .../services/tenancy/organization_service.py | 30 +++++++++---------- .../services/tenancy/provisioning_service.py | 2 +- .../tenancy/workspace_activation_service.py | 8 ++--- .../services/tenancy/workspace_service.py | 20 ++++++------- .../test_invitee_membership_inbox.py | 10 +++---- .../test_org_provider_key_models.py | 2 +- tests/integration/test_org_provider_keys.py | 2 +- .../integration/test_organization_budgets.py | 2 +- .../integration/test_organization_domains.py | 20 ++++++------- ...ganization_guardrail_definition_service.py | 2 +- .../test_organization_guardrails.py | 2 +- .../test_organization_pricing_routes.py | 2 +- .../integration/test_tenancy_authorization.py | 16 +++++----- tests/integration/test_tenancy_races.py | 24 +++++++-------- .../integration/test_workspace_activation.py | 12 ++++---- .../test_workspace_code_execution_policy.py | 2 +- .../integration/test_workspace_mcp_servers.py | 2 +- .../test_workspace_member_budget_policies.py | 2 +- .../integration/test_workspace_web_search.py | 2 +- tests/unit/test_tenancy_errors_home.py | 19 ------------ 29 files changed, 110 insertions(+), 137 deletions(-) rename src/gateway/{services/tenancy/errors.py => exceptions/organizations_exceptions.py} (94%) delete mode 100644 tests/unit/test_tenancy_errors_home.py diff --git a/src/gateway/api/routes/organization_keys.py b/src/gateway/api/routes/organization_keys.py index 6b2a906823..5505895b15 100644 --- a/src/gateway/api/routes/organization_keys.py +++ b/src/gateway/api/routes/organization_keys.py @@ -60,6 +60,7 @@ ) from gateway.auth.models import hash_key, key_suffix from gateway.core.config import GatewayConfig +from gateway.exceptions.organizations_exceptions import WorkspaceNotFoundError from gateway.models.api_keys import APIKey from gateway.models.tenancy import User as TenancyUser from gateway.models.tenancy import Workspace @@ -69,7 +70,6 @@ from gateway.services.model_access import is_allowlist_subset, validate_allowed_models from gateway.services.tenancy import OrganizationService from gateway.services.tenancy.authorization import resolve_workspace_in_organization -from gateway.services.tenancy.errors import WorkspaceNotFoundError from gateway.services.workspace_scope import organization_default_workspace_id router = APIRouter( diff --git a/src/gateway/exceptions/__init__.py b/src/gateway/exceptions/__init__.py index 94a47eb462..b708e422ee 100644 --- a/src/gateway/exceptions/__init__.py +++ b/src/gateway/exceptions/__init__.py @@ -1,4 +1,10 @@ -"""The gateway's error classes, presented at one import path.""" +"""The gateway's error classes, presented at one import path. + +Each class names the HTTP status its condition maps to, and one handler +registered in `gateway.main` renders it as FastAPI's own ``{"detail": ...}`` +body. A service raises a domain error and its routes stay thin, with no +try/except around each one. +""" from gateway.exceptions._base import ( TenancyConflictError, diff --git a/src/gateway/services/tenancy/errors.py b/src/gateway/exceptions/organizations_exceptions.py similarity index 94% rename from src/gateway/services/tenancy/errors.py rename to src/gateway/exceptions/organizations_exceptions.py index a44067564f..09682c42db 100644 --- a/src/gateway/services/tenancy/errors.py +++ b/src/gateway/exceptions/organizations_exceptions.py @@ -1,13 +1,4 @@ -"""Domain errors for the tenancy services, and the HTTP status each carries. - -The platform maps roughly 25 exception modules to statuses in one central table -(`otari-ai` `backend/app/api/exception_handlers.py`). The gateway has no such -table and its routes raise ``HTTPException`` directly, which would mean a -try/except around every one of the tenancy handlers. Instead each error names -its own status here and one handler, registered in `gateway.main`, renders it as -FastAPI's own ``{"detail": ...}`` body, so a rehomed service keeps raising -domain errors and the routes stay thin. -""" +"""Errors the organization and workspace surfaces raise, and the HTTP status each carries.""" from fastapi import status @@ -309,26 +300,21 @@ def __init__(self) -> None: "MembershipUpdateError", "NotAnOrganizationMemberError", "NotAuthorizedError", - "OrganizationMemberAlreadyExistsError", "OrganizationDomainAlreadyClaimedError", "OrganizationDomainClaimedHereError", "OrganizationDomainNotFoundError", "OrganizationDomainNotVerifiedError", + "OrganizationMemberAlreadyExistsError", "OrganizationMemberNotFoundError", "OrganizationNameRequiredError", "OrganizationNotFoundError", "OrganizationSlugUnavailableError", - "TenancyConflictError", - "TenancyError", - "TenancyForbiddenError", - "TenancyNotFoundError", - "TenancyValidationError", "PublicEmailDomainError", "TooManyOrganizationDomainsError", "UnregistrableDomainError", - "WorkspaceAlreadyExistsError", "WorkspaceActivationUnavailableError", "WorkspaceAlreadyActivatedError", + "WorkspaceAlreadyExistsError", "WorkspaceInUseError", "WorkspaceMemberAlreadyExistsError", "WorkspaceMemberNotFoundError", diff --git a/src/gateway/main.py b/src/gateway/main.py index e77b868750..ffdd31f73d 100644 --- a/src/gateway/main.py +++ b/src/gateway/main.py @@ -600,10 +600,10 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: async def _tenancy_error_handler(_: Request, exc: Exception) -> Response: """Render a tenancy domain error as the status it carries. - One handler for the whole family, so a rehomed service keeps raising domain - errors and no tenancy route needs a try/except (see - `gateway.services.tenancy.errors`). The body matches FastAPI's own - ``HTTPException`` shape, so a client cannot tell which layer answered. + One handler for the whole family, so a service keeps raising domain errors + and no route needs a try/except (see `gateway.exceptions`). The body matches + FastAPI's own ``HTTPException`` shape, so a client cannot tell which layer + answered. A 4xx message is written for the caller and is rendered as it is. A 5xx one is not: it describes the deployment rather than the request, and diff --git a/src/gateway/services/overview/overview_service.py b/src/gateway/services/overview/overview_service.py index 3d2b3454a9..cc49e7f6aa 100644 --- a/src/gateway/services/overview/overview_service.py +++ b/src/gateway/services/overview/overview_service.py @@ -14,11 +14,11 @@ import uuid from dataclasses import dataclass +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.models.tenancy import Organization from gateway.models.tenancy import User as TenancyUser from gateway.repositories.overview.overview_repository import Allocation, OverviewRepository from gateway.services.tenancy.deployment_user_service import DeploymentUserService -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.tenancy.workspace_service import WorkspaceService diff --git a/src/gateway/services/playground_service.py b/src/gateway/services/playground_service.py index f174a6b15c..ea31da00fe 100644 --- a/src/gateway/services/playground_service.py +++ b/src/gateway/services/playground_service.py @@ -26,6 +26,7 @@ from gateway.core.config import GatewayConfig from gateway.exceptions import TenancyConflictError, TenancyForbiddenError +from gateway.exceptions.organizations_exceptions import WorkspaceNotFoundError from gateway.models.playground import ( MAX_FAVORITE_MODELS, MAX_SAVED_COMPARISONS, @@ -49,7 +50,6 @@ from gateway.repositories.users_repository import get_or_create_attribution_user from gateway.services.tenancy import OrganizationService from gateway.services.tenancy.authorization import resolve_workspace_in_organization -from gateway.services.tenancy.errors import WorkspaceNotFoundError from gateway.services.workspace_scope import organization_default_workspace_id from gateway.types.session_principal import SessionPrincipal @@ -59,7 +59,7 @@ # ``HTTPException``, because this slice authorizes through ``services/tenancy/`` # and that family is what the handler registered in ``gateway.main`` renders. # The status belongs to the condition rather than to the endpoint, which is the -# rule ``services/tenancy/errors.py`` states. +# rule ``gateway.exceptions`` states. _SPEND_IDENTITY_REVOKED = "Your spend identity has been deactivated on this deployment; ask an operator to restore it." _NO_WORKSPACE = "You are not a member of a workspace on this deployment; ask an operator to add you to one." diff --git a/src/gateway/services/tenancy/authorization.py b/src/gateway/services/tenancy/authorization.py index 4770ad73d5..d06c7fe0e9 100644 --- a/src/gateway/services/tenancy/authorization.py +++ b/src/gateway/services/tenancy/authorization.py @@ -10,9 +10,9 @@ from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.models.tenancy import MANAGEMENT_ROLES, Organization, User, Workspace from gateway.repositories.tenancy import WorkspaceMemberRepository, WorkspaceRepository -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.organization_service import OrganizationService diff --git a/src/gateway/services/tenancy/organization_domain_service.py b/src/gateway/services/tenancy/organization_domain_service.py index d8ca36b395..9bda881af3 100644 --- a/src/gateway/services/tenancy/organization_domain_service.py +++ b/src/gateway/services/tenancy/organization_domain_service.py @@ -51,6 +51,15 @@ is_registrable_domain, normalized_domain, ) +from gateway.exceptions.organizations_exceptions import ( + OrganizationDomainAlreadyClaimedError, + OrganizationDomainClaimedHereError, + OrganizationDomainNotFoundError, + OrganizationDomainNotVerifiedError, + PublicEmailDomainError, + TooManyOrganizationDomainsError, + UnregistrableDomainError, +) from gateway.log_config import logger from gateway.models.tenancy import ( DOMAIN_PROOF_TTL, @@ -67,15 +76,6 @@ from gateway.repositories.tenancy.organization_domain_repository import OrganizationDomainRepository from gateway.repositories.tenancy.organization_member_repository import OrganizationMemberRepository from gateway.services.tenancy.domain_verification import resolve_txt_records -from gateway.services.tenancy.errors import ( - OrganizationDomainAlreadyClaimedError, - OrganizationDomainClaimedHereError, - OrganizationDomainNotFoundError, - OrganizationDomainNotVerifiedError, - PublicEmailDomainError, - TooManyOrganizationDomainsError, - UnregistrableDomainError, -) from gateway.services.tenancy.organization_service import OrganizationService # How long a verification token is, in bytes of entropy before hex encoding. diff --git a/src/gateway/services/tenancy/organization_guardrail_service.py b/src/gateway/services/tenancy/organization_guardrail_service.py index ed8833fbb6..18f3892107 100644 --- a/src/gateway/services/tenancy/organization_guardrail_service.py +++ b/src/gateway/services/tenancy/organization_guardrail_service.py @@ -72,6 +72,7 @@ OrganizationGuardrailTestsItsDefinitionError, OrganizationGuardrailUnsafeUrlError, ) +from gateway.exceptions.organizations_exceptions import WorkspaceNotFoundError from gateway.exceptions.shared_exceptions import SecretBoxUnavailableTenancyError from gateway.log_config import logger from gateway.models.guardrails import ( @@ -89,7 +90,6 @@ decrypt_secret, encrypt_secret, ) -from gateway.services.tenancy.errors import WorkspaceNotFoundError from gateway.services.tenancy.organization_service import OrganizationService from gateway.services.url_safety import UnsafeURLError, validate_mcp_url diff --git a/src/gateway/services/tenancy/organization_service.py b/src/gateway/services/tenancy/organization_service.py index e1f7b03fb4..3ac389f473 100644 --- a/src/gateway/services/tenancy/organization_service.py +++ b/src/gateway/services/tenancy/organization_service.py @@ -19,6 +19,21 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig +from gateway.exceptions.organizations_exceptions import ( + InvitationAlreadyPendingError, + InvitationAlreadyUsedError, + InvitationExpiredError, + InvitationNotFoundError, + InvitationPasswordNotAcceptedError, + MembershipUpdateError, + NotAuthorizedError, + OrganizationMemberAlreadyExistsError, + OrganizationMemberNotFoundError, + OrganizationNameRequiredError, + OrganizationNotFoundError, + OrganizationSlugUnavailableError, + WorkspaceNotFoundError, +) from gateway.models.money import as_float from gateway.models.tenancy import ( MANAGEMENT_ROLES, @@ -70,21 +85,6 @@ from gateway.services.secret_box import secret_box_configured from gateway.services.tenancy.deployment_user_service import DeploymentUserService from gateway.services.tenancy.email_address import validated_email as _validated_email -from gateway.services.tenancy.errors import ( - InvitationAlreadyPendingError, - InvitationAlreadyUsedError, - InvitationExpiredError, - InvitationNotFoundError, - InvitationPasswordNotAcceptedError, - MembershipUpdateError, - NotAuthorizedError, - OrganizationMemberAlreadyExistsError, - OrganizationMemberNotFoundError, - OrganizationNameRequiredError, - OrganizationNotFoundError, - OrganizationSlugUnavailableError, - WorkspaceNotFoundError, -) from gateway.services.tenancy.invitation_email import render_invitation_email # The name first boot gives an organization's workspace, reused so a created diff --git a/src/gateway/services/tenancy/provisioning_service.py b/src/gateway/services/tenancy/provisioning_service.py index fe21b8a1a4..92038f21cb 100644 --- a/src/gateway/services/tenancy/provisioning_service.py +++ b/src/gateway/services/tenancy/provisioning_service.py @@ -31,6 +31,7 @@ from sqlmodel import col from gateway.exceptions import TenancyError, TenancyNotFoundError +from gateway.exceptions.organizations_exceptions import ForeignTenancyError from gateway.log_config import logger from gateway.models.platform import RuntimeSetting from gateway.models.tenancy import Organization, User @@ -42,7 +43,6 @@ WorkspaceRepository, ) from gateway.repositories.users_repository import get_or_create_attribution_user -from gateway.services.tenancy.errors import ForeignTenancyError from gateway.services.tenancy.membership_listener import MembershipListener # Stored in runtime_settings, and deliberately not a SETTABLE_KEY, so diff --git a/src/gateway/services/tenancy/workspace_activation_service.py b/src/gateway/services/tenancy/workspace_activation_service.py index e0b648bf03..3dff54b5fb 100644 --- a/src/gateway/services/tenancy/workspace_activation_service.py +++ b/src/gateway/services/tenancy/workspace_activation_service.py @@ -49,6 +49,10 @@ from gateway.auth.models import hash_key, key_suffix from gateway.core.config import GatewayConfig from gateway.core.usage_source import integration_traffic, served_here +from gateway.exceptions.organizations_exceptions import ( + WorkspaceActivationUnavailableError, + WorkspaceAlreadyActivatedError, +) from gateway.models.api_keys import APIKey from gateway.models.money import as_float from gateway.models.tenancy import User, Workspace, WorkspaceActivationState @@ -56,10 +60,6 @@ from gateway.ports.api_key_format_port import ApiKeyFormatPort from gateway.repositories.users_repository import get_or_create_attribution_user from gateway.services.tenancy import authorization -from gateway.services.tenancy.errors import ( - WorkspaceActivationUnavailableError, - WorkspaceAlreadyActivatedError, -) from gateway.services.tenancy.organization_service import OrganizationService # What the guide calls the key it mints, as the Keys page shows it. One name for diff --git a/src/gateway/services/tenancy/workspace_service.py b/src/gateway/services/tenancy/workspace_service.py index f880842d1f..cdaabca689 100644 --- a/src/gateway/services/tenancy/workspace_service.py +++ b/src/gateway/services/tenancy/workspace_service.py @@ -20,6 +20,16 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession +from gateway.exceptions.organizations_exceptions import ( + InvalidRoleError, + LastWorkspaceError, + NotAnOrganizationMemberError, + WorkspaceAlreadyExistsError, + WorkspaceInUseError, + WorkspaceMemberAlreadyExistsError, + WorkspaceMemberNotFoundError, + WorkspaceNameRequiredError, +) from gateway.models.tenancy import ( MANAGEMENT_ROLES, WORKSPACE_MEMBER_ROLES, @@ -36,16 +46,6 @@ ) from gateway.repositories.tenancy import WorkspaceMemberRepository, WorkspaceRepository from gateway.services.tenancy import authorization -from gateway.services.tenancy.errors import ( - InvalidRoleError, - LastWorkspaceError, - NotAnOrganizationMemberError, - WorkspaceAlreadyExistsError, - WorkspaceInUseError, - WorkspaceMemberAlreadyExistsError, - WorkspaceMemberNotFoundError, - WorkspaceNameRequiredError, -) from gateway.services.tenancy.membership_listener import MembershipListener from gateway.services.tenancy.organization_service import OrganizationService diff --git a/tests/integration/test_invitee_membership_inbox.py b/tests/integration/test_invitee_membership_inbox.py index 52854bdb04..a5482484f5 100644 --- a/tests/integration/test_invitee_membership_inbox.py +++ b/tests/integration/test_invitee_membership_inbox.py @@ -19,6 +19,11 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig +from gateway.exceptions.organizations_exceptions import ( + InvitationAlreadyUsedError, + InvitationExpiredError, + InvitationNotFoundError, +) from gateway.models.tenancy import ( Invitation, InviteOrganizationMemberRequest, @@ -38,11 +43,6 @@ ) from gateway.services.budgets import WorkspaceBudgetDefaultService from gateway.services.tenancy import OrganizationService -from gateway.services.tenancy.errors import ( - InvitationAlreadyUsedError, - InvitationExpiredError, - InvitationNotFoundError, -) _TEST_CONFIG = GatewayConfig() diff --git a/tests/integration/test_org_provider_key_models.py b/tests/integration/test_org_provider_key_models.py index 1cdfd94a46..a8d2ccc546 100644 --- a/tests/integration/test_org_provider_key_models.py +++ b/tests/integration/test_org_provider_key_models.py @@ -24,6 +24,7 @@ from gateway.core.config import GatewayConfig from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions.organizations_exceptions import NotAuthorizedError from gateway.exceptions.providers_exceptions import ( OrgProviderKeyNotFoundError, OrgProviderLastModelError, @@ -57,7 +58,6 @@ from gateway.services.providers import OrgProviderModelService from gateway.services.secret_box import generate_secret_key from gateway.services.tenancy import OrgProviderKeyService -from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.org_provider_key_service import ( cached_org_model_restriction, refresh_org_provider_cache, diff --git a/tests/integration/test_org_provider_keys.py b/tests/integration/test_org_provider_keys.py index 31b6821b2c..1852cc8a8a 100644 --- a/tests/integration/test_org_provider_keys.py +++ b/tests/integration/test_org_provider_keys.py @@ -18,6 +18,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.exceptions.providers_exceptions import ( OrgProviderKeyAlreadyExistsError, OrgProviderKeyArchivedError, @@ -47,7 +48,6 @@ from gateway.services.provider_kwargs import resolve_provider_selector from gateway.services.secret_box import encrypt_secret, generate_secret_key from gateway.services.tenancy import OrgProviderKeyService -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.org_provider_key_service import ( cached_org_model_restriction, refresh_org_provider_cache, diff --git a/tests/integration/test_organization_budgets.py b/tests/integration/test_organization_budgets.py index 9fed0f8a6d..779cd8cef0 100644 --- a/tests/integration/test_organization_budgets.py +++ b/tests/integration/test_organization_budgets.py @@ -39,6 +39,7 @@ OrganizationScopedBudgetNotFoundError, OrganizationScopeNotFoundError, ) +from gateway.exceptions.organizations_exceptions import NotAuthorizedError from gateway.models.api_keys import APIKey from gateway.models.budgets import Budget, BudgetResetLog, ScopedBudget, WorkspaceBudgetDefault from gateway.models.tenancy import Organization, OrganizationMember, User, Workspace, WorkspaceMember @@ -60,7 +61,6 @@ ) from gateway.services.api_keys import ApiKeyService from gateway.services.budgets import BudgetService -from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.organization_service import OrganizationService _BUDGETS = f"{API_ROOT}/organizations/me/budgets" diff --git a/tests/integration/test_organization_domains.py b/tests/integration/test_organization_domains.py index 8de00dd64a..2df5caf5c0 100644 --- a/tests/integration/test_organization_domains.py +++ b/tests/integration/test_organization_domains.py @@ -18,6 +18,16 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import API_ROOT +from gateway.exceptions.organizations_exceptions import ( + NotAuthorizedError, + OrganizationDomainAlreadyClaimedError, + OrganizationDomainClaimedHereError, + OrganizationDomainNotFoundError, + OrganizationDomainNotVerifiedError, + PublicEmailDomainError, + TooManyOrganizationDomainsError, + UnregistrableDomainError, +) from gateway.models.tenancy import ( DOMAIN_PROOF_TTL, DOMAIN_VERIFICATION_TXT_PREFIX, @@ -34,16 +44,6 @@ OrganizationRepository, UserRepository, ) -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - OrganizationDomainAlreadyClaimedError, - OrganizationDomainClaimedHereError, - OrganizationDomainNotFoundError, - OrganizationDomainNotVerifiedError, - PublicEmailDomainError, - TooManyOrganizationDomainsError, - UnregistrableDomainError, -) from gateway.services.tenancy.organization_domain_service import OrganizationDomainService _SERVICE_MODULE = "gateway.services.tenancy.organization_domain_service" diff --git a/tests/integration/test_organization_guardrail_definition_service.py b/tests/integration/test_organization_guardrail_definition_service.py index ad7182003c..e110e5649e 100644 --- a/tests/integration/test_organization_guardrail_definition_service.py +++ b/tests/integration/test_organization_guardrail_definition_service.py @@ -34,6 +34,7 @@ OrganizationGuardrailNotBuildableError, OrganizationGuardrailNotDefinableError, ) +from gateway.exceptions.organizations_exceptions import NotAuthorizedError from gateway.models.guardrails import OrganizationGuardrail, OrganizationGuardrailDefinition from gateway.models.tenancy import Organization, User from gateway.repositories.tenancy import ( @@ -44,7 +45,6 @@ ) from gateway.services.secret_box import decrypt_secret, generate_secret_key from gateway.services.tenancy import organization_guardrail_runner as runner -from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.organization_guardrail_definition_service import ( MAX_DEFINITIONS_PER_ORGANIZATION, OrganizationGuardrailDefinitionCreate, diff --git a/tests/integration/test_organization_guardrails.py b/tests/integration/test_organization_guardrails.py index 98b84a6962..eaa832bf7f 100644 --- a/tests/integration/test_organization_guardrails.py +++ b/tests/integration/test_organization_guardrails.py @@ -37,6 +37,7 @@ OrganizationGuardrailTestsItsDefinitionError, OrganizationGuardrailUnsafeUrlError, ) +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.models.guardrails import ( OrganizationGuardrail, OrganizationGuardrailDefinition, @@ -51,7 +52,6 @@ WorkspaceRepository, ) from gateway.services.secret_box import decrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.organization_guardrail_service import ( MAX_GUARDRAILS_PER_ORGANIZATION, OrganizationGuardrailCreate, diff --git a/tests/integration/test_organization_pricing_routes.py b/tests/integration/test_organization_pricing_routes.py index a11bbff01e..d711fb3169 100644 --- a/tests/integration/test_organization_pricing_routes.py +++ b/tests/integration/test_organization_pricing_routes.py @@ -24,6 +24,7 @@ from gateway.core.config import API_ROOT, GatewayConfig from gateway.exceptions import TenancyValidationError +from gateway.exceptions.organizations_exceptions import NotAuthorizedError from gateway.exceptions.pricing_exceptions import ( OrganizationPricingManagedModelError, OrganizationPricingNotFoundError, @@ -49,7 +50,6 @@ from gateway.services.pricing_service import find_model_pricing from gateway.services.provider_kwargs import credential_ladder_exhausted, get_provider_kwargs from gateway.services.secret_box import encrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import NotAuthorizedError from gateway.services.tenancy.org_provider_key_service import refresh_org_provider_cache, reset_org_provider_cache from gateway.services.workspace_scope import ( organization_for_key_id, diff --git a/tests/integration/test_tenancy_authorization.py b/tests/integration/test_tenancy_authorization.py index 46137b59ec..9f3c4bf239 100644 --- a/tests/integration/test_tenancy_authorization.py +++ b/tests/integration/test_tenancy_authorization.py @@ -14,6 +14,14 @@ from sqlalchemy.ext.asyncio import AsyncSession from gateway.core.config import GatewayConfig +from gateway.exceptions.organizations_exceptions import ( + InvitationAlreadyPendingError, + MembershipUpdateError, + NotAuthorizedError, + OrganizationNameRequiredError, + OrganizationNotFoundError, + WorkspaceNotFoundError, +) from gateway.models.tenancy import ( ActiveOrganizationMemberCreateRequest, ActiveOrganizationMemberUpdateRequest, @@ -37,14 +45,6 @@ from gateway.services.budgets import WorkspaceBudgetDefaultService from gateway.services.tenancy import OrganizationService, WorkspaceService from gateway.services.tenancy.authorization import resolve_visible_workspace_scope -from gateway.services.tenancy.errors import ( - InvitationAlreadyPendingError, - MembershipUpdateError, - NotAuthorizedError, - OrganizationNameRequiredError, - OrganizationNotFoundError, - WorkspaceNotFoundError, -) from gateway.services.tenancy.provisioning_service import DEFAULT_WORKSPACE_NAME _TEST_CONFIG = GatewayConfig() diff --git a/tests/integration/test_tenancy_races.py b/tests/integration/test_tenancy_races.py index 87ce5b93ee..a5422ee7b9 100644 --- a/tests/integration/test_tenancy_races.py +++ b/tests/integration/test_tenancy_races.py @@ -32,6 +32,18 @@ ResetTokenInvalidError, VerificationTokenInvalidError, ) +from gateway.exceptions.organizations_exceptions import ( + ForeignTenancyError, + InvitationAlreadyPendingError, + InvitationAlreadyUsedError, + InvitationPasswordNotAcceptedError, + LastWorkspaceError, + MembershipUpdateError, + NotAuthorizedError, + OrganizationMemberAlreadyExistsError, + WorkspaceAlreadyExistsError, + WorkspaceMemberAlreadyExistsError, +) from gateway.models.api_keys import APIKey from gateway.models.budgets import SCOPE_WORKSPACE, SCOPE_WORKSPACE_MEMBER, ScopedBudget, ScopeType from gateway.models.tenancy import ( @@ -67,18 +79,6 @@ from gateway.services.budgets import BudgetService, WorkspaceBudgetDefaultService from gateway.services.password_service import verify_password_async from gateway.services.tenancy import OrganizationService, WorkspaceService, user_service -from gateway.services.tenancy.errors import ( - ForeignTenancyError, - InvitationAlreadyPendingError, - InvitationAlreadyUsedError, - InvitationPasswordNotAcceptedError, - LastWorkspaceError, - MembershipUpdateError, - NotAuthorizedError, - OrganizationMemberAlreadyExistsError, - WorkspaceAlreadyExistsError, - WorkspaceMemberAlreadyExistsError, -) from gateway.services.tenancy.provisioning_service import ( BOOTSTRAP_IDENTITY_KEY, ensure_bootstrap_identity, diff --git a/tests/integration/test_workspace_activation.py b/tests/integration/test_workspace_activation.py index 12379a1f69..3d9538c77a 100644 --- a/tests/integration/test_workspace_activation.py +++ b/tests/integration/test_workspace_activation.py @@ -19,6 +19,12 @@ from gateway.adapters.api_key_format_adapter import DefaultApiKeyFormatAdapter from gateway.auth.models import hash_key from gateway.core.config import GatewayConfig +from gateway.exceptions.organizations_exceptions import ( + NotAuthorizedError, + WorkspaceActivationUnavailableError, + WorkspaceAlreadyActivatedError, + WorkspaceNotFoundError, +) from gateway.models.api_keys import APIKey from gateway.models.tenancy import Organization, User, Workspace, WorkspaceActivationState from gateway.models.usage import UsageLog @@ -33,12 +39,6 @@ from gateway.repositories.users_repository import get_or_create_attribution_user from gateway.services import playground_dispatch from gateway.services.secret_box import generate_secret_key -from gateway.services.tenancy.errors import ( - NotAuthorizedError, - WorkspaceActivationUnavailableError, - WorkspaceAlreadyActivatedError, - WorkspaceNotFoundError, -) from gateway.services.tenancy.workspace_activation_service import ( ACTIVATION_KEY_NAME, WorkspaceActivationService, diff --git a/tests/integration/test_workspace_code_execution_policy.py b/tests/integration/test_workspace_code_execution_policy.py index d202973dce..7351de78cb 100644 --- a/tests/integration/test_workspace_code_execution_policy.py +++ b/tests/integration/test_workspace_code_execution_policy.py @@ -16,6 +16,7 @@ from pydantic import ValidationError from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.exceptions.tools_exceptions import SandboxImageNotAllowedError, SandboxToolsUnrunnableError from gateway.models.tenancy import Organization, User, Workspace from gateway.models.tools import WorkspaceCodeExecutionPolicy @@ -27,7 +28,6 @@ WorkspaceRepository, ) from gateway.services.sandbox_backend import CODE_EXECUTION_TOOL_NAMES -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.workspace_code_execution_policy_service import ( SERVED_TOOL_NAMES, WorkspaceCodeExecutionPolicyService, diff --git a/tests/integration/test_workspace_mcp_servers.py b/tests/integration/test_workspace_mcp_servers.py index 7d12a3cc11..f6ba747d5f 100644 --- a/tests/integration/test_workspace_mcp_servers.py +++ b/tests/integration/test_workspace_mcp_servers.py @@ -29,6 +29,7 @@ from gateway.api.routes.chat import ChatCompletionRequest from gateway.core.config import GatewayConfig from gateway.core.unit_of_work import UnitOfWork +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.exceptions.tools_exceptions import ( WorkspaceMcpServerAlreadyExistsError, WorkspaceMcpServerLimitReachedError, @@ -46,7 +47,6 @@ WorkspaceRepository, ) from gateway.services.secret_box import decrypt_secret, generate_secret_key -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.workspace_mcp_server_service import ( MAX_ALLOWED_TOOLS, MAX_MCP_SERVERS_PER_WORKSPACE, diff --git a/tests/integration/test_workspace_member_budget_policies.py b/tests/integration/test_workspace_member_budget_policies.py index 56df71b809..0e2538268c 100644 --- a/tests/integration/test_workspace_member_budget_policies.py +++ b/tests/integration/test_workspace_member_budget_policies.py @@ -23,6 +23,7 @@ WorkspaceBudgetDefaultBudgetNotFoundError, WorkspaceBudgetDefaultNotFoundError, ) +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.models.budgets import Budget, ScopedBudget, WorkspaceBudgetDefault from gateway.models.money import as_float from gateway.models.tenancy import ( @@ -38,7 +39,6 @@ from gateway.schemas.budgets import WorkspaceMemberBudgetPolicyCreate, WorkspaceMemberBudgetPolicyUpdate from gateway.services.budgets import WorkspaceBudgetDefaultService from gateway.services.tenancy import OrganizationService, WorkspaceService -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.provisioning_service import ( DEFAULT_ORGANIZATION_SLUG, DEFAULT_WORKSPACE_NAME, diff --git a/tests/integration/test_workspace_web_search.py b/tests/integration/test_workspace_web_search.py index 49722e6b2e..a26485ed09 100644 --- a/tests/integration/test_workspace_web_search.py +++ b/tests/integration/test_workspace_web_search.py @@ -15,6 +15,7 @@ import pytest_asyncio from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from gateway.exceptions.organizations_exceptions import NotAuthorizedError, WorkspaceNotFoundError from gateway.models.tenancy import Organization, User, Workspace from gateway.models.tools import WorkspaceWebSearchConfig from gateway.repositories.tenancy import ( @@ -24,7 +25,6 @@ WorkspaceMemberRepository, WorkspaceRepository, ) -from gateway.services.tenancy.errors import NotAuthorizedError, WorkspaceNotFoundError from gateway.services.tenancy.workspace_web_search_service import ( WorkspaceWebSearchConfigUpdate, WorkspaceWebSearchService, diff --git a/tests/unit/test_tenancy_errors_home.py b/tests/unit/test_tenancy_errors_home.py deleted file mode 100644 index 020fe9dfbc..0000000000 --- a/tests/unit/test_tenancy_errors_home.py +++ /dev/null @@ -1,19 +0,0 @@ -"""The status-carrying error bases have one definition, in ``gateway.exceptions``.""" - -import pytest - -from gateway import exceptions -from gateway.services.tenancy import errors - -_BASES = ( - "TenancyError", - "TenancyNotFoundError", - "TenancyForbiddenError", - "TenancyConflictError", - "TenancyValidationError", -) - - -@pytest.mark.parametrize("name", _BASES) -def test_base_has_one_definition(name: str) -> None: - assert getattr(errors, name) is getattr(exceptions, name) From 6d73aae5df320970a385b4da8837bcc3d276c729 Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Thu, 24 Sep 2026 14:23:13 +0100 Subject: [PATCH 9/9] docs(domains): record each domain's exceptions module The errors split (#1193) gives identity, organizations, guardrails, pricing and tools a module each, and the shared set one for the errors no single domain owns. `tenancy/errors.py` leaves the organizations services list. The service-module counts in "The shape today" are left as they are. Their recorded figures do not reproduce from the tree at the commit they name, so decrementing one for the removed module would record a guess. --- docs/domains.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/domains.md b/docs/domains.md index 48efd3bcd0..bf8d6544be 100644 --- a/docs/domains.md +++ b/docs/domains.md @@ -72,7 +72,7 @@ since. A module "runs queries" when it imports a query builder (`select`, | Service packages per domain | 6: `services/tools/`, which holds the built-in tool registry and no service yet, `services/overview/`, `services/budgets/`, `services/api_keys/`, `services/files/` and `services/providers/`, which holds the organization-scoped half of providers. `services/mail/`, `services/routing/` and `services/tenancy/` are older subpackages | | Repository packages per domain | 6: `repositories/overview/`, `repositories/api_keys/`, `repositories/files/`, `repositories/budgets/`, `repositories/pricing/` and `repositories/providers/`. `repositories/tenancy/` is an older subpackage | | Modules in `schemas/` | Four domain modules so far, `budgets.py`, `files.py`, `overview.py` and `providers.py` | -| Modules in `exceptions/` | The shared error bases in `_base.py`, which the package root re-exports, and three domain modules so far, `budget_exceptions.py`, `files_exceptions.py` and `providers_exceptions.py`. `services/tenancy/errors.py` holds the rest of the tenancy errors in 1,145 lines | +| Modules in `exceptions/` | The shared error bases in `_base.py`, which the package root re-exports, `shared_exceptions.py` for the errors no one domain owns, and nine domain modules: `budget_exceptions.py`, `feedback_exceptions.py`, `files_exceptions.py`, `guardrails_exceptions.py`, `identity_exceptions.py`, `organizations_exceptions.py`, `pricing_exceptions.py`, `providers_exceptions.py` and `tools_exceptions.py` | ## The domains @@ -102,6 +102,7 @@ account administration. `tenancy/password_reset_email.py`, `oauth_service.py`, `password_service.py`, `dashboard_session_service.py` - Repositories: `tenancy/user_repository.py` +- Exceptions: `identity_exceptions.py` Its tables sit in `models/tenancy.py` today, which organizations holds. @@ -116,12 +117,13 @@ provisioning, the setup guide, and the gateway's billing users. `tenancy/authorization.py`, `tenancy/invitation_email.py`, `tenancy/organization_domain_service.py`, `tenancy/domain_verification.py`, `tenancy/provisioning_service.py`, `tenancy/workspace_activation_service.py`, - `tenancy/errors.py`, `workspace_scope.py` + `workspace_scope.py` - Repositories: `tenancy/organization_repository.py`, `tenancy/organization_member_repository.py`, `tenancy/organization_domain_repository.py`, `tenancy/invitation_repository.py`, `tenancy/workspace_repository.py`, `users_repository.py` +- Exceptions: `organizations_exceptions.py` - Models: `tenancy.py`, `users.py` ### api-keys @@ -158,6 +160,7 @@ snapshots. - Services: `pricing_service.py`, `pricing_init_service.py`, `pricing_refresh_service.py`, `organization_pricing_service.py` - Repositories: `pricing/` +- Exceptions: `pricing_exceptions.py` - Models: `pricing.py`, `pricing_schemas.py` ### providers @@ -239,6 +242,7 @@ retrieval and code execution. `tenancy/workspace_web_search_service.py`, `tenancy/workspace_code_execution_policy_service.py`, `code_execution/` - Repositories: `code_execution/` +- Exceptions: `tools_exceptions.py` - Ports: `code_execution_port.py` - Adapters: `code_execution_adapter.py`, `e2b_code_execution_adapter.py` - Models: `tools.py`, `mcp.py` @@ -259,6 +263,7 @@ configuration. `tenancy/organization_guardrail_definition_service.py`, `tenancy/organization_guardrail_runner.py` - Repositories: `tenancy/organization_guardrail_definition_repository.py` +- Exceptions: `guardrails_exceptions.py` - Models: `guardrails.py` ### agent-gates @@ -339,6 +344,7 @@ Cross-cutting modules that several domains import. They stay where they are. - Services: `url_safety.py`, `secret_box.py`, `file_extractors.py` - Repositories: `base_repository.py` +- Exceptions: `shared_exceptions.py` - Models: `base.py`, `money.py`, `secret_fields.py` `file_extractors.py` turns bytes into text and holds no state. Inference