From cad0942f4d2811a092bb9d1cfd6e4ac126bff732 Mon Sep 17 00:00:00 2001 From: Xiyuan Chen <52963600+GareArc@users.noreply.github.com> Date: Wed, 3 Jun 2026 00:31:47 -0700 Subject: [PATCH] fix(api): enforce workspace membership + role checks in auth pipeline (#36931) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- api/controllers/openapi/apps.py | 2 +- api/controllers/openapi/auth/composition.py | 23 +- api/controllers/openapi/auth/conditions.py | 7 + api/controllers/openapi/auth/data.py | 8 +- api/controllers/openapi/auth/pipeline.py | 55 ++- api/controllers/openapi/auth/prepare.py | 40 ++- api/controllers/openapi/auth/role_gate.py | 77 ---- api/controllers/openapi/auth/verify.py | 47 ++- api/controllers/openapi/workspaces.py | 33 +- api/services/account_service.py | 6 +- .../openapi/auth/test_composition.py | 91 ++++- .../openapi/auth/test_conditions.py | 31 +- .../controllers/openapi/auth/test_data.py | 66 ++++ .../controllers/openapi/auth/test_pipeline.py | 54 +++ .../controllers/openapi/auth/test_prepare.py | 171 ++++++++- .../openapi/auth/test_role_gate.py | 330 ------------------ .../controllers/openapi/auth/test_verify.py | 124 +++++-- .../controllers/openapi/conftest.py | 14 +- .../openapi/test_workspaces_members.py | 81 ++--- .../services/test_account_service.py | 4 +- 20 files changed, 712 insertions(+), 552 deletions(-) delete mode 100644 api/controllers/openapi/auth/role_gate.py delete mode 100644 api/tests/unit_tests/controllers/openapi/auth/test_role_gate.py diff --git a/api/controllers/openapi/apps.py b/api/controllers/openapi/apps.py index d3bc4e4680..cd44014faf 100644 --- a/api/controllers/openapi/apps.py +++ b/api/controllers/openapi/apps.py @@ -147,7 +147,7 @@ class AppDescribeApi(AppReadResource): class AppListApi(Resource): @openapi_ns.doc(params=query_params_from_model(AppListQuery)) @openapi_ns.response(200, "App list", openapi_ns.models[AppListResponse.__name__]) - @auth_router.guard(scope=Scope.APPS_READ, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) + @auth_router.guard_workspace(scope=Scope.APPS_READ, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) def get(self, *, auth_data: AuthData): try: query: AppListQuery = AppListQuery.model_validate(request.args.to_dict(flat=True)) diff --git a/api/controllers/openapi/auth/composition.py b/api/controllers/openapi/auth/composition.py index c2c3e12873..66f925c8cd 100644 --- a/api/controllers/openapi/auth/composition.py +++ b/api/controllers/openapi/auth/composition.py @@ -1,11 +1,13 @@ from __future__ import annotations from controllers.openapi.auth.conditions import ( - EDITION_CE, EDITION_EE, + HAS_ALLOWED_ROLES, LOADED_APP_IS_PRIVATE, PATH_HAS_APP_ID, WEBAPP_AUTH_ENABLED, + WORKSPACE_MEMBERSHIP_REQUIRED, + WORKSPACE_SCOPED, ) from controllers.openapi.auth.data import Edition from controllers.openapi.auth.flow import When @@ -15,14 +17,18 @@ from controllers.openapi.auth.prepare import ( load_app, load_app_access_mode, load_tenant, + load_tenant_from_request, + load_workspace_role, resolve_external_user, ) from controllers.openapi.auth.verify import ( check_acl, - check_app_access, - check_membership, + check_app_api_enabled, check_private_app_permission, check_scope, + check_workspace_member, + check_workspace_mismatch, + check_workspace_role, ) from libs.oauth_bearer import TokenType @@ -30,13 +36,17 @@ account_pipeline = AuthPipeline( prepare=[ When(PATH_HAS_APP_ID, then=load_app), When(PATH_HAS_APP_ID, then=load_tenant), - load_account, # all tokens here are account tokens + When(WORKSPACE_MEMBERSHIP_REQUIRED, then=load_tenant_from_request), + load_account, + When(WORKSPACE_SCOPED, then=load_workspace_role), When(PATH_HAS_APP_ID & EDITION_EE, then=load_app_access_mode), ], auth=[ + When(PATH_HAS_APP_ID, then=check_app_api_enabled), check_scope, - When(EDITION_CE & PATH_HAS_APP_ID, then=check_membership), - When(EDITION_EE & PATH_HAS_APP_ID & ~WEBAPP_AUTH_ENABLED, then=check_app_access), + When(WORKSPACE_SCOPED, then=check_workspace_member), + When(PATH_HAS_APP_ID, then=check_workspace_mismatch), + When(HAS_ALLOWED_ROLES, then=check_workspace_role), When(PATH_HAS_APP_ID & EDITION_EE & WEBAPP_AUTH_ENABLED, then=check_acl), When(EDITION_EE & LOADED_APP_IS_PRIVATE, then=check_private_app_permission), ], @@ -50,6 +60,7 @@ external_sso_pipeline = AuthPipeline( When(PATH_HAS_APP_ID, then=load_app_access_mode), ], auth=[ + When(PATH_HAS_APP_ID, then=check_app_api_enabled), check_scope, When(PATH_HAS_APP_ID & WEBAPP_AUTH_ENABLED, then=check_acl), When(LOADED_APP_IS_PRIVATE, then=check_private_app_permission), diff --git a/api/controllers/openapi/auth/conditions.py b/api/controllers/openapi/auth/conditions.py index 2399fc04f1..5ad15e5e41 100644 --- a/api/controllers/openapi/auth/conditions.py +++ b/api/controllers/openapi/auth/conditions.py @@ -50,4 +50,11 @@ EDITION_SAAS = config_cond(lambda: current_edition() == Edition.SAAS) WEBAPP_AUTH_ENABLED = config_cond(lambda: FeatureService.get_system_features().webapp_auth.enabled) +WORKSPACE_MEMBERSHIP_REQUIRED = request_cond(lambda ctx: ctx.workspace_membership) +HAS_ALLOWED_ROLES = request_cond(lambda ctx: ctx.allowed_roles is not None) + +# Caller must belong to the resolved tenant: either an app-scoped path (tenant +# from the app) or an explicit workspace-membership path (tenant from request). +WORKSPACE_SCOPED = PATH_HAS_APP_ID | WORKSPACE_MEMBERSHIP_REQUIRED + LOADED_APP_IS_PRIVATE = data_cond(lambda data: data.app_access_mode == WebAppAccessMode.PRIVATE) diff --git a/api/controllers/openapi/auth/data.py b/api/controllers/openapi/auth/data.py index 30973b5e9b..76b0d90cb4 100644 --- a/api/controllers/openapi/auth/data.py +++ b/api/controllers/openapi/auth/data.py @@ -9,7 +9,7 @@ from werkzeug.exceptions import InternalServerError from configs import dify_config from libs.oauth_bearer import Scope, TokenType -from models.account import Account, Tenant +from models.account import Account, Tenant, TenantAccountRole from models.model import App, EndUser from services.enterprise.enterprise_service import WebAppAccessMode @@ -41,6 +41,8 @@ class RequestContext(BaseModel): token_type: TokenType scope: Scope | None = None path_params: dict[str, str] + workspace_membership: bool = False + allowed_roles: frozenset[TenantAccountRole] | None = None class AuthData(BaseModel): @@ -56,10 +58,14 @@ class AuthData(BaseModel): external_identity: ExternalIdentity | None = None path_params: dict[str, str] = Field(default_factory=dict) + allowed_roles: frozenset[TenantAccountRole] | None = None + app: App | None = None tenant: Tenant | None = None app_access_mode: WebAppAccessMode | None = None + tenant_role: TenantAccountRole | None = None + caller: Account | EndUser | None = None caller_kind: Literal["account", "end_user"] | None = None diff --git a/api/controllers/openapi/auth/pipeline.py b/api/controllers/openapi/auth/pipeline.py index e992e5e5ab..488a971b1e 100644 --- a/api/controllers/openapi/auth/pipeline.py +++ b/api/controllers/openapi/auth/pipeline.py @@ -34,6 +34,7 @@ from libs.oauth_bearer import ( reset_auth_ctx, set_auth_ctx, ) +from models.account import TenantAccountRole from services.feature_service import FeatureService, LicenseStatus @@ -56,11 +57,15 @@ class AuthPipeline: view: Callable, *, scope: Scope | None, + workspace_membership: bool = False, + allowed_roles: frozenset[TenantAccountRole] | None = None, ) -> Any: req_ctx = RequestContext( token_type=identity.token_type, scope=scope, path_params=dict(request.view_args or {}), + workspace_membership=workspace_membership, + allowed_roles=allowed_roles, ) data = AuthData( @@ -71,6 +76,7 @@ class AuthPipeline: scopes=frozenset(identity.scopes), tenants=dict(identity.verified_tenants), required_scope=scope, + allowed_roles=allowed_roles, path_params=dict(req_ctx.path_params), external_identity=( ExternalIdentity(email=identity.subject_email, issuer=identity.subject_issuer) @@ -121,6 +127,41 @@ class PipelineRouter: scope: Scope | None = None, allowed_token_types: frozenset[TokenType] | None = None, edition: frozenset[Edition] | None = None, + workspace_membership: bool = False, + allowed_roles: frozenset[TenantAccountRole] | None = None, + ) -> Callable: + return self._make_decorator( + scope=scope, + allowed_token_types=allowed_token_types, + edition=edition, + workspace_membership=workspace_membership, + allowed_roles=allowed_roles, + ) + + def guard_workspace( + self, + *, + scope: Scope | None = None, + allowed_token_types: frozenset[TokenType] | None = None, + edition: frozenset[Edition] | None = None, + allowed_roles: frozenset[TenantAccountRole] | None = None, + ) -> Callable: + return self._make_decorator( + scope=scope, + allowed_token_types=allowed_token_types, + edition=edition, + workspace_membership=True, + allowed_roles=allowed_roles, + ) + + def _make_decorator( + self, + *, + scope: Scope | None, + allowed_token_types: frozenset[TokenType] | None, + edition: frozenset[Edition] | None, + workspace_membership: bool, + allowed_roles: frozenset[TenantAccountRole] | None, ) -> Callable: def decorator(view: Callable) -> Callable: @wraps(view) @@ -132,6 +173,8 @@ class PipelineRouter: scope=scope, allowed_token_types=allowed_token_types, edition=edition, + workspace_membership=workspace_membership, + allowed_roles=allowed_roles, ) return decorated @@ -147,6 +190,8 @@ class PipelineRouter: scope: Scope | None, allowed_token_types: frozenset[TokenType] | None, edition: frozenset[Edition] | None, + workspace_membership: bool = False, + allowed_roles: frozenset[TenantAccountRole] | None = None, ) -> Any: # 404 not 403 — this edition doesn't expose the feature at all if edition is not None and current_edition() not in edition: @@ -182,7 +227,15 @@ class PipelineRouter: if not license_checked and Edition.EE in route.required_edition: _check_license() - return route.pipeline._run(identity, args, kwargs, view, scope=scope) + return route.pipeline._run( + identity, + args, + kwargs, + view, + scope=scope, + workspace_membership=workspace_membership, + allowed_roles=allowed_roles, + ) def _should_run(step: Any, req_ctx: RequestContext, data: AuthData | None) -> bool: diff --git a/api/controllers/openapi/auth/prepare.py b/api/controllers/openapi/auth/prepare.py index fe6e031b50..5ad8d0647b 100644 --- a/api/controllers/openapi/auth/prepare.py +++ b/api/controllers/openapi/auth/prepare.py @@ -1,5 +1,8 @@ from __future__ import annotations +import uuid + +from flask import request from werkzeug.exceptions import Forbidden, InternalServerError, NotFound, Unauthorized from controllers.openapi.auth.data import AuthData @@ -13,16 +16,18 @@ from services.enterprise.enterprise_service import EnterpriseService, WebAppAcce def load_app(data: AuthData) -> None: + if data.app is not None: + return app_id = data.path_params["app_id"] app = AppService.get_app_by_id(db.session, app_id) if not app or app.status != "normal": raise NotFound("app not found") - if not app.enable_api: - raise Forbidden("service_api_disabled") data.app = app def load_tenant(data: AuthData) -> None: + if data.tenant is not None: + return if data.app is None: raise InternalServerError("pipeline_invariant_violated: app not loaded before load_tenant") tenant = TenantService.get_tenant_by_id(db.session, str(data.app.tenant_id)) @@ -31,7 +36,25 @@ def load_tenant(data: AuthData) -> None: data.tenant = tenant +def load_tenant_from_request(data: AuthData) -> None: + if data.tenant is not None: + return + workspace_id = data.path_params.get("workspace_id") or request.args.get("workspace_id") + if not workspace_id: + raise NotFound("workspace not found") + try: + uuid.UUID(workspace_id) + except ValueError: + raise NotFound("workspace not found") + tenant = TenantService.get_tenant_by_id(db.session, workspace_id) + if tenant is None or tenant.status == TenantStatus.ARCHIVE: + raise NotFound("workspace not found") + data.tenant = tenant + + def load_account(data: AuthData) -> None: + if data.caller is not None: + return account = AccountService.get_account_by_id(db.session, str(data.account_id)) if account is None: raise Unauthorized("account not found") @@ -41,6 +64,19 @@ def load_account(data: AuthData) -> None: data.caller_kind = "account" +def load_workspace_role(data: AuthData) -> None: + if data.tenant_role is not None: + return + if data.tenant is None or data.account_id is None: + return + if data.caller is not None and getattr(data.caller, "status", None) != "active": + return + role = TenantService.get_account_role_in_tenant(db.session, str(data.account_id), str(data.tenant.id)) + if role is None: + return + data.tenant_role = role + + def resolve_external_user(data: AuthData) -> None: if data.tenant is None or data.app is None or data.external_identity is None: raise Unauthorized("missing context for external user resolution") diff --git a/api/controllers/openapi/auth/role_gate.py b/api/controllers/openapi/auth/role_gate.py deleted file mode 100644 index c5e266f63d..0000000000 --- a/api/controllers/openapi/auth/role_gate.py +++ /dev/null @@ -1,77 +0,0 @@ -"""Workspace role gate. - -Layered on top of `validate_bearer` + `accept_subjects(SubjectType.ACCOUNT)` -for routes whose access depends on the caller's `TenantAccountJoin.role` -in the workspace named by the `workspace_id` path parameter. - -Usage:: - - @openapi_ns.route("/workspaces//members") - class Members(Resource): - @validate_bearer(accept=ACCEPT_USER_ANY) - @accept_subjects(SubjectType.ACCOUNT) - @require_workspace_role() # any member - def get(self, workspace_id: str): ... - - @validate_bearer(accept=ACCEPT_USER_ANY) - @accept_subjects(SubjectType.ACCOUNT) - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) - def post(self, workspace_id: str): ... - -Non-member callers get 404 (matching `GET /openapi/v1/workspaces/`) -so workspace IDs do not leak across tenants. A member without one of the -allowed roles gets 403. -""" - -from __future__ import annotations - -from collections.abc import Callable -from functools import wraps -from typing import TypeVar - -from werkzeug.exceptions import Forbidden, NotFound - -from extensions.ext_database import db -from libs.oauth_bearer import try_get_auth_ctx -from models.account import TenantAccountRole -from services.account_service import TenantService - -F = TypeVar("F", bound=Callable[..., object]) - - -def require_workspace_role(*allowed_roles: TenantAccountRole) -> Callable[[F], F]: - """Gate a route on the caller's role in ``workspace_id``. - - Pass no roles to require only membership. Pass one or more roles to - require the caller's role be in that set. - """ - - allowed = frozenset(allowed_roles) - - def deco(fn: F) -> F: - @wraps(fn) - def wrapper(*args: object, **kwargs: object) -> object: - ctx = try_get_auth_ctx() - if ctx is None or ctx.account_id is None: - raise RuntimeError( - "require_workspace_role called without account-bearer context; " - "stack validate_bearer + accept_subjects(SubjectType.ACCOUNT) above it" - ) - - workspace_id = kwargs.get("workspace_id") - if not workspace_id: - raise RuntimeError("require_workspace_role expects a 'workspace_id' route parameter") - - role = TenantService.get_account_role_in_tenant(db.session, str(ctx.account_id), str(workspace_id)) - - if role is None: - raise NotFound("workspace not found") - - if allowed and role not in allowed: - raise Forbidden("insufficient workspace role") - - return fn(*args, **kwargs) - - return wrapper # type: ignore[return-value] - - return deco diff --git a/api/controllers/openapi/auth/verify.py b/api/controllers/openapi/auth/verify.py index 22410b3374..8cd7a30f5e 100644 --- a/api/controllers/openapi/auth/verify.py +++ b/api/controllers/openapi/auth/verify.py @@ -1,10 +1,11 @@ from __future__ import annotations -from werkzeug.exceptions import Forbidden, Unauthorized +from flask import request +from werkzeug.exceptions import Forbidden, NotFound, UnprocessableEntity from controllers.openapi.auth.data import AuthData from extensions.ext_database import db -from libs.oauth_bearer import Scope, TokenType, check_workspace_membership +from libs.oauth_bearer import Scope, TokenType from services.account_service import AccountService, TenantService from services.enterprise.enterprise_service import EnterpriseService, WebAppAccessMode @@ -17,17 +18,39 @@ def check_scope(data: AuthData) -> None: raise Forbidden("insufficient_scope") -def check_membership(data: AuthData) -> None: +def check_workspace_member(data: AuthData) -> None: + """Assert the caller belongs to the resolved tenant. + + `load_workspace_role` stashes the membership role (None when the caller is + not a member or is inactive). A missing membership surfaces as 404, not + 403, so workspace IDs don't leak across tenants. + """ + if data.tenant_role is None: + raise NotFound("workspace not found") + + +def check_workspace_mismatch(data: AuthData) -> None: if data.tenant is None: - raise Unauthorized("tenant unset") - if data.account_id is None: - raise Unauthorized("account_id unset") - check_workspace_membership( - account_id=data.account_id, - tenant_id=data.tenant.id, - token_hash=data.token_hash, - membership_cache=data.tenants, - ) + return + request_workspace_id = data.path_params.get("workspace_id") or request.args.get("workspace_id") + if request_workspace_id and request_workspace_id != str(data.tenant.id): + raise UnprocessableEntity("workspace_id does not match app's workspace") + + +def check_workspace_role(data: AuthData) -> None: + if data.allowed_roles is None: + return + if data.tenant_role is None: + raise NotFound("workspace not found") + if data.tenant_role not in data.allowed_roles: + raise Forbidden("insufficient workspace role") + + +def check_app_api_enabled(data: AuthData) -> None: + if data.app is None: + return + if not data.app.enable_api: + raise Forbidden("service_api_disabled") def check_app_access(data: AuthData) -> None: diff --git a/api/controllers/openapi/workspaces.py b/api/controllers/openapi/workspaces.py index b23012a810..7d01db8dc1 100644 --- a/api/controllers/openapi/workspaces.py +++ b/api/controllers/openapi/workspaces.py @@ -5,9 +5,8 @@ endpoints. Account bearers (dfoa_) see every tenant they're a member of. External SSO bearers (dfoe_) have no account_id and so see an empty list — that matches /openapi/v1/account. -Member-management endpoints are gated by both `accept_subjects` (SSO out) -and `require_workspace_role` (membership / role lookup against the path's -``workspace_id``). +Member-management endpoints use ``guard_workspace`` which enforces +workspace membership and optional role requirements via the auth pipeline. """ from __future__ import annotations @@ -37,7 +36,6 @@ from controllers.openapi._models import ( ) from controllers.openapi.auth.composition import auth_router from controllers.openapi.auth.data import AuthData -from controllers.openapi.auth.role_gate import require_workspace_role from extensions.ext_database import db from libs.oauth_bearer import Scope, TokenType from models import Account, Tenant, TenantAccountJoin @@ -152,8 +150,7 @@ class WorkspaceSwitchApi(Resource): """ @openapi_ns.response(200, "Workspace detail", openapi_ns.models[WorkspaceDetailResponse.__name__]) - @auth_router.guard(scope=Scope.WORKSPACE_READ, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) - @require_workspace_role() + @auth_router.guard_workspace(scope=Scope.WORKSPACE_READ, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) def post(self, workspace_id: str, *, auth_data: AuthData): account = _load_account(auth_data.account_id) @@ -179,8 +176,7 @@ class WorkspaceMembersApi(Resource): @openapi_ns.doc(params=query_params_from_model(MemberListQuery)) @openapi_ns.response(200, "Member list", openapi_ns.models[MemberListResponse.__name__]) - @auth_router.guard(scope=Scope.WORKSPACE_READ, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) - @require_workspace_role() + @auth_router.guard_workspace(scope=Scope.WORKSPACE_READ, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) def get(self, workspace_id: str, *, auth_data: AuthData): try: query = MemberListQuery.model_validate(request.args.to_dict(flat=True)) @@ -202,8 +198,11 @@ class WorkspaceMembersApi(Resource): @openapi_ns.expect(openapi_ns.models[MemberInvitePayload.__name__]) @openapi_ns.response(201, "Member invited", openapi_ns.models[MemberInviteResponse.__name__]) - @auth_router.guard(scope=Scope.WORKSPACE_WRITE, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) + @auth_router.guard_workspace( + scope=Scope.WORKSPACE_WRITE, + allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT}), + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN}), + ) def post(self, workspace_id: str, *, auth_data: AuthData): payload = _validate_body(MemberInvitePayload) inviter = _load_account(auth_data.account_id) @@ -253,8 +252,11 @@ class WorkspaceMemberApi(Resource): """ @openapi_ns.response(200, "Member removed", openapi_ns.models[MemberActionResponse.__name__]) - @auth_router.guard(scope=Scope.WORKSPACE_WRITE, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) + @auth_router.guard_workspace( + scope=Scope.WORKSPACE_WRITE, + allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT}), + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN}), + ) def delete(self, workspace_id: str, member_id: str, *, auth_data: AuthData): operator = _load_account(auth_data.account_id) tenant = _load_tenant(workspace_id) @@ -284,8 +286,11 @@ class WorkspaceMemberRoleApi(Resource): @openapi_ns.expect(openapi_ns.models[MemberRoleUpdatePayload.__name__]) @openapi_ns.response(200, "Role updated", openapi_ns.models[MemberActionResponse.__name__]) - @auth_router.guard(scope=Scope.WORKSPACE_WRITE, allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT})) - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) + @auth_router.guard_workspace( + scope=Scope.WORKSPACE_WRITE, + allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT}), + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN}), + ) def put(self, workspace_id: str, member_id: str, *, auth_data: AuthData): payload = _validate_body(MemberRoleUpdatePayload) operator = _load_account(auth_data.account_id) diff --git a/api/services/account_service.py b/api/services/account_service.py index 89a0a5d8e2..6eb35cb23b 100644 --- a/api/services/account_service.py +++ b/api/services/account_service.py @@ -1329,9 +1329,9 @@ class TenantService: ) -> TenantAccountRole | None: """Return the caller's role in ``tenant_id``, or ``None`` if not a member. - Backs ``controllers.openapi.auth.role_gate.require_workspace_role``: - the gate maps ``None`` to 404 (non-member — no cross-tenant ID leak) - and an out-of-set role to 403, so it never touches the ORM itself. + Backs the openapi auth pipeline's ``load_workspace_role`` prepare step: + ``None`` is treated as non-member (the pipeline maps it to 404 — no + cross-tenant ID leak) and an out-of-set role to 403. ``None``/empty ``account_id`` short-circuits to ``None`` so SSO bearers (no account) collapse to the non-member path. Mirrors the diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_composition.py b/api/tests/unit_tests/controllers/openapi/auth/test_composition.py index 028d32009d..6c4a38b5d2 100644 --- a/api/tests/unit_tests/controllers/openapi/auth/test_composition.py +++ b/api/tests/unit_tests/controllers/openapi/auth/test_composition.py @@ -1,7 +1,16 @@ +import uuid + from controllers.openapi.auth.composition import account_pipeline, auth_router, external_sso_pipeline +from controllers.openapi.auth.data import RequestContext from controllers.openapi.auth.flow import When from controllers.openapi.auth.pipeline import AuthPipeline, PipelineRoute, PipelineRouter +from controllers.openapi.auth.verify import ( + check_workspace_member, + check_workspace_mismatch, + check_workspace_role, +) from libs.oauth_bearer import TokenType +from models.account import TenantAccountRole def test_account_pipeline_is_auth_pipeline(): @@ -16,20 +25,20 @@ def test_auth_router_is_pipeline_router(): assert isinstance(auth_router, PipelineRouter) -def test_account_pipeline_prepare_has_four_entries(): - assert len(account_pipeline._prepare) == 4 +def test_account_pipeline_prepare_has_six_entries(): + assert len(account_pipeline._prepare) == 6 -def test_account_auth_list_has_five_entries(): - assert len(account_pipeline._auth) == 5 +def test_account_auth_list_has_seven_entries(): + assert len(account_pipeline._auth) == 7 def test_external_sso_pipeline_prepare_has_four_entries(): assert len(external_sso_pipeline._prepare) == 4 -def test_external_sso_auth_list_has_three_entries(): - assert len(external_sso_pipeline._auth) == 3 +def test_external_sso_auth_list_has_four_entries(): + assert len(external_sso_pipeline._auth) == 4 def test_account_pipeline_has_unconditional_load_account(): @@ -41,17 +50,14 @@ def test_external_sso_pipeline_all_prepare_entries_are_when(): assert all(isinstance(s, When) for s in external_sso_pipeline._prepare) -def test_first_auth_entry_is_check_scope_in_both_pipelines(): - assert not isinstance(account_pipeline._auth[0], When) - assert not isinstance(external_sso_pipeline._auth[0], When) +def test_account_pipeline_has_one_unconditional_auth_step(): + non_when = [s for s in account_pipeline._auth if not isinstance(s, When)] + assert len(non_when) == 1 -def test_remaining_auth_entries_are_when_for_account(): - assert all(isinstance(s, When) for s in account_pipeline._auth[1:]) - - -def test_remaining_auth_entries_are_when_for_external_sso(): - assert all(isinstance(s, When) for s in external_sso_pipeline._auth[1:]) +def test_external_sso_pipeline_has_one_unconditional_auth_step(): + non_when = [s for s in external_sso_pipeline._auth if not isinstance(s, When)] + assert len(non_when) == 1 def test_router_routes_contain_both_token_types(): @@ -71,3 +77,58 @@ def test_account_route_has_no_required_edition(): route = auth_router._routes[TokenType.OAUTH_ACCOUNT] assert isinstance(route, PipelineRoute) assert route.required_edition is None + + +def _selected_auth_steps(*, app_id: bool, workspace_membership: bool, allowed_roles): + ctx = RequestContext( + token_type=TokenType.OAUTH_ACCOUNT, + scope=None, + path_params={"app_id": str(uuid.uuid4())} if app_id else {}, + workspace_membership=workspace_membership, + allowed_roles=allowed_roles, + ) + selected = [] + for step in account_pipeline._auth: + if isinstance(step, When): + if step.applies(ctx, None): + selected.append(step._step) + else: + selected.append(step) + return selected + + +_ALL_ROLES = frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.NORMAL}) + + +def test_workspace_path_selects_membership_check(): + steps = _selected_auth_steps(app_id=False, workspace_membership=True, allowed_roles=None) + assert check_workspace_member in steps + assert check_workspace_role not in steps + + +def test_app_path_selects_membership_check(): + steps = _selected_auth_steps(app_id=True, workspace_membership=False, allowed_roles=None) + assert check_workspace_member in steps + assert check_workspace_role not in steps + + +def test_roles_set_selects_both_membership_and_role_check(): + steps = _selected_auth_steps(app_id=False, workspace_membership=True, allowed_roles=_ALL_ROLES) + assert check_workspace_member in steps + assert check_workspace_role in steps + + +def test_plain_path_selects_no_membership_or_role_step(): + steps = _selected_auth_steps(app_id=False, workspace_membership=False, allowed_roles=None) + assert check_workspace_member not in steps + assert check_workspace_role not in steps + + +def test_app_path_selects_workspace_mismatch_check(): + steps = _selected_auth_steps(app_id=True, workspace_membership=False, allowed_roles=None) + assert check_workspace_mismatch in steps + + +def test_workspace_path_skips_workspace_mismatch_check(): + steps = _selected_auth_steps(app_id=False, workspace_membership=True, allowed_roles=None) + assert check_workspace_mismatch not in steps diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_conditions.py b/api/tests/unit_tests/controllers/openapi/auth/test_conditions.py index 8367933984..159e5b4216 100644 --- a/api/tests/unit_tests/controllers/openapi/auth/test_conditions.py +++ b/api/tests/unit_tests/controllers/openapi/auth/test_conditions.py @@ -4,11 +4,13 @@ from controllers.openapi.auth.conditions import ( EDITION_CE, EDITION_EE, EDITION_SAAS, + HAS_ALLOWED_ROLES, LOADED_APP_IS_PRIVATE, PATH_HAS_APP_ID, TOKEN_IS_OAUTH_ACCOUNT, TOKEN_IS_OAUTH_EXTERNAL_SSO, WEBAPP_AUTH_ENABLED, + WORKSPACE_MEMBERSHIP_REQUIRED, Cond, config_cond, data_cond, @@ -16,13 +18,15 @@ from controllers.openapi.auth.conditions import ( ) from controllers.openapi.auth.data import AuthData, Edition, RequestContext from libs.oauth_bearer import TokenType +from models.account import TenantAccountRole from services.enterprise.enterprise_service import WebAppAccessMode -def _ctx(token_type=TokenType.OAUTH_ACCOUNT, path_params=None): +def _ctx(token_type=TokenType.OAUTH_ACCOUNT, path_params=None, **kwargs): return RequestContext( token_type=token_type, path_params=path_params or {}, + **kwargs, ) @@ -141,3 +145,28 @@ def test_loaded_app_is_private(): assert LOADED_APP_IS_PRIVATE(_ctx(), data_public) is False assert LOADED_APP_IS_PRIVATE(_ctx(), data_none) is False assert LOADED_APP_IS_PRIVATE(_ctx(), None) is False + + +def test_workspace_membership_required_true(): + assert WORKSPACE_MEMBERSHIP_REQUIRED(_ctx(workspace_membership=True)) is True + + +def test_workspace_membership_required_false(): + assert WORKSPACE_MEMBERSHIP_REQUIRED(_ctx(workspace_membership=False)) is False + + +def test_workspace_membership_required_default(): + assert WORKSPACE_MEMBERSHIP_REQUIRED(_ctx()) is False + + +def test_has_allowed_roles_true(): + ctx = _ctx(allowed_roles=frozenset({TenantAccountRole.OWNER})) + assert HAS_ALLOWED_ROLES(ctx) is True + + +def test_has_allowed_roles_false(): + assert HAS_ALLOWED_ROLES(_ctx(allowed_roles=None)) is False + + +def test_has_allowed_roles_default(): + assert HAS_ALLOWED_ROLES(_ctx()) is False diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_data.py b/api/tests/unit_tests/controllers/openapi/auth/test_data.py index c39ed9c6d0..14167ec3ac 100644 --- a/api/tests/unit_tests/controllers/openapi/auth/test_data.py +++ b/api/tests/unit_tests/controllers/openapi/auth/test_data.py @@ -115,3 +115,69 @@ def test_auth_data_token_id_optional(): scopes=frozenset(), ) assert data.token_id is None + + +def test_request_context_workspace_membership_default_false(): + ctx = RequestContext(token_type=TokenType.OAUTH_ACCOUNT, path_params={}) + assert ctx.workspace_membership is False + + +def test_request_context_workspace_membership_set(): + ctx = RequestContext(token_type=TokenType.OAUTH_ACCOUNT, path_params={}, workspace_membership=True) + assert ctx.workspace_membership is True + + +def test_request_context_allowed_roles_default_none(): + ctx = RequestContext(token_type=TokenType.OAUTH_ACCOUNT, path_params={}) + assert ctx.allowed_roles is None + + +def test_request_context_allowed_roles_set(): + from models.account import TenantAccountRole + + roles = frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN}) + ctx = RequestContext(token_type=TokenType.OAUTH_ACCOUNT, path_params={}, allowed_roles=roles) + assert ctx.allowed_roles == roles + + +def test_auth_data_allowed_roles_default_none(): + data = AuthData( + token_type=TokenType.OAUTH_ACCOUNT, + token_hash="abc", + scopes=frozenset(), + ) + assert data.allowed_roles is None + + +def test_auth_data_allowed_roles_set(): + from models.account import TenantAccountRole + + roles = frozenset({TenantAccountRole.ADMIN}) + data = AuthData( + token_type=TokenType.OAUTH_ACCOUNT, + token_hash="abc", + scopes=frozenset(), + allowed_roles=roles, + ) + assert data.allowed_roles == roles + + +def test_auth_data_tenant_role_default_none(): + data = AuthData( + token_type=TokenType.OAUTH_ACCOUNT, + token_hash="abc", + scopes=frozenset(), + ) + assert data.tenant_role is None + + +def test_auth_data_tenant_role_set(): + from models.account import TenantAccountRole + + data = AuthData( + token_type=TokenType.OAUTH_ACCOUNT, + token_hash="abc", + scopes=frozenset(), + tenant_role=TenantAccountRole.ADMIN, + ) + assert data.tenant_role == TenantAccountRole.ADMIN diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_pipeline.py b/api/tests/unit_tests/controllers/openapi/auth/test_pipeline.py index a92f90112f..37b400b92d 100644 --- a/api/tests/unit_tests/controllers/openapi/auth/test_pipeline.py +++ b/api/tests/unit_tests/controllers/openapi/auth/test_pipeline.py @@ -247,6 +247,60 @@ def test_guard_populates_external_identity_from_subject_email(app): assert received["data"].external_identity.issuer == "https://idp.example.com" +def test_guard_workspace_sets_membership_and_roles(app): + from models.account import TenantAccountRole + + router = _make_router() + received = {} + + with app.test_request_context("/test", headers={"Authorization": "Bearer tok"}): + with ( + patch("controllers.openapi.auth.pipeline.extract_bearer", return_value="tok"), + patch("controllers.openapi.auth.pipeline.get_authenticator") as mock_auth, + patch("controllers.openapi.auth.pipeline.set_auth_ctx", return_value=MagicMock()), + patch("controllers.openapi.auth.pipeline.reset_auth_ctx"), + ): + mock_auth.return_value.authenticate.return_value = _fake_identity() + + roles = frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN}) + + @router.guard_workspace( + scope=Scope.FULL, + allowed_token_types=frozenset({TokenType.OAUTH_ACCOUNT}), + allowed_roles=roles, + ) + def view(*, auth_data): + received["data"] = auth_data + + view() + + assert isinstance(received["data"], AuthData) + assert received["data"].allowed_roles == roles + + +def test_guard_workspace_without_roles(app): + router = _make_router() + received = {} + + with app.test_request_context("/test", headers={"Authorization": "Bearer tok"}): + with ( + patch("controllers.openapi.auth.pipeline.extract_bearer", return_value="tok"), + patch("controllers.openapi.auth.pipeline.get_authenticator") as mock_auth, + patch("controllers.openapi.auth.pipeline.set_auth_ctx", return_value=MagicMock()), + patch("controllers.openapi.auth.pipeline.reset_auth_ctx"), + ): + mock_auth.return_value.authenticate.return_value = _fake_identity() + + @router.guard_workspace(scope=Scope.FULL) + def view(*, auth_data): + received["data"] = auth_data + + view() + + assert isinstance(received["data"], AuthData) + assert received["data"].allowed_roles is None + + def test_guard_no_external_identity_when_subject_email_absent(app): router = _make_router() received = {} diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_prepare.py b/api/tests/unit_tests/controllers/openapi/auth/test_prepare.py index 39d8aafa0e..dd061ebf48 100644 --- a/api/tests/unit_tests/controllers/openapi/auth/test_prepare.py +++ b/api/tests/unit_tests/controllers/openapi/auth/test_prepare.py @@ -2,6 +2,7 @@ import uuid from unittest.mock import MagicMock, patch import pytest +from flask import Flask from werkzeug.exceptions import Forbidden, NotFound, Unauthorized from controllers.openapi.auth.data import AuthData, ExternalIdentity @@ -10,9 +11,12 @@ from controllers.openapi.auth.prepare import ( load_app, load_app_access_mode, load_tenant, + load_tenant_from_request, + load_workspace_role, resolve_external_user, ) from libs.oauth_bearer import TokenType +from models.account import TenantAccountRole def _make_auth_data(**kwargs) -> AuthData: @@ -54,14 +58,21 @@ def test_load_app_raises_not_found_when_not_normal(): load_app(data) -def test_load_app_raises_forbidden_when_api_disabled(): +def test_load_app_stashes_app_even_when_api_disabled(): app = MagicMock() app.status = "normal" app.enable_api = False data = _make_auth_data(path_params={"app_id": "abc"}) with patch("controllers.openapi.auth.prepare.AppService.get_app_by_id", return_value=app): - with pytest.raises(Forbidden): - load_app(data) + load_app(data) + assert data.app is app + + +def test_load_app_skips_when_already_set(): + existing_app = MagicMock() + data = _make_auth_data(app=existing_app, path_params={"app_id": "abc"}) + load_app(data) + assert data.app is existing_app def test_load_tenant_writes_tenant(): @@ -75,6 +86,13 @@ def test_load_tenant_writes_tenant(): assert data.tenant is tenant +def test_load_tenant_skips_when_already_set(): + existing_tenant = MagicMock() + data = _make_auth_data(app=MagicMock(), tenant=existing_tenant) + load_tenant(data) + assert data.tenant is existing_tenant + + def test_load_tenant_raises_forbidden_when_archived(): from models.account import TenantStatus @@ -115,6 +133,13 @@ def test_load_account_writes_caller(): assert data.caller_kind == "account" +def test_load_account_skips_when_already_set(): + existing_caller = MagicMock() + data = _make_auth_data(account_id=uuid.uuid4(), caller=existing_caller) + load_account(data) + assert data.caller is existing_caller + + def test_load_account_sets_current_tenant_when_tenant_present(): account = MagicMock() tenant = MagicMock() @@ -181,3 +206,143 @@ def test_load_app_access_mode_no_op_when_app_missing(): data = _make_auth_data() load_app_access_mode(data) assert data.app_access_mode is None + + +@pytest.fixture +def flask_app(): + return Flask(__name__) + + +def test_load_tenant_from_request_from_path_params(flask_app): + tenant = MagicMock() + tenant.status = "normal" + wid = str(uuid.uuid4()) + data = _make_auth_data(path_params={"workspace_id": wid}) + with flask_app.test_request_context("/test"): + with patch("controllers.openapi.auth.prepare.TenantService.get_tenant_by_id", return_value=tenant): + load_tenant_from_request(data) + assert data.tenant is tenant + + +def test_load_tenant_from_request_from_query_param(flask_app): + tenant = MagicMock() + tenant.status = "normal" + wid = str(uuid.uuid4()) + data = _make_auth_data(path_params={}) + with flask_app.test_request_context(f"/test?workspace_id={wid}"): + with patch("controllers.openapi.auth.prepare.TenantService.get_tenant_by_id", return_value=tenant): + load_tenant_from_request(data) + assert data.tenant is tenant + + +def test_load_tenant_from_request_skips_when_already_set(flask_app): + existing_tenant = MagicMock() + data = _make_auth_data(tenant=existing_tenant, path_params={}) + with flask_app.test_request_context("/test"): + load_tenant_from_request(data) + assert data.tenant is existing_tenant + + +def test_load_tenant_from_request_raises_not_found_when_no_id(flask_app): + data = _make_auth_data(path_params={}) + with flask_app.test_request_context("/test"): + with pytest.raises(NotFound): + load_tenant_from_request(data) + + +def test_load_tenant_from_request_raises_not_found_when_missing(flask_app): + wid = str(uuid.uuid4()) + data = _make_auth_data(path_params={"workspace_id": wid}) + with flask_app.test_request_context("/test"): + with patch("controllers.openapi.auth.prepare.TenantService.get_tenant_by_id", return_value=None): + with pytest.raises(NotFound): + load_tenant_from_request(data) + + +def test_load_tenant_from_request_raises_not_found_when_archived(flask_app): + from models.account import TenantStatus + + tenant = MagicMock() + tenant.status = TenantStatus.ARCHIVE + wid = str(uuid.uuid4()) + data = _make_auth_data(path_params={"workspace_id": wid}) + with flask_app.test_request_context("/test"): + with patch("controllers.openapi.auth.prepare.TenantService.get_tenant_by_id", return_value=tenant): + with pytest.raises(NotFound): + load_tenant_from_request(data) + + +def test_load_tenant_from_request_raises_not_found_when_invalid_uuid(flask_app): + data = _make_auth_data(path_params={"workspace_id": "not-a-uuid"}) + with flask_app.test_request_context("/test"): + with pytest.raises(NotFound): + load_tenant_from_request(data) + + +# --- load_workspace_role --- + + +def test_load_workspace_role_stashes_role(): + tenant = MagicMock() + tenant.id = uuid.uuid4() + caller = MagicMock() + caller.status = "active" + data = _make_auth_data(account_id=uuid.uuid4(), tenant=tenant, caller=caller) + with patch( + "controllers.openapi.auth.prepare.TenantService.get_account_role_in_tenant", + return_value=TenantAccountRole.ADMIN, + ): + load_workspace_role(data) + assert data.tenant_role == TenantAccountRole.ADMIN + + +def test_load_workspace_role_none_when_not_member(): + tenant = MagicMock() + tenant.id = uuid.uuid4() + caller = MagicMock() + caller.status = "active" + data = _make_auth_data(account_id=uuid.uuid4(), tenant=tenant, caller=caller) + with patch( + "controllers.openapi.auth.prepare.TenantService.get_account_role_in_tenant", + return_value=None, + ): + load_workspace_role(data) + assert data.tenant_role is None + + +def test_load_workspace_role_none_when_account_inactive(): + tenant = MagicMock() + tenant.id = uuid.uuid4() + caller = MagicMock() + caller.status = "banned" + data = _make_auth_data(account_id=uuid.uuid4(), tenant=tenant, caller=caller) + load_workspace_role(data) + assert data.tenant_role is None + + +def test_load_workspace_role_skips_when_already_set(): + tenant = MagicMock() + tenant.id = uuid.uuid4() + caller = MagicMock() + caller.status = "active" + data = _make_auth_data( + account_id=uuid.uuid4(), + tenant=tenant, + caller=caller, + tenant_role=TenantAccountRole.OWNER, + ) + load_workspace_role(data) + assert data.tenant_role == TenantAccountRole.OWNER + + +def test_load_workspace_role_skips_when_tenant_missing(): + data = _make_auth_data(account_id=uuid.uuid4()) + load_workspace_role(data) + assert data.tenant_role is None + + +def test_load_workspace_role_skips_when_account_id_missing(): + tenant = MagicMock() + data = _make_auth_data(tenant=tenant, account_id=None) + load_workspace_role(data) + assert data.tenant_role is None diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_role_gate.py b/api/tests/unit_tests/controllers/openapi/auth/test_role_gate.py deleted file mode 100644 index 68b436e824..0000000000 --- a/api/tests/unit_tests/controllers/openapi/auth/test_role_gate.py +++ /dev/null @@ -1,330 +0,0 @@ -"""Role-gate tests. - -The decorator wraps `validate_bearer` + `accept_subjects` and must: -- 404 when caller is not a member of ``workspace_id`` (parity with - `GET /openapi/v1/workspaces/`; prevents tenant-id existence leak) -- 403 when caller IS a member but their role is not in the allowed set -- pass through when role matches (or when no role restriction given) -- raise RuntimeError on missing auth context / account_id / workspace_id — - those are wiring bugs, not user-driven failures - -Identity is read from the openapi auth ContextVar — the slot -`validate_bearer` publishes — so these tests seed it via `_seed` -(``set_auth_ctx``), NOT ``flask.g``. `test_seeding_only_flask_g_*` -locks in that ``g`` is *not* a valid identity source. -""" - -from __future__ import annotations - -import uuid -from contextlib import contextmanager -from datetime import UTC, datetime -from unittest.mock import patch - -import pytest -from flask import Flask -from werkzeug.exceptions import Forbidden, NotFound - -from controllers.openapi.auth.role_gate import require_workspace_role -from libs.oauth_bearer import AuthContext, Scope, SubjectType, TokenType, reset_auth_ctx, set_auth_ctx -from models.account import TenantAccountRole - -# Tokens from `_seed`'s `set_auth_ctx` calls, drained after each test so a -# published identity can't leak into the next (the ContextVar is module-global -# and worker threads are reused). Seed via `_seed(...)`, never `flask.g`. -_seed_tokens: list = [] - - -def _seed(ctx: AuthContext) -> None: - _seed_tokens.append(set_auth_ctx(ctx)) - - -@pytest.fixture(autouse=True) -def _reset_auth_ctx(): - yield - while _seed_tokens: - reset_auth_ctx(_seed_tokens.pop()) - - -def _account_ctx(account_id: uuid.UUID | None = None) -> AuthContext: - return AuthContext( - subject_type=SubjectType.ACCOUNT, - subject_email="user@example.com", - subject_issuer="dify:account", - account_id=account_id or uuid.uuid4(), - client_id="difyctl", - scopes=frozenset({Scope.FULL}), - token_id=uuid.uuid4(), - token_type=TokenType.OAUTH_ACCOUNT, - expires_at=datetime.now(UTC), - token_hash="h1", - verified_tenants={}, - ) - - -def _sso_ctx() -> AuthContext: - return AuthContext( - subject_type=SubjectType.EXTERNAL_SSO, - subject_email="sso@partner.com", - subject_issuer="https://idp.partner.com", - account_id=None, - client_id="difyctl", - scopes=frozenset({Scope.APPS_RUN}), - token_id=uuid.uuid4(), - token_type=TokenType.OAUTH_EXTERNAL_SSO, - expires_at=datetime.now(UTC), - token_hash="h2", - verified_tenants={}, - ) - - -@contextmanager -def _stub_role(role: TenantAccountRole | None): - """Stub the service-layer membership lookup the gate delegates to. - - The gate no longer issues SQL itself — it calls - ``TenantService.get_account_role_in_tenant`` and acts purely on the - returned role (``None`` → non-member). These tests pin that behaviour; - the query itself is covered in ``TestTenantService``. - """ - with patch( - "controllers.openapi.auth.role_gate.TenantService.get_account_role_in_tenant", - return_value=role, - ) as mocked: - yield mocked - - -# --------------------------------------------------------------------------- -# Non-member → 404 -# --------------------------------------------------------------------------- - - -def test_non_member_gets_404(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - _seed(_account_ctx()) - with _stub_role(None): - with pytest.raises(NotFound): - view(workspace_id=workspace_id) - - -# --------------------------------------------------------------------------- -# Member with insufficient role → 403 -# --------------------------------------------------------------------------- - - -def test_normal_member_blocked_when_admin_required(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/members"): - _seed(_account_ctx()) - with _stub_role(TenantAccountRole.NORMAL): - with pytest.raises(Forbidden): - view(workspace_id=workspace_id) - - -def test_editor_blocked_when_admin_required(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/members"): - _seed(_account_ctx()) - with _stub_role(TenantAccountRole.EDITOR): - with pytest.raises(Forbidden): - view(workspace_id=workspace_id) - - -# --------------------------------------------------------------------------- -# Member with allowed role → pass -# --------------------------------------------------------------------------- - - -def test_admin_passes_when_admin_required(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/members"): - _seed(_account_ctx()) - with _stub_role(TenantAccountRole.ADMIN): - assert view(workspace_id=workspace_id) == "ok" - - -def test_owner_passes_when_admin_required(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role(TenantAccountRole.OWNER, TenantAccountRole.ADMIN) - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/members"): - _seed(_account_ctx()) - with _stub_role(TenantAccountRole.OWNER): - assert view(workspace_id=workspace_id) == "ok" - - -# --------------------------------------------------------------------------- -# Membership-only (no role restriction) -# --------------------------------------------------------------------------- - - -def test_membership_only_passes_for_any_role(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - for role in ( - TenantAccountRole.OWNER, - TenantAccountRole.ADMIN, - TenantAccountRole.EDITOR, - TenantAccountRole.NORMAL, - TenantAccountRole.DATASET_OPERATOR, - ): - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - _seed(_account_ctx()) - with _stub_role(role): - assert view(workspace_id=workspace_id) == "ok" - - -def test_membership_only_still_404s_non_member(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - _seed(_account_ctx()) - with _stub_role(None): - with pytest.raises(NotFound): - view(workspace_id=workspace_id) - - -# --------------------------------------------------------------------------- -# Lookup is scoped to the caller's account_id and the URL workspace_id -# --------------------------------------------------------------------------- - - -def test_lookup_is_scoped_to_caller_and_workspace(): - """The decorator must delegate the lookup keyed on - `(caller's account_id, URL workspace_id)` — otherwise a member of - workspace A could quietly hit endpoints for workspace B. Assert the - exact arguments handed to the service; the SQL those arguments compile - to is pinned in ``TestTenantService.test_get_account_role_in_tenant_*``. - """ - - app = Flask(__name__) - account_id = uuid.uuid4() - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - _seed(_account_ctx(account_id=account_id)) - with _stub_role(TenantAccountRole.NORMAL) as mocked: - view(workspace_id=workspace_id) - - _session, passed_account_id, passed_workspace_id = mocked.call_args.args - assert passed_account_id == str(account_id) - assert passed_workspace_id == workspace_id - - -# --------------------------------------------------------------------------- -# Wiring bugs surface as RuntimeError (loud), not 403 (silent) -# --------------------------------------------------------------------------- - - -def test_missing_auth_ctx_is_runtime_error(): - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - with pytest.raises(RuntimeError): - view(workspace_id=workspace_id) - - -def test_seeding_only_flask_g_does_not_satisfy_gate(): - """Regression — pins the identity source to the ContextVar, not ``flask.g``. - - Production fills the ContextVar (``validate_bearer`` → ``set_auth_ctx``) - and never touches ``g.auth_ctx``. An earlier revision of this gate read - ``g.auth_ctx``, so every real request raised RuntimeError → 500 while the - suite stayed green (it seeded ``g`` directly). Here we seed ONLY ``g`` and - leave the ContextVar empty: the gate must still raise, proving it does not - accept ``g`` as an identity source. Reading ``g`` again would let the - membership lookup run (stubbed to succeed) and this would fail. - """ - from flask import g - - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - g.auth_ctx = _account_ctx() # the wrong slot — must be ignored - with _stub_role(TenantAccountRole.OWNER): - with pytest.raises(RuntimeError): - view(workspace_id=workspace_id) - - -def test_sso_caller_is_runtime_error(): - """External SSO context has account_id=None — the caller stacked the - role gate without `accept_subjects(SubjectType.ACCOUNT)`. That's a - wiring bug, surface it as RuntimeError rather than 404 the SSO user.""" - - app = Flask(__name__) - workspace_id = str(uuid.uuid4()) - - @require_workspace_role() - def view(workspace_id: str) -> str: - return "ok" - - with app.test_request_context(f"/openapi/v1/workspaces/{workspace_id}/switch"): - _seed(_sso_ctx()) - with pytest.raises(RuntimeError): - view(workspace_id=workspace_id) - - -def test_missing_workspace_id_kwarg_is_runtime_error(): - app = Flask(__name__) - - @require_workspace_role() - def view() -> str: - return "ok" - - with app.test_request_context("/openapi/v1/foo"): - _seed(_account_ctx()) - with pytest.raises(RuntimeError): - view() diff --git a/api/tests/unit_tests/controllers/openapi/auth/test_verify.py b/api/tests/unit_tests/controllers/openapi/auth/test_verify.py index c7e0cd7402..96de1a2eda 100644 --- a/api/tests/unit_tests/controllers/openapi/auth/test_verify.py +++ b/api/tests/unit_tests/controllers/openapi/auth/test_verify.py @@ -2,18 +2,22 @@ import uuid from unittest.mock import MagicMock, patch import pytest -from werkzeug.exceptions import Forbidden, Unauthorized +from flask import Flask +from werkzeug.exceptions import Forbidden, NotFound from controllers.openapi.auth.data import AuthData from controllers.openapi.auth.verify import ( check_acl, check_app_access, - check_membership, + check_app_api_enabled, check_private_app_permission, check_scope, + check_workspace_member, + check_workspace_mismatch, + check_workspace_role, ) from libs.oauth_bearer import Scope, TokenType -from models.account import Tenant +from models.account import Tenant, TenantAccountRole from models.model import App from services.enterprise.enterprise_service import WebAppAccessMode @@ -41,28 +45,13 @@ def test_check_scope_raises_forbidden_when_scope_missing(): check_scope(_data(required_scope=Scope.APPS_RUN, scopes=frozenset({Scope.APPS_READ}))) -def test_check_membership_raises_unauthorized_when_tenant_none(): - with pytest.raises(Unauthorized): - check_membership(_data(tenant=None)) +def test_check_workspace_member_raises_not_found_when_no_role(): + with pytest.raises(NotFound, match="workspace not found"): + check_workspace_member(_data(tenant_role=None)) -def test_check_membership_calls_check_workspace_membership(): - tenant = MagicMock(spec=Tenant) - tenant.id = "tenant-1" - data = _data( - account_id=uuid.uuid4(), - token_hash="myhash", - tenants={"tenant-1": True}, - tenant=tenant, - ) - with patch("controllers.openapi.auth.verify.check_workspace_membership") as mock_cwm: - check_membership(data) - mock_cwm.assert_called_once_with( - account_id=data.account_id, - tenant_id="tenant-1", - token_hash="myhash", - membership_cache=data.tenants, - ) +def test_check_workspace_member_passes_when_role_present(): + check_workspace_member(_data(tenant_role=TenantAccountRole.NORMAL)) def test_check_app_access_passes_when_tenant_none(): @@ -140,3 +129,92 @@ def test_check_private_app_permission_passes_when_allowed(): target = "controllers.openapi.auth.verify.EnterpriseService.WebAppAuth.is_user_allowed_to_access_webapp" with patch(target, return_value=True): check_private_app_permission(data) + + +# --- check_workspace_mismatch --- + + +@pytest.fixture +def flask_app(): + return Flask(__name__) + + +def test_check_workspace_mismatch_passes_when_tenant_none(flask_app): + with flask_app.test_request_context("/test"): + check_workspace_mismatch(_data(tenant=None)) + + +def test_check_workspace_mismatch_passes_when_ids_match(flask_app): + tenant = MagicMock(spec=Tenant) + tid = uuid.uuid4() + tenant.id = tid + with flask_app.test_request_context(f"/test?workspace_id={tid}"): + check_workspace_mismatch(_data(tenant=tenant, path_params={})) + + +def test_check_workspace_mismatch_raises_422_on_mismatch(flask_app): + from werkzeug.exceptions import UnprocessableEntity + + tenant = MagicMock(spec=Tenant) + tenant.id = uuid.uuid4() + other_id = uuid.uuid4() + with flask_app.test_request_context(f"/test?workspace_id={other_id}"): + with pytest.raises(UnprocessableEntity): + check_workspace_mismatch(_data(tenant=tenant, path_params={})) + + +def test_check_workspace_mismatch_passes_when_no_request_workspace_id(flask_app): + tenant = MagicMock(spec=Tenant) + tenant.id = uuid.uuid4() + with flask_app.test_request_context("/test"): + check_workspace_mismatch(_data(tenant=tenant, path_params={})) + + +# --- check_workspace_role --- + + +def test_check_workspace_role_passes_when_allowed_roles_none(): + check_workspace_role(_data(allowed_roles=None)) + + +def test_check_workspace_role_raises_not_found_when_not_member(): + data = _data(tenant_role=None, allowed_roles=frozenset({TenantAccountRole.ADMIN})) + with pytest.raises(NotFound): + check_workspace_role(data) + + +def test_check_workspace_role_raises_forbidden_when_wrong_role(): + data = _data( + tenant_role=TenantAccountRole.EDITOR, + allowed_roles=frozenset({TenantAccountRole.OWNER}), + ) + with pytest.raises(Forbidden, match="insufficient workspace role"): + check_workspace_role(data) + + +def test_check_workspace_role_passes_when_role_allowed(): + data = _data( + tenant_role=TenantAccountRole.ADMIN, + allowed_roles=frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN}), + ) + check_workspace_role(data) + + +# --- check_app_api_enabled --- + + +def test_check_app_api_enabled_passes_when_enabled(): + app = MagicMock(spec=App) + app.enable_api = True + check_app_api_enabled(_data(app=app)) + + +def test_check_app_api_enabled_raises_forbidden_when_disabled(): + app = MagicMock(spec=App) + app.enable_api = False + with pytest.raises(Forbidden, match="service_api_disabled"): + check_app_api_enabled(_data(app=app)) + + +def test_check_app_api_enabled_passes_when_app_none(): + check_app_api_enabled(_data(app=None)) diff --git a/api/tests/unit_tests/controllers/openapi/conftest.py b/api/tests/unit_tests/controllers/openapi/conftest.py index 18b3b2fabf..70302810d2 100644 --- a/api/tests/unit_tests/controllers/openapi/conftest.py +++ b/api/tests/unit_tests/controllers/openapi/conftest.py @@ -9,7 +9,18 @@ from controllers.openapi.auth.pipeline import PipelineRouter from libs.oauth_bearer import Scope, TokenType -def _stub_execute(self, args, kwargs, view, *, scope=None, allowed_token_types=None, edition=None): +def _stub_execute( + self, + args, + kwargs, + view, + *, + scope=None, + allowed_token_types=None, + edition=None, + workspace_membership=False, + allowed_roles=None, +): """Bypass all auth logic; inject minimal AuthData and call the view directly.""" kwargs["auth_data"] = AuthData( token_type=TokenType.OAUTH_ACCOUNT, @@ -18,6 +29,7 @@ def _stub_execute(self, args, kwargs, view, *, scope=None, allowed_token_types=N token_id=uuid.uuid4(), scopes=frozenset({Scope.FULL}), required_scope=scope, + allowed_roles=allowed_roles, ) return view(*args, **kwargs) diff --git a/api/tests/unit_tests/controllers/openapi/test_workspaces_members.py b/api/tests/unit_tests/controllers/openapi/test_workspaces_members.py index 6e32487348..548b58286e 100644 --- a/api/tests/unit_tests/controllers/openapi/test_workspaces_members.py +++ b/api/tests/unit_tests/controllers/openapi/test_workspaces_members.py @@ -9,9 +9,10 @@ Coverage: Auth-pipeline plumbing is bypassed via the `bypass_pipeline` fixture from conftest.py; the bearer identity is seeded into the openapi auth ContextVar -via `_seed` (the slot `validate_bearer` publishes), and the role gate's DB -lookup is mocked. Tests that exercise endpoint *bodies* skip the decorators -via ``__wrapped__`` since those layers are covered in `auth/test_role_gate.py`. +via `_seed` (the slot `validate_bearer` publishes). Tests that exercise +endpoint *bodies* skip the single `guard_workspace` decorator via +``__wrapped__`` — membership and role enforcement live in the auth pipeline +and are covered in `auth/test_prepare.py` and `auth/test_verify.py`. """ from __future__ import annotations @@ -268,7 +269,7 @@ def test_switch_returns_workspace_detail_with_current_true(app, bypass_pipeline, with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/switch", method="POST"): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + body, status = api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) assert status == 200 assert body["id"] == ws_id @@ -296,7 +297,7 @@ def test_switch_404s_when_service_raises_account_not_link_tenant(app, bypass_pip with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/switch", method="POST"): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(NotFound): - api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) # --------------------------------------------------------------------------- @@ -330,7 +331,7 @@ def test_members_list_returns_normalized_rows(app, bypass_pipeline, monkeypatch) with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/members"): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.get.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + body, status = api.get.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) assert status == 200 assert body["page"] == 1 @@ -372,7 +373,7 @@ def test_members_list_paginates_with_query_params(app, bypass_pipeline, monkeypa with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/members?page=2&limit=2"): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.get.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + body, status = api.get.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) assert status == 200 assert body["page"] == 2 @@ -395,7 +396,7 @@ def test_members_list_rejects_unknown_query_param(app, bypass_pipeline, monkeypa with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/members?pg=2"): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(BadRequest): - api.get.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.get.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) # --------------------------------------------------------------------------- @@ -433,7 +434,7 @@ def test_invite_happy_path_returns_invite_url_and_member_id(app, bypass_pipeline content_type="application/json", ): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + body, status = api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) assert status == 201 assert body["result"] == "success" @@ -518,7 +519,7 @@ def test_invite_blocked_by_saas_members_cap(app, bypass_pipeline, monkeypatch): with _invite_request(app, ws_id, acct_id): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(Forbidden) as exc_info: - api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) body = exc_info.value.response.json assert body["code"] == "members.limit_exceeded" @@ -564,7 +565,7 @@ def test_invite_blocked_by_ee_workspace_members_license(app, bypass_pipeline, mo with _invite_request(app, ws_id, acct_id): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(Forbidden) as exc_info: - api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) body = exc_info.value.response.json assert body["code"] == "workspace_members.license_exceeded" @@ -603,7 +604,7 @@ def test_invite_ce_passes_when_both_caps_disabled(app, bypass_pipeline, monkeypa with _invite_request(app, ws_id, acct_id): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + body, status = api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) assert status == 201 assert body["email"] == "new@example.com" @@ -632,7 +633,7 @@ def test_invite_400_when_already_in_tenant(app, bypass_pipeline, monkeypatch): ): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(BadRequest): - api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) # --------------------------------------------------------------------------- @@ -665,7 +666,7 @@ def test_delete_member_happy_path(app, bypass_pipeline, monkeypatch): method="DELETE", ): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.delete.__wrapped__.__wrapped__( + body, status = api.delete.__wrapped__( api, workspace_id=ws_id, member_id=member_id, auth_data=_auth_data(acct_id) ) @@ -707,7 +708,7 @@ def test_delete_member_exception_mapping(app, bypass_pipeline, monkeypatch, exc, ): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(expected): - api.delete.__wrapped__.__wrapped__( + api.delete.__wrapped__( api, workspace_id=ws_id, member_id=member_id, @@ -734,7 +735,7 @@ def test_delete_member_404_when_member_missing(app, bypass_pipeline, monkeypatch ): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(NotFound): - api.delete.__wrapped__.__wrapped__( + api.delete.__wrapped__( api, workspace_id=ws_id, member_id=member_id, @@ -774,9 +775,7 @@ def test_update_role_happy_path(app, bypass_pipeline, monkeypatch): content_type="application/json", ): _seed(_auth_ctx(account_id=acct_id)) - body, status = api.put.__wrapped__.__wrapped__( - api, workspace_id=ws_id, member_id=member_id, auth_data=_auth_data(acct_id) - ) + body, status = api.put.__wrapped__(api, workspace_id=ws_id, member_id=member_id, auth_data=_auth_data(acct_id)) assert status == 200 assert body == {"result": "success"} @@ -820,7 +819,7 @@ def test_update_role_exception_mapping(app, bypass_pipeline, monkeypatch, exc, e ): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(expected): - api.put.__wrapped__.__wrapped__( + api.put.__wrapped__( api, workspace_id=ws_id, member_id=member_id, @@ -828,44 +827,6 @@ def test_update_role_exception_mapping(app, bypass_pipeline, monkeypatch, exc, e ) -# --------------------------------------------------------------------------- -# Role gate composition — non-member sees 404 even with valid bearer -# --------------------------------------------------------------------------- - - -def test_non_member_caller_gets_404_on_switch(app, bypass_pipeline, monkeypatch): - """End-to-end: caller has valid account bearer but no membership in - the requested workspace. The role gate must short-circuit to 404 - before any TenantService method is touched.""" - ws_id = str(uuid.uuid4()) - acct_id = uuid.uuid4() - api = WorkspaceSwitchApi() - - mock_db = MagicMock() - mock_db.session.execute.return_value.scalar_one_or_none.return_value = None - - switch_mock = Mock() - monkeypatch.setattr( - sys.modules["controllers.openapi.workspaces"], - "TenantService", - _tenant_service(switch_tenant=switch_mock), - ) - monkeypatch.setattr(sys.modules["controllers.openapi.workspaces"], "db", mock_db) - monkeypatch.setattr(sys.modules["controllers.openapi.auth.role_gate"], "db", mock_db) - - with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/switch", method="POST"): - _seed(_auth_ctx(account_id=acct_id)) - # Strip only the bearer + surface-gate wrappers; keep the role gate. - # Decorator stack (innermost → outermost): - # role_gate → accept_subjects → validate_bearer - # `post.__wrapped__` is now the role-gate wrapper directly (auth_router.guard is the only outer wrapper). - gated = api.post.__wrapped__ - with pytest.raises(NotFound): - gated(api, workspace_id=ws_id) - - switch_mock.assert_not_called() - - # --------------------------------------------------------------------------- # _load_tenant rejects archived tenant # --------------------------------------------------------------------------- @@ -891,7 +852,7 @@ def test_load_tenant_rejects_archived_workspace(app, bypass_pipeline, monkeypatc with app.test_request_context(f"/openapi/v1/workspaces/{ws_id}/members"): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(NotFound): - api.get.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.get.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) # --------------------------------------------------------------------------- @@ -925,4 +886,4 @@ def test_invite_400_when_register_error(app, bypass_pipeline, monkeypatch): ): _seed(_auth_ctx(account_id=acct_id)) with pytest.raises(BadRequest): - api.post.__wrapped__.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) + api.post.__wrapped__(api, workspace_id=ws_id, auth_data=_auth_data(acct_id)) diff --git a/api/tests/unit_tests/services/test_account_service.py b/api/tests/unit_tests/services/test_account_service.py index f18e5a61b8..626c4b9706 100644 --- a/api/tests/unit_tests/services/test_account_service.py +++ b/api/tests/unit_tests/services/test_account_service.py @@ -638,8 +638,8 @@ class TestTenantService: callable_func(*args, **kwargs) # ==================== get_account_role_in_tenant Tests ==================== - # Backs `require_workspace_role`: None => non-member (gate maps to 404), - # otherwise the caller's role (gate maps an out-of-set role to 403). + # Backs the auth pipeline's `load_workspace_role`: None => non-member + # (pipeline maps to 404), otherwise the caller's role (out-of-set role => 403). def test_get_account_role_in_tenant_returns_role_for_member(self): """A row in TenantAccountJoin yields the caller's role."""