diff --git a/api/controllers/openapi/apps.py b/api/controllers/openapi/apps.py index 6e7167db6b..3019bf7026 100644 --- a/api/controllers/openapi/apps.py +++ b/api/controllers/openapi/apps.py @@ -1,11 +1,7 @@ -"""GET /openapi/v1/apps and per-app reads (single, parameters, describe). +"""GET /openapi/v1/apps and per-app reads. -Read endpoints attach via `AppReadResource`, which stacks -`validate_bearer + require_scope` as method_decorators. List order is -innermost-first: `validate_bearer` is last in the list and ends up -outermost, so it sets `g.auth_ctx` before `require_scope` reads it. -The OAuth bearer pipeline is reserved for /run (which gates on -webapp_auth ACL). +Decorator order: `method_decorators` is innermost-first. `validate_bearer` +is last → outermost → sets `g.auth_ctx` before `require_scope` reads it. """ from __future__ import annotations @@ -44,9 +40,6 @@ from libs.oauth_bearer import ( from models import App, Tenant from models.model import AppMode, Tag, TagBinding -# Shared decorator stack for `apps:read`-scoped endpoints. List order is -# innermost-first; `validate_bearer` (last) wraps outermost so it sets -# `g.auth_ctx` before `require_scope` reads it. _APPS_READ_DECORATORS = [ require_scope(Scope.APPS_READ), validate_bearer(accept=ACCEPT_USER_ANY), @@ -62,17 +55,13 @@ _EMPTY_PARAMETERS: dict[str, Any] = { class AppReadResource(Resource): - """Base for `/apps/` read endpoints. Stacks bearer auth + scope check - on every method, then exposes `_load()` so subclasses don't repeat the - SSO-guard / app-load / membership-check ritual.""" + """Base for per-app read endpoints; subclasses call `_load()` for SSO/membership/exists checks.""" method_decorators = _APPS_READ_DECORATORS def _load(self, app_id: str) -> tuple[App, AuthContext]: ctx = g.auth_ctx - # Per-app reads are account-only; SSO subjects 404 to avoid leaking - # ID space (and dfoe_ already lacks apps:read scope, so this is - # defensive against future scope changes). + # Account-only; SSO subjects 404 (don't leak ID space). if ctx.subject_type != SubjectType.ACCOUNT or ctx.account_id is None: raise NotFound("app not found") @@ -151,12 +140,7 @@ class AppDescribeApi(AppReadResource): class AppListQuery(BaseModel): - """Query-param validator for `GET /openapi/v1/apps`. - - `mode` is a closed set (AppMode) — invalid values surface as 422 - instead of returning silently-empty data. `workspace_id` is required; - page / limit have numeric bounds; name / tag have length caps. - """ + """`mode` is a closed enum — unknown values 422 instead of silently-empty data.""" workspace_id: str page: int = Field(1, ge=1) @@ -173,8 +157,7 @@ class AppListApi(Resource): def get(self): ctx = g.auth_ctx if ctx.subject_type != SubjectType.ACCOUNT or ctx.account_id is None: - # An account-required endpoint reachable only via dfoa_ in practice - # (dfoe_ lacks apps:read). Defensive guard for future scope shifts. + # Defensive: dfoe_ lacks apps:read today, but guard against future scope shifts. return PaginationEnvelope[AppListRow].build(page=1, limit=0, total=0, items=[]).model_dump(mode="json"), 200 try: @@ -185,8 +168,6 @@ class AppListApi(Resource): workspace_id = query.workspace_id require_workspace_member(ctx, workspace_id) - tenant_name = db.session.execute(sa.select(Tenant.name).where(Tenant.id == workspace_id)).scalar_one_or_none() - page = query.page limit = query.limit mode = query.mode.value if query.mode else None @@ -221,6 +202,12 @@ class AppListApi(Resource): .all() ) + tenant_name: str | None = None + if rows: + tenant_name = db.session.execute( + sa.select(Tenant.name).where(Tenant.id == workspace_id) + ).scalar_one_or_none() + items = [ AppListRow( id=str(r.id), diff --git a/api/controllers/openapi/apps_permitted.py b/api/controllers/openapi/apps_permitted.py index 74fa1147b7..1aae847973 100644 --- a/api/controllers/openapi/apps_permitted.py +++ b/api/controllers/openapi/apps_permitted.py @@ -6,7 +6,7 @@ import sqlalchemy as sa from flask import g, request from flask_restx import Resource from pydantic import BaseModel, ConfigDict, Field, ValidationError -from werkzeug.exceptions import UnprocessableEntity +from werkzeug.exceptions import InternalServerError, UnprocessableEntity from controllers.openapi import openapi_ns from controllers.openapi._models import ( @@ -28,11 +28,7 @@ from services.enterprise.app_permitted_service import list_permitted_apps class AppPermittedListQuery(BaseModel): - """Query-param validator for `GET /openapi/v1/apps/permitted`. - - Strict — `extra='forbid'` rejects `workspace_id`, `tag`, and any other - param not explicitly allowed for this surface. Returns 422 on violation. - """ + """Strict (`extra='forbid'`) — rejects `workspace_id`/`tag`/etc. that are valid on /apps but not here.""" model_config = ConfigDict(extra="forbid") @@ -43,7 +39,7 @@ class AppPermittedListQuery(BaseModel): @openapi_ns.route("/apps/permitted") -class AppListPermittedApi(Resource): +class AppPermittedListApi(Resource): method_decorators = [ require_scope(Scope.APPS_READ_PERMITTED), validate_bearer(accept=ACCEPT_USER_EXT_SSO), @@ -57,9 +53,13 @@ class AppListPermittedApi(Resource): raise UnprocessableEntity(exc.json()) ctx = g.auth_ctx + # Fail-fast: empty subject would silently corrupt the EE allow-list query. + if not ctx.subject_email or not ctx.subject_issuer: + raise InternalServerError("malformed external_sso bearer: missing subject identity") + page_result = list_permitted_apps( - subject_email=ctx.subject_email or "", - subject_issuer=ctx.subject_issuer or "", + subject_email=ctx.subject_email, + subject_issuer=ctx.subject_issuer, page=query.page, limit=query.limit, mode=query.mode.value if query.mode else None, @@ -85,8 +85,7 @@ class AppListPermittedApi(Resource): for r in page_result.data: app = apps_by_id.get(r.app_id) if not app or app.status != "normal": - # Allow-list referenced an app that no longer exists or was archived; - # filter it out rather than emit a partial row. + # Skip allow-list entries where the app is missing/archived. continue tenant = tenants_by_id.get(str(app.tenant_id)) items.append( @@ -95,7 +94,7 @@ class AppListPermittedApi(Resource): name=app.name, description=app.description, mode=app.mode, - tags=[], # tags are tenant-scoped; not surfaced cross-tenant + tags=[], # tenant-scoped; not surfaced cross-tenant updated_at=app.updated_at.isoformat() if app.updated_at else None, created_by_name=None, # cross-tenant author leak prevention workspace_id=str(app.tenant_id), @@ -103,6 +102,7 @@ class AppListPermittedApi(Resource): ) ) + # total/has_more reflect the EE-side allow-list; len(items) may be < limit when local rows are dropped. env = PaginationEnvelope[AppListRow].build( page=query.page, limit=query.limit, total=page_result.total, items=items ) diff --git a/api/tests/unit_tests/controllers/openapi/test_apps_permitted_query.py b/api/tests/unit_tests/controllers/openapi/test_apps_permitted_query.py index 9c6306d284..1d78c89b27 100644 --- a/api/tests/unit_tests/controllers/openapi/test_apps_permitted_query.py +++ b/api/tests/unit_tests/controllers/openapi/test_apps_permitted_query.py @@ -42,3 +42,10 @@ def test_query_validates_mode_against_app_mode(): def test_query_clamps_limit_at_max(): with pytest.raises(ValidationError): AppPermittedListQuery.model_validate({"limit": 500}) + + +def test_query_accepts_valid_mode(): + """Pin the happy path: AppMode values pass.""" + q = AppPermittedListQuery.model_validate({"mode": "chat"}) + assert q.mode is not None + assert q.mode.value == "chat" diff --git a/api/tests/unit_tests/libs/test_oauth_bearer.py b/api/tests/unit_tests/libs/test_oauth_bearer.py index da93c88fc8..4620525b23 100644 --- a/api/tests/unit_tests/libs/test_oauth_bearer.py +++ b/api/tests/unit_tests/libs/test_oauth_bearer.py @@ -17,3 +17,13 @@ def test_dfoe_token_kind_carries_apps_read_permitted(): registry = build_registry(MagicMock(), MagicMock()) dfoe = next(k for k in registry.kinds() if k.prefix == "dfoe_") assert Scope.APPS_READ_PERMITTED in dfoe.scopes + + +def test_dfoa_token_kind_does_not_carry_apps_read_permitted(): + """dfoa_ relies on Scope.FULL umbrella; the explicit permitted scope + is reserved for dfoe_.""" + from libs.oauth_bearer import Scope, build_registry + + registry = build_registry(MagicMock(), MagicMock()) + dfoa = next(k for k in registry.kinds() if k.prefix == "dfoa_") + assert Scope.APPS_READ_PERMITTED not in dfoa.scopes