diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 45fe666..4ac6831 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -242,11 +242,13 @@ jobs: # - latest PDP image: the suite against permitio/pdp-v2:latest, on pydantic 2. # Red here with `pytest` green means the newest PDP release behaves unlike # PINNED_PDP_IMAGE. - # - cloud PDP: tests/test_cloud_pdp_e2e.py against the hosted cloud PDP. Each - # test builds a small RBAC policy in the scratch environment, waits for the - # cloud PDP to apply it, and asserts the exact answers of check, bulk_check, - # get_user_permissions and filter_objects. The module skips wherever PDP_URL - # points at a PDP container (see that module). + # - cloud PDP: tests/test_cloud_pdp_e2e.py against the hosted cloud PDP. Its + # tests build a small RBAC policy in the scratch environment, wait for the + # cloud PDP to apply it, and assert the exact answers of check, bulk_check, + # get_user_permissions and filter_objects. One more checks that + # get_user_tenants, which the cloud PDP does not serve, raises the SDK's + # error for its 404. The module skips wherever PDP_URL points at a PDP + # container (see that module). # Each leg makes its own scratch environment, keyed by run, attempt and leg, # so it never shares one with a `pytest` lane, the other leg or a re-run. e2e-unpinned-pdp: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 32ba583..d46f8db 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,18 +146,21 @@ passed or not: differently from the pinned one. - `e2e (cloud PDP)` is not a required check. Once both `pytest` jobs pass, it runs `tests/test_cloud_pdp_e2e.py` against the hosted cloud PDP, - `https://cloudpdp.api.permit.io`, with no container. Each test creates its own small RBAC - policy in the scratch environment, waits for the cloud PDP to apply it, and checks the - exact answers of `check`, `bulk_check`, `get_user_permissions` and `filter_objects`. The - module runs only against the cloud PDP and skips anywhere else, so this job fails if any - of its tests is skipped. + `https://cloudpdp.api.permit.io`, with no container. Its tests create a small RBAC policy + in the scratch environment, wait for the cloud PDP to apply it, and check the exact + answers of `check`, `bulk_check`, `get_user_permissions` and `filter_objects`. One more + checks that `get_user_tenants`, which the cloud PDP does not serve, raises the SDK's + error for its 404. The module runs only against the cloud PDP and skips anywhere else, + so this job fails if any of its tests is skipped. The jobs set: - `PDP_API_KEY`: the scratch environment's API key. Every e2e test fails without it. - `PDP_URL`: `http://localhost:7766`, the PDP container, or `https://cloudpdp.api.permit.io` in `e2e (cloud PDP)`. When it is unset, `tests/test_cloud_pdp_e2e.py` uses the cloud PDP - and every other test `http://localhost:7766`. + and every other test `http://localhost:7766`. The `get_user_tenants` tests in + `tests/test_tenant_membership_e2e.py` need a PDP container, because the cloud PDP does not + serve that query, so they skip, with the reason, when `PDP_URL` is the cloud PDP. - `API_TIER=prod`: sends the SDK's API calls to `https://api.permit.io`. - `ORG_PDP_API_KEY` and `PROJECT_PDP_API_KEY`: the same key, read by `tests/endpoints/test_envs.py`. diff --git a/README.md b/README.md index 2cd909d..2f9c9c7 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,24 @@ await permit.check("alice", "edit", {"type": "document", "key": "readme", "tenan - The other methods are `list()`, `get()`, `delete()`, `remove_user()` and `remove_role()`. The blocking client, `permit.sync.Permit`, has the same methods. +## Tenant membership + +`permit.api.tenants.create_user("acme", {"key": "alice"})` creates the user as a member of the +`acme` tenant, with no role there. Any `role_assignments` in the user data are granted as +`permit.api.users.create()` grants them, each in the tenant it names. It fails with +`PermitAlreadyExistsError` (409) when a user with that key already exists, so give an existing +user a role in the tenant with `permit.api.users.assign_role()` instead. The request always +goes to the Permit REST API, even with `proxy_facts_via_pdp`, and needs an environment-level +API key, or a broader key with the SDK's API context set to the environment. +`permit.api.tenants.delete_tenant_user()` answers 404 for a member with no role, so remove +such a member with `permit.api.users.delete()`. + +`permit.get_user_tenants("alice")` asks the PDP for the tenants in which the user has a +tenant-level role, as `TenantDetails` objects with a `key` and `attributes`. Membership +without a role, such as `create_user()` creates, is not listed. Only the container PDP serves +this query: the cloud PDP answers 404, which the SDK raises as a `PermitConnectionError`. +Both methods are on the blocking client too. + ## Type checking The package ships a `py.typed` marker (PEP 561), so mypy, pyright and IDEs check your @@ -116,6 +134,8 @@ each one issues a `DeprecationWarning` that says what to do instead. - **The flat methods on `permit.api`**, such as `permit.api.get_user()`. Use the grouped APIs instead, such as `permit.api.users.get()`. Each flat method's warning names its replacement. +- **`permit.api.tenants.add_user()`**, an alias of `permit.api.tenants.create_user()`. The + route creates the user, so `create_user()` is the name that says what it does. By default, Python shows these warnings only when the code that triggers them is in `__main__`, such as the script you run. pytest shows them in its warnings summary. To see diff --git a/permit/__init__.py b/permit/__init__.py index 8dbc757..d21a8fb 100644 --- a/permit/__init__.py +++ b/permit/__init__.py @@ -14,6 +14,7 @@ from permit.enforcement.interfaces import AssignedRole as AssignedRole from permit.enforcement.interfaces import AuthorizedUsersResult as AuthorizedUsersResult from permit.enforcement.interfaces import ResourceInput as ResourceInput +from permit.enforcement.interfaces import TenantDetails as TenantDetails from permit.enforcement.interfaces import UserInput as UserInput from permit.exceptions import PermitAlreadyExistsError as PermitAlreadyExistsError from permit.exceptions import PermitApiDetailedError as PermitApiDetailedError diff --git a/permit/_sync_types.pyi b/permit/_sync_types.pyi index efdc3c0..242426e 100644 --- a/permit/_sync_types.pyi +++ b/permit/_sync_types.pyi @@ -90,7 +90,7 @@ from permit.api.models import ( from permit.api.users import _UserSyncInput from permit.config import PermitConfig from permit.enforcement.enforcer import Action, CheckQuery, Resource, User -from permit.enforcement.interfaces import AuthorizedUsersResult +from permit.enforcement.interfaces import AuthorizedUsersResult, TenantDetails from permit.pdp_api.base import BasePdpPermitApi from permit.pdp_api.models import RoleAssignment from permit.utils.context import Context, ContextStore @@ -2219,6 +2219,53 @@ class SyncTenantsApi(BasePermitApi): PermitContextError: If the configured ApiContext does not match the required endpoint context. """ + def create_user(self, tenant_key: str, user_data: ModelInput[UserCreate]) -> UserRead: + """Creates a user as a member of a tenant. + + The API creates the user and adds it to the tenant without any role. It answers 409 + when a user with that key already exists, whichever tenants it is in, so this cannot + add an existing user to another tenant: grant that user a role in the tenant with + ``api.users.assign_role()`` instead. Role assignments listed in ``user_data`` are + granted as ``api.users.create()`` grants them, each in the tenant it names. + + The request always goes to the Permit REST API, even with ``proxy_facts_via_pdp`` + set, so ``wait_for_sync()`` does not make it wait for the PDP. A membership without a + role does not show in ``permit.get_user_tenants()``, which lists the tenants in which + the user has a role, and ``delete_tenant_user()`` cannot remove it: delete the user + with ``api.users.delete()`` instead. + + Needs an environment-level API key, or a broader key with the SDK's API context set + to the environment. + + Args: + tenant_key: The key or id of the tenant. + user_data: The user to create, as a ``UserCreate`` or an equivalent dict. + + Returns: + the created user, whose ``associated_tenants`` include the tenant. + + Raises: + PermitAlreadyExistsError: If a user with this key already exists, or a role + assignment in ``user_data`` names a tenant other than the one its resource + instance is in. + PermitNotFoundError: If the tenant does not exist, or a role assignment in + ``user_data`` names a role, tenant or resource that does not exist. + PermitApiError: If the API returns any other error HTTP status code. + PermitContextError: If the configured ApiContext does not match the required endpoint + context. + """ + def add_user(self, tenant_key: str, user_data: ModelInput[UserCreate]) -> UserRead: + """Deprecated: use ``create_user()`` instead, which this calls. + + The route creates the user, so it cannot add an existing user to a tenant. + + Args: + tenant_key: The key or id of the tenant. + user_data: The user to create, as a ``UserCreate`` or an equivalent dict. + + Returns: + the created user, as ``create_user()`` returns it. + """ def get(self, tenant_key: str) -> TenantRead: """Retrieves a tenant by its key. @@ -2309,14 +2356,24 @@ class SyncTenantsApi(BasePermitApi): context. """ def delete_tenant_user(self, tenant_key: str, user_key: str) -> None: - """Deletes a user from a tenant, removing all roles granted to the user in that tenant. + """Removes the roles a user holds in a tenant. + + The API removes the user's tenant-level roles in the tenant, and answers 404 when the + user holds none there. That includes a member that ``create_user()`` created without a + role, which this cannot remove: delete such a user with ``api.users.delete()``. + + When the user is then left with no tenant-level role in any tenant, the API deletes + the user, even if the user is still a member of a tenant without a role or holds roles + on resource instances, so ``create_user()`` can create a user with that key again. + Otherwise the user stays a member of the tenant, with no tenant-level role there. Args: - tenant_key: The key of the tenant from which the user will be deleted. - user_key: The key of the user to be deleted. + tenant_key: The key of the tenant. + user_key: The key of the user whose roles in the tenant to remove. Raises: - PermitApiError: If the API returns an error HTTP status code. + PermitApiError: If the user holds no tenant-level role in the tenant (404), or the + API returns any other error HTTP status code. PermitContextError: If the configured ApiContext does not match the required endpoint context. """ @@ -2770,6 +2827,32 @@ class SyncEnforcer: Raises: PermitConnectionError: If the PDP rejects the request or cannot be reached. """ + def get_user_tenants(self, user: User, context: Context | None = None) -> list[TenantDetails]: + """Get the tenants in which a user has a role, as the PDP knows them. + + The PDP lists a tenant when the user has a tenant-level role in it, the kind + ``api.users.assign_role()`` grants. A role on a resource instance does not count, and + neither does membership without a role, such as ``api.tenants.create_user()`` creates. + The PDP answers from the data it has synced, so a change made through the API shows + up once the PDP has it. + + Only the container PDP serves this query. The cloud PDP does not, and answers 404, + which this method raises as a ``PermitConnectionError`` that says so. + + Args: + user: The user key, or a user dict with a ``key`` and optionally ``attributes``, + ``email``, ``first_name`` and ``last_name``, as ``check()`` takes it. + context: The query's context, merged over the context store's base context. + Defaults to None. + + Returns: + The user's tenants, each with its key and attributes. Empty when the user has no + tenant-level role or the PDP does not know the user. + + Raises: + PermitConnectionError: If the PDP answers 404 (as the cloud PDP does), answers any + other error status, or cannot be reached. + """ def filter_objects( self, user: User, action: Action, context: Context, resources: list[dict[str, Any]] ) -> list[dict[str, Any]]: diff --git a/permit/api/tenants.py b/permit/api/tenants.py index 11ae144..3d5550d 100644 --- a/permit/api/tenants.py +++ b/permit/api/tenants.py @@ -23,7 +23,10 @@ TenantDeleteBulkOperationResult, TenantRead, TenantUpdate, + UserCreate, + UserRead, ) +from permit.utils.deprecation import deprecated from permit.utils.model_input import ModelInput, ModelListInput @@ -34,6 +37,11 @@ class TenantsApi(BasePermitApi): def __tenants(self) -> SimpleHttpClient: if self.config.proxy_facts_via_pdp: return self._build_http_client("/facts/tenants", use_pdp=True) + return self.__api_tenants + + @property + def __api_tenants(self) -> SimpleHttpClient: + """The tenants collection on the Permit REST API, whatever proxy_facts_via_pdp says.""" return self._build_http_client( f"/v2/facts/{self.config.api_context.project}/{self.config.api_context.environment}/tenants" ) @@ -95,6 +103,64 @@ async def list_tenant_users( params=pagination_params(page, per_page), ) + @validate_arguments + async def create_user(self, tenant_key: str, user_data: ModelInput[UserCreate]) -> UserRead: + """Creates a user as a member of a tenant. + + The API creates the user and adds it to the tenant without any role. It answers 409 + when a user with that key already exists, whichever tenants it is in, so this cannot + add an existing user to another tenant: grant that user a role in the tenant with + ``api.users.assign_role()`` instead. Role assignments listed in ``user_data`` are + granted as ``api.users.create()`` grants them, each in the tenant it names. + + The request always goes to the Permit REST API, even with ``proxy_facts_via_pdp`` + set, so ``wait_for_sync()`` does not make it wait for the PDP. A membership without a + role does not show in ``permit.get_user_tenants()``, which lists the tenants in which + the user has a role, and ``delete_tenant_user()`` cannot remove it: delete the user + with ``api.users.delete()`` instead. + + Needs an environment-level API key, or a broader key with the SDK's API context set + to the environment. + + Args: + tenant_key: The key or id of the tenant. + user_data: The user to create, as a ``UserCreate`` or an equivalent dict. + + Returns: + the created user, whose ``associated_tenants`` include the tenant. + + Raises: + PermitAlreadyExistsError: If a user with this key already exists, or a role + assignment in ``user_data`` names a tenant other than the one its resource + instance is in. + PermitNotFoundError: If the tenant does not exist, or a role assignment in + ``user_data`` names a role, tenant or resource that does not exist. + PermitApiError: If the API returns any other error HTTP status code. + PermitContextError: If the configured ApiContext does not match the required endpoint + context. + """ + await self._ensure_access_level(ApiKeyAccessLevel.ENVIRONMENT_LEVEL_API_KEY) + await self._ensure_context(ApiContextLevel.ENVIRONMENT) + return await self.__api_tenants.post(f"/{tenant_key}/users", model=UserRead, json=user_data) + + @deprecated( + "permit.api.tenants.add_user() is deprecated and will be removed in permit 4.0; " + "use permit.api.tenants.create_user() instead." + ) + async def add_user(self, tenant_key: str, user_data: ModelInput[UserCreate]) -> UserRead: + """Deprecated: use ``create_user()`` instead, which this calls. + + The route creates the user, so it cannot add an existing user to a tenant. + + Args: + tenant_key: The key or id of the tenant. + user_data: The user to create, as a ``UserCreate`` or an equivalent dict. + + Returns: + the created user, as ``create_user()`` returns it. + """ + return await self.create_user(tenant_key, user_data) + async def _get(self, tenant_key: str) -> TenantRead: return await self.__tenants.get(f"/{tenant_key}", model=TenantRead) @@ -219,14 +285,24 @@ async def delete(self, tenant_key: str) -> None: @validate_arguments async def delete_tenant_user(self, tenant_key: str, user_key: str) -> None: - """Deletes a user from a tenant, removing all roles granted to the user in that tenant. + """Removes the roles a user holds in a tenant. + + The API removes the user's tenant-level roles in the tenant, and answers 404 when the + user holds none there. That includes a member that ``create_user()`` created without a + role, which this cannot remove: delete such a user with ``api.users.delete()``. + + When the user is then left with no tenant-level role in any tenant, the API deletes + the user, even if the user is still a member of a tenant without a role or holds roles + on resource instances, so ``create_user()`` can create a user with that key again. + Otherwise the user stays a member of the tenant, with no tenant-level role there. Args: - tenant_key: The key of the tenant from which the user will be deleted. - user_key: The key of the user to be deleted. + tenant_key: The key of the tenant. + user_key: The key of the user whose roles in the tenant to remove. Raises: - PermitApiError: If the API returns an error HTTP status code. + PermitApiError: If the user holds no tenant-level role in the tenant (404), or the + API returns any other error HTTP status code. PermitContextError: If the configured ApiContext does not match the required endpoint context. """ diff --git a/permit/enforcement/enforcer.py b/permit/enforcement/enforcer.py index c9d5102..9c019fe 100644 --- a/permit/enforcement/enforcer.py +++ b/permit/enforcement/enforcer.py @@ -8,7 +8,12 @@ from typing_extensions import NotRequired, TypedDict from permit.config import PermitConfig -from permit.enforcement.interfaces import AuthorizedUsersResult, ResourceInput, UserInput +from permit.enforcement.interfaces import ( + AuthorizedUsersResult, + ResourceInput, + TenantDetails, + UserInput, +) from permit.exceptions import PermitConnectionError from permit.utils.context import Context, ContextStore from permit.utils.dicts import deep_merge @@ -543,6 +548,83 @@ async def get_user_permissions( error=err, ) from err + async def get_user_tenants( + self, user: User, context: Context | None = None + ) -> list[TenantDetails]: + """Get the tenants in which a user has a role, as the PDP knows them. + + The PDP lists a tenant when the user has a tenant-level role in it, the kind + ``api.users.assign_role()`` grants. A role on a resource instance does not count, and + neither does membership without a role, such as ``api.tenants.create_user()`` creates. + The PDP answers from the data it has synced, so a change made through the API shows + up once the PDP has it. + + Only the container PDP serves this query. The cloud PDP does not, and answers 404, + which this method raises as a ``PermitConnectionError`` that says so. + + Args: + user: The user key, or a user dict with a ``key`` and optionally ``attributes``, + ``email``, ``first_name`` and ``last_name``, as ``check()`` takes it. + context: The query's context, merged over the context store's base context. + Defaults to None. + + Returns: + The user's tenants, each with its key and attributes. Empty when the user has no + tenant-level role or the PDP does not know the user. + + Raises: + PermitConnectionError: If the PDP answers 404 (as the cloud PDP does), answers any + other error status, or cannot be reached. + """ + normalized_user: UserInput = ( + UserInput(key=user) if isinstance(user, str) else UserInput(**user) + ) + body = { + "user": normalized_user.dict(exclude_unset=True), + "context": self._context_store.get_derived_context(context or {}), + } + + async with aiohttp.ClientSession(headers=self._headers, **self._timeout_config) as session: + url = f"{self._base_url}/user-tenants" + try: + async with session.post(url, data=json.dumps(body)) as response: + if response.status == HTTPStatus.NOT_FOUND: + msg = ( + f"permit.get_user_tenants() got status code 404 from the PDP at " + f"{self._base_url}: only the container PDP serves /user-tenants, " + f"and the cloud PDP does not.\n" + f"Point the SDK's `pdp` setting at a container PDP to use it.\n" + f"Read more about setting up the PDP at {SETUP_PDP_DOCS_LINK}" + ) + raise PermitConnectionError(msg) + if response.status != HTTPStatus.OK: + error_body = await read_error_body(response) + msg = ( + f"permit.get_user_tenants() got an unexpected status code: " + f"{response.status} from the PDP at {self._base_url}.\n" + f"Response body: {error_body}\n" + f"Read more about setting up the PDP at {SETUP_PDP_DOCS_LINK}" + ) + raise PermitConnectionError(msg) + content = await response.json() + except aiohttp.ClientError as err: + sdk_logger.error(f"Error in permit.get_user_tenants(): {err}") + msg = ( + f"Permit SDK got error: {err}, \n" + f"and cannot connect to the PDP container, please check your configuration " + f"and make sure it's running at {self._base_url} and accepting requests. \n" + f"Read more about setting up the PDP at {SETUP_PDP_DOCS_LINK}" + ) + raise PermitConnectionError(msg, error=err) from err + + sdk_logger.debug( + f"permit.get_user_tenants() response:\n" + f"input: {pformat(body, indent=2)}\n" + f"response data: {pformat(content, indent=2)}" + ) + tenants: list[TenantDetails] = parse_obj_as(list[TenantDetails], content) + return tenants + async def filter_objects( self, user: User, action: Action, context: Context, resources: list[dict[str, Any]] ) -> list[dict[str, Any]]: diff --git a/permit/enforcement/interfaces.py b/permit/enforcement/interfaces.py index 5408208..6a2e503 100644 --- a/permit/enforcement/interfaces.py +++ b/permit/enforcement/interfaces.py @@ -85,6 +85,15 @@ class AuthorizedUserAssignment(BaseModel): AuthorizedUsersDict = Dict[str, List[AuthorizedUserAssignment]] # noqa: UP006 +class TenantDetails(BaseModel): + """A tenant as the PDP describes it in a `get_user_tenants()` answer.""" + + key: str = Field(..., description="The tenant key") + attributes: dict[str, Any] = Field( + default_factory=dict, description="The tenant's attributes, empty when it has none" + ) + + class AuthorizedUsersResult(BaseModel): """The result of an `authorized_users()` query.""" diff --git a/permit/permit.py b/permit/permit.py index a636a05..680e3a3 100644 --- a/permit/permit.py +++ b/permit/permit.py @@ -15,7 +15,7 @@ Resource, User, ) -from permit.enforcement.interfaces import AuthorizedUsersResult +from permit.enforcement.interfaces import AuthorizedUsersResult, TenantDetails from permit.logger import configure_logger from permit.pdp_api.pdp_api_client import PermitPdpApiClient from permit.utils.context import Context @@ -269,6 +269,41 @@ async def get_user_permissions( """ return await self._enforcer.get_user_permissions(user, tenants, resources, resource_types) + async def get_user_tenants( + self, user: User, context: Context | None = None + ) -> list[TenantDetails]: + """Get the tenants in which a user has a role, as the PDP knows them. + + The PDP lists a tenant when the user has a tenant-level role in it, the kind + ``api.users.assign_role()`` grants. A role on a resource instance does not count, and + neither does membership without a role, such as ``api.tenants.create_user()`` creates. + The PDP answers from the data it has synced, so a change made through the API shows + up once the PDP has it. + + Only the container PDP serves this query. The cloud PDP does not, and answers 404, + which this method raises as a ``PermitConnectionError`` that says so. + + Args: + user: The user key, or a user dict with a ``key`` and optionally ``attributes``, + ``email``, ``first_name`` and ``last_name``, as ``check()`` takes it. + context: The query's context, merged over the context store's base context. + Defaults to None. + + Returns: + list[TenantDetails]: The user's tenants, each with its key and attributes. Empty + when the user has no tenant-level role or the PDP does not know the user. + + Raises: + PermitConnectionError: If the PDP answers 404 (as the cloud PDP does), answers any + other error status, or cannot be reached. + + Examples: + # the tenants in which alice has a role + tenants = await permit.get_user_tenants("alice") + keys = [tenant.key for tenant in tenants] + """ + return await self._enforcer.get_user_tenants(user, context) + async def filter_objects( self, user: User, action: Action, context: Context, resources: list[dict[str, Any]] ) -> list[dict[str, Any]]: diff --git a/permit/sync.py b/permit/sync.py index 84bc0d7..f46637a 100644 --- a/permit/sync.py +++ b/permit/sync.py @@ -10,7 +10,7 @@ SyncEnforcer, User, ) -from permit.enforcement.interfaces import AuthorizedUsersResult +from permit.enforcement.interfaces import AuthorizedUsersResult, TenantDetails from permit.pdp_api.pdp_api_client import SyncPDPApi from permit.permit import Permit as AsyncPermit from permit.utils.context import Context @@ -209,6 +209,41 @@ def get_user_permissions( # type: ignore[override] user, tenants, resources, resource_types ) + def get_user_tenants( # type: ignore[override] + self, user: User, context: Context | None = None + ) -> list[TenantDetails]: + """Get the tenants in which a user has a role, as the PDP knows them. + + The PDP lists a tenant when the user has a tenant-level role in it, the kind + ``api.users.assign_role()`` grants. A role on a resource instance does not count, and + neither does membership without a role, such as ``api.tenants.create_user()`` creates. + The PDP answers from the data it has synced, so a change made through the API shows + up once the PDP has it. + + Only the container PDP serves this query. The cloud PDP does not, and answers 404, + which this method raises as a ``PermitConnectionError`` that says so. + + Args: + user: The user key, or a user dict with a ``key`` and optionally ``attributes``, + ``email``, ``first_name`` and ``last_name``, as ``check()`` takes it. + context: The query's context, merged over the context store's base context. + Defaults to None. + + Returns: + list[TenantDetails]: The user's tenants, each with its key and attributes. Empty + when the user has no tenant-level role or the PDP does not know the user. + + Raises: + PermitConnectionError: If the PDP answers 404 (as the cloud PDP does), answers any + other error status, or cannot be reached. + + Examples: + # the tenants in which alice has a role + tenants = permit.get_user_tenants("alice") + keys = [tenant.key for tenant in tenants] + """ + return self._enforcer.get_user_tenants(user, context) # type: ignore[return-value] + def filter_objects( # type: ignore[override] self, user: User, action: Action, context: Context, resources: list[dict[str, Any]] ) -> list[dict[str, Any]]: diff --git a/tests/conftest.py b/tests/conftest.py index e29f4c0..9767dec 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,7 +13,7 @@ from permit.api.base import SimpleHttpClient from permit.exceptions import PermitApiError from permit.sync import Permit as SyncPermit -from tests.utils import offline_config +from tests.utils import CLOUD_PDP_URL, offline_config # pytest_httpserver's `httpserver` fixture binds a free port chosen by the OS, # so parallel runs on one machine cannot collide. Tests reach it through @@ -42,9 +42,7 @@ def config(httpserver: HTTPServer) -> PermitConfig: @pytest.fixture def permit_config() -> PermitConfig: default_pdp_address = ( - "https://cloudpdp.api.permit.io" - if os.getenv("CLOUD_PDP") == "true" - else "http://localhost:7766" + CLOUD_PDP_URL if os.getenv("CLOUD_PDP") == "true" else "http://localhost:7766" ) default_api_address = ( "https://api.permit.io" if os.getenv("API_TIER") == "prod" else "http://localhost:8000" @@ -81,7 +79,7 @@ def sync_permit(permit_config: PermitConfig) -> SyncPermit: @pytest.fixture def permit_config_cloud() -> PermitConfig: token = os.getenv("PDP_API_KEY", "") - pdp_address = os.getenv("PDP_URL", "https://cloudpdp.api.permit.io") + pdp_address = os.getenv("PDP_URL", CLOUD_PDP_URL) api_url = os.getenv("PDP_CONTROL_PLANE", "https://api.permit.io") if not token: diff --git a/tests/test_cloud_pdp_e2e.py b/tests/test_cloud_pdp_e2e.py index 2798aac..423fd95 100644 --- a/tests/test_cloud_pdp_e2e.py +++ b/tests/test_cloud_pdp_e2e.py @@ -8,6 +8,9 @@ RBAC decides on the resource type and tenant alone, so the resources these tests ask about need not exist as resource instances. + +``get_user_tenants`` needs no policy: only the container PDP serves it, and its test +checks that the cloud PDP's 404 for it reaches the caller as the error that says so. """ import functools @@ -19,10 +22,8 @@ import pytest -from permit import Permit -from tests.utils import delete_quietly, poll_for, unique_key - -CLOUD_PDP_URL: Final[str] = "https://cloudpdp.api.permit.io" +from permit import Permit, PermitConnectionError +from tests.utils import CLOUD_PDP_URL, delete_quietly, poll_for, unique_key # conftest's `permit_cloud` fixture resolves its address as # os.getenv("PDP_URL", CLOUD_PDP_URL), so it only reaches the cloud PDP when @@ -253,3 +254,13 @@ async def test_filter_objects(permit_cloud: Permit, cloud_policy: CloudPolicy) - assert kept == expected assert await permit_cloud.filter_objects(policy.user, DENIED_ACTION, {}, resources) == [] + + +async def test_get_user_tenants_is_not_served(permit_cloud: Permit) -> None: + with pytest.raises(PermitConnectionError) as raised: + await permit_cloud.get_user_tenants(unique_key("cloud-user")) + + message = str(raised.value) + assert "got status code 404 from the PDP" in message + assert "only the container PDP serves /user-tenants" in message + assert raised.value.original_error is None diff --git a/tests/test_fix_logging.py b/tests/test_fix_logging.py index 5422f42..95cb08b 100644 --- a/tests/test_fix_logging.py +++ b/tests/test_fix_logging.py @@ -269,7 +269,7 @@ async def test_an_api_key_the_pdp_echoes_back_is_redacted( async def test_errors_raised_for_a_pdp_that_echoes_the_key_do_not_hold_it( httpserver: HTTPServer, ) -> None: - for path in ("/allowed", "/allowed/bulk", "/authorized_users"): + for path in ("/allowed", "/allowed/bulk", "/authorized_users", "/user-tenants"): httpserver.expect_request(path, method="POST").respond_with_handler(echo_the_key) permit = Permit(make_config(httpserver)) sync_permit = SyncPermit(make_config(httpserver)) @@ -279,6 +279,7 @@ async def test_errors_raised_for_a_pdp_that_echoes_the_key_do_not_hold_it( lambda: permit.check("user-1", "read", "document"), lambda: permit.bulk_check(BULK), lambda: permit.authorized_users("read", "document"), + lambda: permit.get_user_tenants("user-1"), ): with pytest.raises(PermitConnectionError) as error: await call() diff --git a/tests/test_groups_e2e.py b/tests/test_groups_e2e.py index dc8a714..6ee63e8 100644 --- a/tests/test_groups_e2e.py +++ b/tests/test_groups_e2e.py @@ -13,7 +13,7 @@ """ import functools -from collections.abc import AsyncIterator, Callable +from collections.abc import AsyncIterator from contextlib import AsyncExitStack, ExitStack from dataclasses import dataclass from typing import Any, Final @@ -25,7 +25,7 @@ from permit.api.models import GroupAddRole, GroupAssignment, GroupRead, GroupReadSchema, UserRead from permit.exceptions import PermitApiError from permit.sync import Permit as SyncPermit -from tests.utils import delete_quietly, handle_cleanup_error, poll_for, unique_key +from tests.utils import delete_quietly, delete_quietly_blocking, poll_for, unique_key pytestmark = pytest.mark.e2e @@ -50,14 +50,6 @@ settled = functools.partial(poll_for, timeout=PROPAGATION_TIMEOUT, interval=POLL_INTERVAL) -def delete_quietly_blocking(delete: Callable[[], None], description: str) -> None: - """Delete one object at teardown through the blocking client, as ``delete_quietly``.""" - try: - delete() - except PermitApiError as error: - handle_cleanup_error(error, f"could not delete {description}") - - @dataclass(frozen=True) class GroupPolicy: """The keys of one test's policy, all unique to it.""" diff --git a/tests/test_tenant_membership_e2e.py b/tests/test_tenant_membership_e2e.py new file mode 100644 index 0000000..e004dbc --- /dev/null +++ b/tests/test_tenant_membership_e2e.py @@ -0,0 +1,329 @@ +"""Tenant membership against the Permit API and a container PDP (PER-16678). + +``tenants.create_user`` creates a user as a member of one tenant, with no role there. The +user must be new: the API answers 409 for a key that already exists, so it does not add +an existing user to another tenant. ``tenants.delete_tenant_user`` removes the roles the +user holds in the tenant, and answers 404 for a member with no role there. A user who +still holds a role in another tenant stays a member; the API deletes a user left with no +role in any tenant. + +``get_user_tenants`` asks the PDP for the user's tenants. The PDP lists the tenants in +which the user holds a role assigned in the tenant, with each tenant's attributes; a +member with no role in a tenant is not listed. Only a container PDP serves the route, so +the tests that call it skip when the PDP in use is the hosted cloud PDP. + +Each test makes its own tenants, role and user in the environment the API key belongs to. +Every key is unique to the run, and every delete is registered before the create it +undoes, so a test that fails part way still removes what it made. Teardown runs in +reverse order of registration, and a 404 there counts as success. +""" + +import functools +import time +from collections.abc import AsyncIterator, Callable +from contextlib import AsyncExitStack, ExitStack +from typing import Any, Final, TypeVar + +import pytest + +from permit import Permit, PermitConfig, User, UserCreate, UserRead +from permit.exceptions import PermitApiError +from permit.sync import Permit as SyncPermit +from tests.utils import ( + CLOUD_PDP_URL, + delete_quietly, + delete_quietly_blocking, + poll_for, + unique_key, +) + +pytestmark = pytest.mark.e2e + +NOT_FOUND: Final[int] = 404 +CONFLICT: Final[int] = 409 +REGION: Final[dict[str, Any]] = {"region": "eu"} + +# Writes reach the PDP asynchronously. The bound is reached only when an answer never +# converges; polling returns as soon as it does. +PROPAGATION_TIMEOUT: Final[float] = 60.0 +POLL_INTERVAL: Final[float] = 0.5 + +settled = functools.partial(poll_for, timeout=PROPAGATION_TIMEOUT, interval=POLL_INTERVAL) + +T = TypeVar("T") + + +@pytest.fixture +def container_pdp(permit_config: PermitConfig) -> None: + """Skip the test when the PDP the ``permit`` fixtures call is the hosted cloud PDP. + + The cloud PDP answers 404 for ``get_user_tenants``. The CI jobs that run this module + start a container PDP and point PDP_URL at it. + """ + if permit_config.pdp.startswith(CLOUD_PDP_URL): + pytest.skip( + f"container-PDP-only test: the PDP in use is the cloud PDP ({permit_config.pdp}), " + "which does not serve get_user_tenants. Point PDP_URL at a container PDP." + ) + + +@pytest.fixture +async def teardown() -> AsyncIterator[AsyncExitStack]: + """The deletes a test registers, run once the test ends.""" + async with AsyncExitStack() as stack: + yield stack + + +async def create_tenant( + permit: Permit, teardown: AsyncExitStack, name: str, attributes: dict[str, Any] | None = None +) -> str: + """Create a tenant with a unique key, its delete registered first, and return the key.""" + key = unique_key(name) + teardown.push_async_callback( + delete_quietly, functools.partial(permit.api.tenants.delete, key), f"tenant '{key}'" + ) + tenant: dict[str, Any] = {"key": key, "name": key} + if attributes is not None: + tenant["attributes"] = attributes + await permit.api.tenants.create(tenant) + return key + + +async def create_role(permit: Permit, teardown: AsyncExitStack) -> str: + """Create a role with a unique key, its delete registered first, and return the key.""" + key = unique_key("member-role") + teardown.push_async_callback( + delete_quietly, functools.partial(permit.api.roles.delete, key), f"role '{key}'" + ) + await permit.api.roles.create({"key": key, "name": key}) + return key + + +def register_user_delete(permit: Permit, teardown: AsyncExitStack, user_key: str) -> None: + """Register the delete of a user that ``create_user`` is about to create.""" + teardown.push_async_callback( + delete_quietly, functools.partial(permit.api.users.delete, user_key), f"user '{user_key}'" + ) + + +async def assign_role( + permit: Permit, teardown: AsyncExitStack, user_key: str, role: str, tenant: str +) -> None: + """Give the user a role in the tenant, its removal registered first.""" + assignment = {"user": user_key, "role": role, "tenant": tenant} + teardown.push_async_callback( + delete_quietly, + functools.partial(permit.api.users.unassign_role, assignment), + f"role assignment {assignment}", + ) + await permit.api.users.assign_role(assignment) + + +def tenant_roles(user: UserRead) -> list[tuple[str, list[str]]]: + """The tenants the API lists the user in, each with the roles the user holds there.""" + return [(tenant.tenant, tenant.roles) for tenant in user.associated_tenants or []] + + +async def tenants_of( + permit: Permit, user: User, context: dict[str, Any] | None = None +) -> dict[str, Any]: + """What ``get_user_tenants`` answers for the user, as ``{tenant key: attributes}``. + + A mapping, because the order the PDP lists the tenants in is not part of the answer. + """ + tenants = await permit.get_user_tenants(user, context=context) + return {tenant.key: tenant.attributes for tenant in tenants} + + +def poll_for_blocking(fetch: Callable[[], T], expected: T) -> T: + """``poll_for`` for the blocking client, with this module's bounds.""" + deadline = time.monotonic() + PROPAGATION_TIMEOUT + answer = fetch() + while answer != expected and time.monotonic() < deadline: + time.sleep(POLL_INTERVAL) + answer = fetch() + return answer + + +async def test_create_user_creates_a_member_with_no_role( + permit: Permit, teardown: AsyncExitStack +) -> None: + tenants = permit.api.tenants + tenant = await create_tenant(permit, teardown, "member-tenant") + other_tenant = await create_tenant(permit, teardown, "other-tenant") + role = await create_role(permit, teardown) + user_key = unique_key("member") + register_user_delete(permit, teardown, user_key) + + async def members_of(tenant_key: str) -> list[tuple[str, list[tuple[str, list[str]]]]]: + listed = await tenants.list_tenant_users(tenant_key) + return [(user.key, tenant_roles(user)) for user in listed.data] + + member = await tenants.create_user( + tenant, + UserCreate( + key=user_key, + email=f"{user_key}@example.com", + first_name="Ada", + last_name="Lovelace", + attributes={"department": "eng"}, + ), + ) + + assert member.key == user_key + assert member.email == f"{user_key}@example.com" + assert (member.first_name, member.last_name) == ("Ada", "Lovelace") + assert member.attributes == {"department": "eng"} + assert tenant_roles(member) == [(tenant, [])] + assert member.roles == [] + listed = await tenants.list_tenant_users(tenant) + assert [(user.key, tenant_roles(user), user.roles) for user in listed.data] == [ + (user_key, [(tenant, [])], []) + ] + assert listed.total_count == 1 + assert (await tenants.list_tenant_users(other_tenant)).data == [] + + # The API creates the user, so a key that already exists is refused rather than added + # to the other tenant. + with pytest.raises(PermitApiError) as existing_user: + await tenants.create_user(other_tenant, {"key": user_key}) + assert existing_user.value.status_code == CONFLICT + assert (await tenants.list_tenant_users(other_tenant)).data == [] + + missing_tenant = unique_key("missing-tenant") + unadded_key = unique_key("member") + register_user_delete(permit, teardown, unadded_key) + with pytest.raises(PermitApiError) as no_tenant: + await tenants.create_user(missing_tenant, {"key": unadded_key}) + assert no_tenant.value.status_code == NOT_FOUND + + # The member holds no role in the tenant, so delete_tenant_user has nothing to remove. + with pytest.raises(PermitApiError) as no_role: + await tenants.delete_tenant_user(tenant, user_key) + assert no_role.value.status_code == NOT_FOUND + assert await members_of(tenant) == [(user_key, [(tenant, [])])] + + await assign_role(permit, teardown, user_key, role, tenant) + await assign_role(permit, teardown, user_key, role, other_tenant) + await tenants.delete_tenant_user(tenant, user_key) + + # The user still holds a role in the other tenant, so they stay a member of this one. + assert await members_of(tenant) == [(user_key, [(tenant, [])])] + assert await members_of(other_tenant) == [(user_key, [(other_tenant, [role])])] + + await tenants.delete_tenant_user(other_tenant, user_key) + + # No role is left in any tenant, so the API deleted the user, membership and all. + assert await members_of(other_tenant) == [] + assert await members_of(tenant) == [] + with pytest.raises(PermitApiError) as deleted_user: + await permit.api.users.get(user_key) + assert deleted_user.value.status_code == NOT_FOUND + with pytest.raises(PermitApiError) as removed_twice: + await tenants.delete_tenant_user(tenant, user_key) + assert removed_twice.value.status_code == NOT_FOUND + + +@pytest.mark.usefixtures("container_pdp") +async def test_get_user_tenants_lists_the_tenants_the_user_holds_a_role_in( + permit: Permit, teardown: AsyncExitStack +) -> None: + member_tenant = await create_tenant(permit, teardown, "member-tenant", attributes=REGION) + role_tenant = await create_tenant(permit, teardown, "role-tenant") + outside_tenant = await create_tenant(permit, teardown, "outside-tenant") + role = await create_role(permit, teardown) + user_key = unique_key("member") + register_user_delete(permit, teardown, user_key) + answered: set[str] = set() + + async def user_tenants( + user: User = user_key, context: dict[str, Any] | None = None + ) -> dict[str, Any]: + tenants = await tenants_of(permit, user, context) + answered.update(tenants) + return tenants + + await permit.api.tenants.create_user(member_tenant, {"key": user_key}) + await assign_role(permit, teardown, user_key, role, role_tenant) + + # The user is a member of both tenants, and holds a role in one of them only. + expected: dict[str, Any] = {role_tenant: {}} + assert await settled(user_tenants, expected=expected) == expected + + await assign_role(permit, teardown, user_key, role, member_tenant) + + expected = {member_tenant: REGION, role_tenant: {}} + assert await settled(user_tenants, expected=expected) == expected + # The answer depends on the user's key alone: the user's attributes and the query's + # context do not change it. + with_attributes: dict[str, Any] = {"key": user_key, "attributes": {"department": "eng"}} + assert await settled(lambda: user_tenants(with_attributes), expected=expected) == expected + assert ( + await settled(lambda: user_tenants(user_key, {"source": "e2e"}), expected=expected) + == expected + ) + + await permit.api.tenants.delete_tenant_user(member_tenant, user_key) + + expected = {role_tenant: {}} + assert await settled(user_tenants, expected=expected) == expected + + await permit.api.tenants.delete_tenant_user(role_tenant, user_key) + + expected = {} + assert await settled(user_tenants, expected=expected) == expected + assert outside_tenant not in answered + + +@pytest.mark.usefixtures("container_pdp") +def test_the_blocking_client_adds_a_member_and_lists_their_tenants( + sync_permit: SyncPermit, +) -> None: + api = sync_permit.api + tenant = unique_key("member-tenant") + role = unique_key("member-role") + user_key = unique_key("member") + + def user_tenants() -> dict[str, Any]: + return {found.key: found.attributes for found in sync_permit.get_user_tenants(user_key)} + + with ExitStack() as teardown: + teardown.callback( + delete_quietly_blocking, + functools.partial(api.tenants.delete, tenant), + f"tenant '{tenant}'", + ) + api.tenants.create({"key": tenant, "name": tenant, "attributes": REGION}) + teardown.callback( + delete_quietly_blocking, functools.partial(api.roles.delete, role), f"role '{role}'" + ) + api.roles.create({"key": role, "name": role}) + teardown.callback( + delete_quietly_blocking, + functools.partial(api.users.delete, user_key), + f"user '{user_key}'", + ) + + member = api.tenants.create_user(tenant, {"key": user_key}) + + assert member.key == user_key + assert tenant_roles(member) == [(tenant, [])] + assignment = {"user": user_key, "role": role, "tenant": tenant} + teardown.callback( + delete_quietly_blocking, + functools.partial(api.users.unassign_role, assignment), + f"role assignment {assignment}", + ) + api.users.assign_role(assignment) + listed = api.tenants.list_tenant_users(tenant) + assert [(user.key, tenant_roles(user)) for user in listed.data] == [ + (user_key, [(tenant, [role])]) + ] + expected: dict[str, Any] = {tenant: REGION} + assert poll_for_blocking(user_tenants, expected) == expected + + api.tenants.delete_tenant_user(tenant, user_key) + + assert api.tenants.list_tenant_users(tenant).data == [] + expected = {} + assert poll_for_blocking(user_tenants, expected) == expected diff --git a/tests/test_tenant_membership_offline.py b/tests/test_tenant_membership_offline.py new file mode 100644 index 0000000..fe12ec4 --- /dev/null +++ b/tests/test_tenant_membership_offline.py @@ -0,0 +1,490 @@ +"""Offline tests for tenant membership (PER-16678): tenants.create_user() and get_user_tenants(). + +Each call goes through the async and the blocking client, and the test checks the request +it puts on the wire (method, path, query string, headers and JSON body) and what the +response parses into. Every request is served by a local ``pytest_httpserver`` and the API +context is pre-populated, so no API key and no ``/v2/api-key/scope`` lookup are needed. +""" + +import asyncio +import inspect +from operator import attrgetter +from typing import Any, NamedTuple + +import pytest +from pydantic.v1 import ValidationError +from pytest_httpserver import HTTPServer +from werkzeug import Request + +from permit import Permit, TenantDetails, UserCreate, UserRead +from permit.config import PermitConfig +from permit.enforcement.enforcer import Enforcer +from permit.exceptions import ( + PermitAlreadyExistsError, + PermitApiError, + PermitConnectionError, + PermitContextError, + PermitNotFoundError, +) +from permit.sync import Permit as SyncPermit +from tests.utils import FACTS, ORG, PROJECT, Call, call, sent + +FLAVOURS = ["async", "sync"] + +# The headers the SDK sets. The wait-for-sync ones are listed so that sending one shows. +HEADERS = ("Authorization", "Content-Type", "X-Wait-Timeout", "X-Timeout-Policy") +JSON_HEADERS: dict[str, str | None] = { + "Authorization": "Bearer test-token", + "Content-Type": "application/json", + "X-Wait-Timeout": None, + "X-Timeout-Policy": None, +} + + +class Case(NamedTuple): + """One SDK call, and the path and JSON body of the one request it must send.""" + + call: Call + path: str + body: Any + + +def invoke(config: PermitConfig, flavour: str, target: Call) -> object: + """Call ``permit.`` on the async or the blocking client.""" + permit = Permit(config) if flavour == "async" else SyncPermit(config) + result = attrgetter(target.path)(permit)(*target.args, **target.kwargs) + if flavour == "async": + return asyncio.run(result) + assert not inspect.isawaitable(result) + return result + + +def sent_headers(request: Request) -> dict[str, str | None]: + return {name: request.headers.get(name) for name in HEADERS} + + +# --- get_user_tenants() ----------------------------------------------------------- + + +USER_TENANTS = "/user-tenants" +PDP_TENANTS = [ + {"key": "t1", "attributes": {"tier": "gold", "region": {"name": "eu"}}}, + {"key": "t2", "attributes": {}}, + {"key": "t3"}, +] +ATTRIBUTES = {"dept": "eng", "level": 3} + + +GET_USER_TENANTS_CASES = { + "str-user": Case( + call("get_user_tenants", "alice"), + USER_TENANTS, + {"user": {"key": "alice"}, "context": {}}, + ), + "str-user-context": Case( + call("get_user_tenants", "alice", {"region": "eu"}), + USER_TENANTS, + {"user": {"key": "alice"}, "context": {"region": "eu"}}, + ), + "dict-user-attributes": Case( + call("get_user_tenants", {"key": "alice", "attributes": ATTRIBUTES}), + USER_TENANTS, + {"user": {"key": "alice", "attributes": ATTRIBUTES}, "context": {}}, + ), + "dict-user-attributes-context": Case( + call( + "get_user_tenants", + user={"key": "alice", "attributes": ATTRIBUTES}, + context={"region": "eu"}, + ), + USER_TENANTS, + {"user": {"key": "alice", "attributes": ATTRIBUTES}, "context": {"region": "eu"}}, + ), + "dict-user-aliases": Case( + call( + "get_user_tenants", + {"key": "alice", "firstName": "Alice", "lastName": "Smith", "email": "a@example.com"}, + ), + USER_TENANTS, + { + "user": { + "key": "alice", + "first_name": "Alice", + "last_name": "Smith", + "email": "a@example.com", + }, + "context": {}, + }, + ), +} + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("case", GET_USER_TENANTS_CASES.values(), ids=GET_USER_TENANTS_CASES.keys()) +def test_get_user_tenants_posts_the_user_and_context_to_the_pdp( + httpserver: HTTPServer, config: PermitConfig, case: Case, flavour: str +) -> None: + httpserver.expect_request(case.path, method="POST").respond_with_json(PDP_TENANTS) + + invoke(config, flavour, case.call) + + assert [sent(request) for request, _ in httpserver.log] == [ + {"method": "POST", "path": case.path, "query": [], "body": case.body} + ] + assert [sent_headers(request) for request, _ in httpserver.log] == [JSON_HEADERS] + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_get_user_tenants_returns_the_pdp_tenants_as_tenant_details( + httpserver: HTTPServer, config: PermitConfig, flavour: str +) -> None: + """A tenant the PDP sends without attributes gets an empty dict.""" + httpserver.expect_request(USER_TENANTS, method="POST").respond_with_json(PDP_TENANTS) + + result = invoke(config, flavour, call("get_user_tenants", "alice")) + + assert type(result) is list + assert [type(tenant) for tenant in result] == [TenantDetails] * 3 + assert result == [ + TenantDetails(key="t1", attributes={"tier": "gold", "region": {"name": "eu"}}), + TenantDetails(key="t2", attributes={}), + TenantDetails(key="t3", attributes={}), + ] + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_get_user_tenants_returns_an_empty_list_for_a_user_in_no_tenant( + httpserver: HTTPServer, config: PermitConfig, flavour: str +) -> None: + httpserver.expect_request(USER_TENANTS, method="POST").respond_with_json([]) + + assert invoke(config, flavour, call("get_user_tenants", "nobody")) == [] + + +async def test_get_user_tenants_merges_the_query_context_over_the_context_store( + httpserver: HTTPServer, config: PermitConfig +) -> None: + enforcer = Enforcer(config) + enforcer.context_store.add({"region": "eu", "flags": {"a": 1}}) + httpserver.expect_request(USER_TENANTS, method="POST").respond_with_json([]) + + await enforcer.get_user_tenants("alice", {"flags": {"b": 2}}) + + assert [sent(request)["body"] for request, _ in httpserver.log] == [ + {"user": {"key": "alice"}, "context": {"region": "eu", "flags": {"a": 1, "b": 2}}} + ] + assert enforcer.context_store.get_derived_context({}) == {"region": "eu", "flags": {"a": 1}} + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize( + ("body", "content_type"), + [("", None), ('{"detail": "Not Found"}', "application/json")], + ids=["empty", "json"], +) +def test_a_pdp_answering_404_raises_a_connection_error_that_asks_for_a_container_pdp( + httpserver: HTTPServer, + config: PermitConfig, + body: str, + content_type: str | None, + flavour: str, +) -> None: + """The cloud PDP does not serve /user-tenants and answers 404 for it.""" + httpserver.expect_request(USER_TENANTS, method="POST").respond_with_data( + body, status=404, content_type=content_type + ) + + with pytest.raises(PermitConnectionError) as raised: + invoke(config, flavour, call("get_user_tenants", "alice")) + + message = str(raised.value) + assert "got status code 404 from the PDP" in message + assert "only the container PDP serves /user-tenants, and the cloud PDP does not" in message + assert raised.value.original_error is None + assert len(httpserver.log) == 1 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("status", [401, 500]) +@pytest.mark.parametrize( + ("pdp_path", "target"), + [ + (USER_TENANTS, call("get_user_tenants", "alice")), + ("/user-permissions", call("get_user_permissions", "alice")), + ], + ids=["get_user_tenants", "get_user_permissions"], +) +def test_another_pdp_error_status_raises_a_connection_error_as_get_user_permissions_does( + *, + httpserver: HTTPServer, + config: PermitConfig, + pdp_path: str, + target: Call, + status: int, + flavour: str, +) -> None: + httpserver.expect_request(pdp_path, method="POST").respond_with_json( + {"detail": "boom"}, status=status + ) + + with pytest.raises(PermitConnectionError) as raised: + invoke(config, flavour, target) + + message = str(raised.value) + assert f"got an unexpected status code: {status}" in message + assert "container PDP serves" not in message + assert raised.value.original_error is None + assert len(httpserver.log) == 1 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_get_user_tenants_puts_the_pdp_error_body_in_the_error( + httpserver: HTTPServer, config: PermitConfig, flavour: str +) -> None: + httpserver.expect_request(USER_TENANTS, method="POST").respond_with_json( + {"detail": "policy engine unavailable"}, status=500 + ) + + with pytest.raises(PermitConnectionError) as raised: + invoke(config, flavour, call("get_user_tenants", "alice")) + + assert "Response body: {'detail': 'policy engine unavailable'}" in str(raised.value) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_get_user_tenants_raises_a_connection_error_when_the_pdp_is_unreachable( + config: PermitConfig, flavour: str +) -> None: + config.pdp = "http://localhost:1" + + with pytest.raises( + PermitConnectionError, match="cannot connect to the PDP container" + ) as raised: + invoke(config, flavour, call("get_user_tenants", "alice")) + + assert raised.value.original_error is not None + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize( + "user", [{"attributes": ATTRIBUTES}, {"key": None}], ids=["no-key", "null-key"] +) +def test_get_user_tenants_rejects_a_user_without_a_key_before_sending( + httpserver: HTTPServer, config: PermitConfig, user: dict[str, Any], flavour: str +) -> None: + with pytest.raises(ValidationError): + invoke(config, flavour, call("get_user_tenants", user)) + + assert httpserver.log == [] + + +# --- tenants.create_user() ----------------------------------------------------------- + + +NOW = "2024-01-01T00:00:00+00:00" +TENANT_ID = "00000000-0000-4000-8000-000000000020" +TENANT_USERS = f"{FACTS}/tenants/t1/users" + +NEW_USER = { + "key": "alice", + "email": "alice@example.com", + "first_name": "Alice", + "attributes": {"dept": "eng"}, +} +USER_WITH_ROLES = {"key": "alice", "role_assignments": [{"role": "viewer", "tenant": "t2"}]} +USER_READ = { + "key": "alice", + "id": "00000000-0000-4000-8000-000000000021", + "organization_id": "00000000-0000-4000-8000-000000000022", + "project_id": "00000000-0000-4000-8000-000000000023", + "environment_id": "00000000-0000-4000-8000-000000000024", + "associated_tenants": [{"tenant": "t1", "roles": [], "status": "active"}], + "roles": [], + "created_at": NOW, + "updated_at": NOW, + "email": "alice@example.com", + "first_name": "Alice", + "attributes": {"dept": "eng"}, +} + +CREATE_USER_CASES = { + "model": Case( + call("api.tenants.create_user", "t1", UserCreate(**NEW_USER)), TENANT_USERS, NEW_USER + ), + "dict": Case(call("api.tenants.create_user", "t1", NEW_USER), TENANT_USERS, NEW_USER), + "key-only": Case( + call("api.tenants.create_user", "t1", {"key": "bob"}), TENANT_USERS, {"key": "bob"} + ), + "tenant-id-keywords": Case( + call("api.tenants.create_user", tenant_key=TENANT_ID, user_data={"key": "alice"}), + f"{FACTS}/tenants/{TENANT_ID}/users", + {"key": "alice"}, + ), + "role-assignments": Case( + call("api.tenants.create_user", "t1", USER_WITH_ROLES), TENANT_USERS, USER_WITH_ROLES + ), +} + + +@pytest.fixture +def pdp_server(httpserver_ipv4: HTTPServer) -> HTTPServer: + """A server of its own for the PDP, so a request reaching it is told from one to the API.""" + return httpserver_ipv4 + + +@pytest.fixture +def split_config(config: PermitConfig, pdp_server: HTTPServer) -> PermitConfig: + """The offline config with the API on ``httpserver`` and the PDP on ``pdp_server``.""" + config.pdp = pdp_server.url_for("").rstrip("/") + return config + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("proxy_facts_via_pdp", [False, True], ids=["api", "proxy-via-pdp"]) +@pytest.mark.parametrize("case", CREATE_USER_CASES.values(), ids=CREATE_USER_CASES.keys()) +def test_create_user_posts_the_user_to_the_api( + *, + httpserver: HTTPServer, + pdp_server: HTTPServer, + split_config: PermitConfig, + case: Case, + proxy_facts_via_pdp: bool, + flavour: str, +) -> None: + """create_user() goes to the Permit REST API whether or not facts are proxied via the PDP.""" + split_config.proxy_facts_via_pdp = proxy_facts_via_pdp + httpserver.expect_request(case.path, method="POST").respond_with_json(USER_READ) + + result = invoke(split_config, flavour, case.call) + + assert [sent(request) for request, _ in httpserver.log] == [ + {"method": "POST", "path": case.path, "query": [], "body": case.body} + ] + assert [sent_headers(request) for request, _ in httpserver.log] == [JSON_HEADERS] + assert pdp_server.log == [] + assert type(result) is UserRead + assert result == UserRead.parse_obj(USER_READ) + + +ADD_USER_WARNING = ( + r"^permit\.api\.tenants\.add_user\(\) is deprecated and will be removed in permit 4\.0; " + r"use permit\.api\.tenants\.create_user\(\) instead\.$" +) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_add_user_warns_at_the_call_and_sends_what_create_user_sends( + httpserver: HTTPServer, config: PermitConfig, flavour: str +) -> None: + """add_user() is a deprecated alias of create_user(): one warning, then the same request.""" + httpserver.expect_request(TENANT_USERS, method="POST").respond_with_json(USER_READ) + + async def call_awaiting() -> UserRead: + return await Permit(config).api.tenants.add_user("t1", NEW_USER) + + def call_blocking() -> UserRead: + return SyncPermit(config).api.tenants.add_user("t1", NEW_USER) + + with pytest.warns(DeprecationWarning, match=ADD_USER_WARNING) as caught: + result = asyncio.run(call_awaiting()) if flavour == "async" else call_blocking() + + assert [warning.filename for warning in caught] == [__file__] + assert [sent(request) for request, _ in httpserver.log] == [ + {"method": "POST", "path": TENANT_USERS, "query": [], "body": NEW_USER} + ] + assert result == UserRead.parse_obj(USER_READ) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize( + ("proxy_facts_via_pdp", "path"), + [(False, f"{TENANT_USERS}/alice"), (True, "/facts/tenants/t1/users/alice")], + ids=["api", "proxy-via-pdp"], +) +def test_delete_tenant_user_still_follows_proxy_facts_via_pdp( + *, + httpserver: HTTPServer, + pdp_server: HTTPServer, + split_config: PermitConfig, + proxy_facts_via_pdp: bool, + path: str, + flavour: str, +) -> None: + """The tenants API's other calls keep the routing create_user() opts out of.""" + split_config.proxy_facts_via_pdp = proxy_facts_via_pdp + server, other = (pdp_server, httpserver) if proxy_facts_via_pdp else (httpserver, pdp_server) + server.expect_request(path, method="DELETE").respond_with_data("", status=204) + + invoke(split_config, flavour, call("api.tenants.delete_tenant_user", "t1", "alice")) + + assert [sent(request) for request, _ in server.log] == [ + {"method": "DELETE", "path": path, "query": [], "body": None} + ] + assert other.log == [] + + +class ApiError(NamedTuple): + """An error status, the error code the API sends with it, and what the SDK raises.""" + + status: int + error_code: str + raises: type[PermitApiError] + + +API_ERRORS = { + "tenant-not-found": ApiError(404, "NOT_FOUND", PermitNotFoundError), + "user-exists": ApiError(409, "DUPLICATE_ENTITY", PermitAlreadyExistsError), +} + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("error", API_ERRORS.values(), ids=API_ERRORS.keys()) +def test_create_user_raises_the_matching_permit_api_error( + httpserver: HTTPServer, config: PermitConfig, error: ApiError, flavour: str +) -> None: + detail = { + "id": "request-1", + "title": f"status {error.status}", + "error_code": error.error_code, + "message": f"status {error.status}", + } + httpserver.expect_request(TENANT_USERS, method="POST").respond_with_json( + detail, status=error.status + ) + + with pytest.raises(PermitApiError) as raised: + invoke(config, flavour, call("api.tenants.create_user", "t1", {"key": "alice"})) + + assert type(raised.value) is error.raises + assert raised.value.status_code == error.status + assert raised.value.details == detail + assert len(httpserver.log) == 1 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_create_user_refuses_a_project_context_before_sending( + httpserver: HTTPServer, config: PermitConfig, flavour: str +) -> None: + """A project-level key needs the SDK's API context set to an environment first.""" + config.api_context._save_api_key_accessible_scope(org=ORG, project=PROJECT) + config.api_context.set_project_level_context(ORG, PROJECT) + + with pytest.raises(PermitContextError): + invoke(config, flavour, call("api.tenants.create_user", "t1", {"key": "alice"})) + + assert httpserver.log == [] + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize( + "user_data", + [{"email": "alice@example.com"}, {"key": "has space"}, {"key": "alice", "email": "nope"}], + ids=["no-key", "invalid-key", "invalid-email"], +) +def test_create_user_rejects_an_invalid_user_before_sending( + httpserver: HTTPServer, config: PermitConfig, user_data: dict[str, Any], flavour: str +) -> None: + with pytest.raises(ValidationError): + invoke(config, flavour, call("api.tenants.create_user", "t1", user_data)) + + assert httpserver.log == [] diff --git a/tests/type_check/consumer.py b/tests/type_check/consumer.py index 3413fcf..1d63fd8 100644 --- a/tests/type_check/consumer.py +++ b/tests/type_check/consumer.py @@ -11,7 +11,15 @@ from typing_extensions import assert_type -from permit import Permit, PermitApiError, PermitConfig, UserCreate, UserInput, UserRead +from permit import ( + Permit, + PermitApiError, + PermitConfig, + TenantDetails, + UserCreate, + UserInput, + UserRead, +) from permit.api.elements import UserLoginAsResponse from permit.api.models import ( BulkRoleAssignmentReport, @@ -70,6 +78,16 @@ async def async_client() -> None: list[bool], ) assert_type(await permit.get_user_permissions("u"), dict[str, Any]) + tenants = await permit.get_user_tenants("u") + assert_type(tenants, list[TenantDetails]) + assert_type(tenants[0].key, str) + assert_type(tenants[0].attributes, dict[str, Any]) + assert_type( + await permit.get_user_tenants( + {"key": "u", "attributes": {"dept": "eng"}}, context={"region": "eu"} + ), + list[TenantDetails], + ) # Optional model fields are optional to the type checker too. user = UserCreate(key="u") @@ -94,6 +112,9 @@ async def async_client() -> None: await permit.api.users.bulk_create([user, {"key": "u3"}]), UserCreateBulkOperationResult ) assert_type(await permit.api.tenants.create(tenant), TenantRead) + assert_type(await permit.api.tenants.create_user("t1", user), UserRead) + assert_type(await permit.api.tenants.create_user("t1", {"key": "u6"}), UserRead) + assert_type(await permit.api.tenants.add_user("t1", {"key": "u6"}), UserRead) await permit.api.tenants.bulk_create([{"key": "t2", "name": "T2"}]) assert_type(await permit.api.roles.create(role), RoleRead) await permit.api.resources.create( @@ -158,10 +179,15 @@ def sync_client() -> None: assert_type(permit.check("user", "read", "document"), bool) assert_type(permit.get_user_permissions("u"), dict[str, Any]) + assert_type(permit.get_user_tenants("u"), list[TenantDetails]) + assert_type(permit.get_user_tenants({"key": "u"}, {"region": "eu"}), list[TenantDetails]) assert_type(permit.api.users.get("u"), UserRead) assert_type(permit.api.users.list(), PaginatedResultUserRead) assert_type(permit.api.tenants.create(TenantCreate(key="t1", name="T1")), TenantRead) assert_type(permit.api.tenants.list(), list[TenantRead]) + assert_type(permit.api.tenants.create_user("t1", {"key": "u6"}), UserRead) + assert_type(permit.api.tenants.create_user("t1", UserCreate(key="u7")), UserRead) + assert_type(permit.api.tenants.add_user("t1", {"key": "u6"}), UserRead) assert_type(permit.api.users.create({"key": "u2"}), UserRead) permit.api.users.assign_role({"user": "u", "role": "admin", "tenant": "t1"}) permit.api.users.bulk_create([UserCreate(key="u3"), {"key": "u4"}]) @@ -191,9 +217,11 @@ async def mistakes_stay_errors() -> None: UserInput(key="u", firstname="A") # type: ignore[call-arg] # Accepting dicts does not mean accepting anything. await permit.api.users.create("u") # type: ignore[arg-type] + await permit.api.tenants.create_user("t1", "u") # type: ignore[arg-type] # SDK models are pydantic v1 models, so the pydantic v2 API does not exist on them. UserCreate(key="u").model_dump() # type: ignore[attr-defined] # The blocking client returns values, not awaitables. await sync_permit.api.users.get("u") # type: ignore[misc] + await sync_permit.get_user_tenants("u") # type: ignore[misc] # The async client returns awaitables, not values. _ = permit.api.users.get("u").email # type: ignore[attr-defined] diff --git a/tests/utils.py b/tests/utils.py index 9cafabf..66229e6 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -63,6 +63,10 @@ def sent(request: Request) -> dict[str, Any]: # --- end-to-end tests --------------------------------------------------------- +# The hosted cloud PDP. conftest.py's fixtures can default to it, and the e2e tests that +# run only on it, or never on it, compare the PDP address they are given with it. +CLOUD_PDP_URL = "https://cloudpdp.api.permit.io" + def handle_api_error(error: PermitApiError, message: str) -> None: err = ( @@ -110,6 +114,14 @@ async def delete_quietly(delete: Callable[[], Awaitable[None]], description: str handle_cleanup_error(error, f"could not delete {description}") +def delete_quietly_blocking(delete: Callable[[], None], description: str) -> None: + """Delete one object at teardown through the blocking client, as ``delete_quietly``.""" + try: + delete() + except PermitApiError as error: + handle_cleanup_error(error, f"could not delete {description}") + + T = TypeVar("T")