From 2834eadfd5ca13732e24d96482135c940dba5543 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 06:39:27 +0300 Subject: [PATCH 01/13] Wire-test the schema APIs on the async and the blocking client tests/test_schema_offline.py calls every public method of the resources, resource_attributes, resource_relations, resource_roles, roles, condition_sets and condition_set_rules APIs through both clients, each closed after the call. It checks the request each sends (method, path, query string, headers and JSON body), the type the response parses into, the Permit*Error an API error response raises, and that with proxy_facts_via_pdp on the requests still go to the API. The 27 schema operations these tests are the first to send are covered now, so their `untested` entries leave the API coverage allowlist (PER-16177). Co-Authored-By: Claude Opus 5.5 --- .github/scripts/api_coverage_allowlist.json | 216 ---- tests/test_schema_offline.py | 1208 +++++++++++++++++++ 2 files changed, 1208 insertions(+), 216 deletions(-) create mode 100644 tests/test_schema_offline.py diff --git a/.github/scripts/api_coverage_allowlist.json b/.github/scripts/api_coverage_allowlist.json index 851b1729..95394fcf 100644 --- a/.github/scripts/api_coverage_allowlist.json +++ b/.github/scripts/api_coverage_allowlist.json @@ -648,30 +648,6 @@ "ticket": "PER-16177", "reason": "Called by permit.api.role_assignments.bulk_unassign(); no offline test sends this request yet." }, - { - "api": "control-plane", - "operation": "GET /v2/facts/{proj_id}/{env_id}/set_rules", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_set_rules.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/set_rules", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_set_rules.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/set_rules", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_set_rules.delete(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/facts/{proj_id}/{env_id}/tenants/{tenant_id}/users", @@ -1288,46 +1264,6 @@ "ticket": "PER-16737", "reason": "P2: roles.bulk_replace()." }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/condition_sets", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_sets.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/schema/{proj_id}/{env_id}/condition_sets", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_sets.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/condition_sets/{condition_set_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_sets.get(), permit.api.condition_sets.get_by_id(), permit.api.condition_sets.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/schema/{proj_id}/{env_id}/condition_sets/{condition_set_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_sets.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PATCH /v2/schema/{proj_id}/{env_id}/condition_sets/{condition_set_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.condition_sets.update(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/schema/{proj_id}/{env_id}/condition_sets/{condition_set_id}/ancestors", @@ -1392,118 +1328,6 @@ "ticket": "PER-16342", "reason": "The spec's summary labels it EAP while its tag is GA; add it once the label is settled." }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/resources", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resources.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PUT /v2/schema/{proj_id}/{env_id}/resources/{resource_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resources.replace(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/attributes", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_attributes.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/attributes", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_attributes.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/attributes/{attribute_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_attributes.get(), permit.api.resource_attributes.get_by_id(), permit.api.resource_attributes.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/attributes/{attribute_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_attributes.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PATCH /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/attributes/{attribute_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_attributes.update(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/relations", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_relations.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/relations/{relation_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_relations.get(), permit.api.resource_relations.get_by_id(), permit.api.resource_relations.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/relations/{relation_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_relations.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.get(), permit.api.resource_roles.get_by_id(), permit.api.resource_roles.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PATCH /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.update(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}/ancestors", @@ -1520,30 +1344,6 @@ "ticket": "PER-16337", "reason": "Hierarchy helper for the dashboard, derivable from extends and parent_id; no SDK has it and nobody has asked." }, - { - "api": "control-plane", - "operation": "POST /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}/implicit_grants", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.create_role_derivation(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}/implicit_grants", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.delete_role_derivation(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PUT /v2/schema/{proj_id}/{env_id}/resources/{resource_id}/roles/{role_id}/implicit_grants/conditions", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_roles.update_role_derivation_conditions(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/schema/{proj_id}/{env_id}/roles/{role_id}/ancestors", @@ -1560,22 +1360,6 @@ "ticket": "PER-16337", "reason": "Hierarchy helper for the dashboard, derivable from extends and parent_id; no SDK has it and nobody has asked." }, - { - "api": "control-plane", - "operation": "POST /v2/schema/{proj_id}/{env_id}/roles/{role_id}/permissions", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.roles.assign_permissions(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/schema/{proj_id}/{env_id}/roles/{role_id}/permissions", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.roles.remove_permissions(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/schema/{proj_id}/{env_id}/users/attributes", diff --git a/tests/test_schema_offline.py b/tests/test_schema_offline.py new file mode 100644 index 00000000..3aa90f34 --- /dev/null +++ b/tests/test_schema_offline.py @@ -0,0 +1,1208 @@ +"""Offline tests for the schema APIs of permit.api (PER-16177). + +The APIs are ``resources``, ``resource_attributes``, ``resource_relations``, +``resource_roles``, ``roles``, ``condition_sets`` and ``condition_set_rules``. Every public +method is called through the async and the blocking client, each closed once the call +returns, and the test checks the request it puts on the wire (method, path, query string, +headers and JSON body), the type the response parses into, and the error that an API error +response raises. + +None of these methods follows ``proxy_facts_via_pdp``: with it on, their requests still go +to the API, ``condition_set_rules`` included, whose routes are under ``/v2/facts``. 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 BaseModel +from pytest_httpserver import HTTPServer +from werkzeug import Request + +from permit import Permit +from permit.api.condition_set_rules import ConditionSetRulesApi +from permit.api.condition_sets import ConditionSetsApi +from permit.api.models import ( + AttributeType, + ConditionSetCreate, + ConditionSetRead, + ConditionSetRuleCreate, + ConditionSetRuleRead, + ConditionSetRuleRemove, + ConditionSetType, + ConditionSetUpdate, + DerivedRoleRuleCreate, + DerivedRoleRuleDelete, + DerivedRoleRuleRead, + PaginatedResultRelationRead, + PermitBackendSchemasSchemaDerivedRoleRuleDerivationSettings, + RelationCreate, + RelationRead, + ResourceAttributeCreate, + ResourceAttributeRead, + ResourceAttributeUpdate, + ResourceCreate, + ResourceRead, + ResourceReplace, + ResourceRoleCreate, + ResourceRoleRead, + ResourceRoleUpdate, + ResourceUpdate, + RoleCreate, + RoleRead, + RoleUpdate, +) +from permit.api.resource_attributes import ResourceAttributesApi +from permit.api.resource_relations import ResourceRelationsApi +from permit.api.resource_roles import ResourceRolesApi +from permit.api.resources import ResourcesApi +from permit.api.roles import RolesApi +from permit.config import PermitConfig +from permit.exceptions import ( + PermitAlreadyExistsError, + PermitApiDetailedError, + PermitApiError, + PermitNotFoundError, + PermitValidationError, +) +from permit.sync import Permit as SyncPermit +from tests.utils import FACTS, SCHEMA, Call, call, offline_config, sent + +FLAVOURS = ["async", "sync"] + +# The schema APIs, by the attribute of permit.api that holds each. +SCHEMA_APIS: dict[str, type] = { + "resources": ResourcesApi, + "resource_attributes": ResourceAttributesApi, + "resource_relations": ResourceRelationsApi, + "resource_roles": ResourceRolesApi, + "roles": RolesApi, + "condition_sets": ConditionSetsApi, + "condition_set_rules": ConditionSetRulesApi, +} + +RESOURCES = f"{SCHEMA}/resources" +DOCUMENT = f"{RESOURCES}/document" +ROLES = f"{SCHEMA}/roles" +CONDITION_SETS = f"{SCHEMA}/condition_sets" +SET_RULES = f"{FACTS}/set_rules" + +TIMESTAMP = "2026-01-01T00:00:00+00:00" +RESOURCE_ID = "6a1b2c3d-0000-4000-8000-000000000101" +FOLDER_ID = "6a1b2c3d-0000-4000-8000-000000000102" +ACTION_ID = "6a1b2c3d-0000-4000-8000-000000000103" +ATTRIBUTE_ID = "6a1b2c3d-0000-4000-8000-000000000104" +RELATION_ID = "6a1b2c3d-0000-4000-8000-000000000105" +ROLE_ID = "6a1b2c3d-0000-4000-8000-000000000106" +CONDITION_SET_ID = "6a1b2c3d-0000-4000-8000-000000000107" +RULE_ID = "6a1b2c3d-0000-4000-8000-000000000108" + +DEFAULT_PAGE = [("page", "1"), ("per_page", "100")] +SECOND_PAGE = [("page", "2"), ("per_page", "10")] + +# 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, +} + + +def read(object_id: str, **fields: Any) -> dict[str, Any]: + """A read model's JSON: its ``object_id``, ``fields``, and what every read model has.""" + return { + "id": object_id, + "organization_id": "6a1b2c3d-0000-4000-8000-000000000001", + "project_id": "6a1b2c3d-0000-4000-8000-000000000002", + "environment_id": "6a1b2c3d-0000-4000-8000-000000000003", + "created_at": TIMESTAMP, + "updated_at": TIMESTAMP, + **fields, + } + + +# --- what the API answers with --------------------------------------------------------- + +RESOURCE = read( + RESOURCE_ID, + key="document", + name="Document", + actions={"read": {"id": ACTION_ID, "key": "read", "name": "Read"}}, +) +ATTRIBUTE = read( + ATTRIBUTE_ID, + key="owner", + type="string", + resource_id=RESOURCE_ID, + resource_key="document", + built_in=False, +) +RELATION = read( + RELATION_ID, + key="parent", + name="Parent", + subject_resource="folder", + subject_resource_id=FOLDER_ID, + object_resource="document", + object_resource_id=RESOURCE_ID, +) +RELATION_PAGE = {"data": [RELATION], "total_count": 1, "page_count": 1} +RESOURCE_ROLE = read( + ROLE_ID, + key="editor", + name="Editor", + resource="document", + resource_id=RESOURCE_ID, + permissions=["document:read", "document:write"], +) +DERIVATION = { + "role_id": ROLE_ID, + "resource_id": FOLDER_ID, + "relation_id": RELATION_ID, + "role": "editor", + "on_resource": "folder", + "linked_by_relation": "parent", + "when": {"no_direct_roles_on_object": False}, +} +DERIVATION_SETTINGS = {"no_direct_roles_on_object": True} +ROLE = read(ROLE_ID, key="admin", name="Admin", permissions=["document:read"]) +CONDITIONS = {"allOf": [{"user.location": {"equals": "US"}}]} +CONDITION_SET = read( + CONDITION_SET_ID, key="us_employees", name="US employees", type="userset", conditions=CONDITIONS +) +RULE = read( + RULE_ID, + key="us_employees_document:read_private_docs", + user_set="us_employees", + permission="document:read", + resource_set="private_docs", +) + +# --- what the SDK sends ---------------------------------------------------------------- + +NEW_RESOURCE = { + "key": "document", + "name": "Document", + "actions": {"read": {}, "write": {"name": "Write"}}, + "attributes": {"owner": {"type": "string"}}, +} +REPLACEMENT = { + "name": "Document", + "actions": {"read": {}}, + "roles": {"viewer": {"name": "Viewer", "permissions": ["read"]}}, + "relations": {"parent": "folder"}, +} +NEW_ATTRIBUTE = {"key": "owner", "type": "string", "description": "Who owns the document"} +NEW_RELATION = {"key": "parent", "name": "Parent", "subject_resource": "folder"} +NEW_RESOURCE_ROLE = {"key": "editor", "name": "Editor", "permissions": ["read", "write"]} +DERIVED_RESOURCE_ROLE = { + "key": "editor", + "name": "Editor", + "extends": ["viewer"], + "granted_to": { + "users_with_role": [ + {"role": "editor", "on_resource": "folder", "linked_by_relation": "parent"} + ] + }, +} +DERIVATION_RULE = {"role": "editor", "on_resource": "folder", "linked_by_relation": "parent"} +NEW_ROLE = {"key": "admin", "name": "Admin", "permissions": ["document:read"]} +NEW_USER_SET = {"key": "us_employees", "name": "US employees", "type": "userset"} +NEW_RESOURCE_SET = { + "key": "private_docs", + "name": "Private documents", + "type": "resourceset", + "resource_id": "document", + "conditions": {"allOf": [{"resource.private": {"equals": True}}]}, +} +SET_RULE = { + "user_set": "us_employees", + "permission": "document:read", + "resource_set": "private_docs", +} + + +class Case(NamedTuple): + """One SDK call and the one request it must send. + + ``response`` is the JSON the server answers with, or None for a 204 with no body. + ``model`` is what the response parses into, or None when the method returns nothing; a + list response parses into a list of ``model``. + """ + + call: Call + method: str + path: str + query: list[tuple[str, str]] + body: Any + response: dict[str, Any] | list[dict[str, Any]] | None + model: type[BaseModel] | None + + @property + def request(self) -> dict[str, Any]: + """The request, as ``tests.utils.sent()`` shows it.""" + return {"method": self.method, "path": self.path, "query": self.query, "body": self.body} + + +CASES = { + # resources + "resources.list": Case( + call=call("resources.list"), + method="GET", + path=RESOURCES, + query=DEFAULT_PAGE, + body=None, + response=[RESOURCE], + model=ResourceRead, + ), + "resources.list-page": Case( + call=call("resources.list", page=2, per_page=10), + method="GET", + path=RESOURCES, + query=SECOND_PAGE, + body=None, + response=[], + model=ResourceRead, + ), + "resources.get": Case( + call=call("resources.get", "document"), + method="GET", + path=DOCUMENT, + query=[], + body=None, + response=RESOURCE, + model=ResourceRead, + ), + "resources.get_by_key": Case( + call=call("resources.get_by_key", "document"), + method="GET", + path=DOCUMENT, + query=[], + body=None, + response=RESOURCE, + model=ResourceRead, + ), + "resources.get_by_id": Case( + call=call("resources.get_by_id", RESOURCE_ID), + method="GET", + path=f"{RESOURCES}/{RESOURCE_ID}", + query=[], + body=None, + response=RESOURCE, + model=ResourceRead, + ), + "resources.create": Case( + call=call("resources.create", ResourceCreate(**NEW_RESOURCE)), + method="POST", + path=RESOURCES, + query=[], + body=NEW_RESOURCE, + response=RESOURCE, + model=ResourceRead, + ), + "resources.create-dict": Case( + call=call("resources.create", NEW_RESOURCE), + method="POST", + path=RESOURCES, + query=[], + body=NEW_RESOURCE, + response=RESOURCE, + model=ResourceRead, + ), + "resources.update": Case( + call=call("resources.update", "document", ResourceUpdate(name="Doc")), + method="PATCH", + path=DOCUMENT, + query=[], + body={"name": "Doc"}, + response=RESOURCE, + model=ResourceRead, + ), + "resources.update-clears-a-field": Case( + call=call("resources.update", "document", {"description": None}), + method="PATCH", + path=DOCUMENT, + query=[], + body={"description": None}, + response=RESOURCE, + model=ResourceRead, + ), + "resources.replace": Case( + call=call("resources.replace", "document", ResourceReplace(**REPLACEMENT)), + method="PUT", + path=DOCUMENT, + query=[], + body=REPLACEMENT, + response=RESOURCE, + model=ResourceRead, + ), + "resources.replace-dict": Case( + call=call("resources.replace", "document", REPLACEMENT), + method="PUT", + path=DOCUMENT, + query=[], + body=REPLACEMENT, + response=RESOURCE, + model=ResourceRead, + ), + "resources.delete": Case( + call=call("resources.delete", "document"), + method="DELETE", + path=DOCUMENT, + query=[], + body=None, + response=None, + model=None, + ), + # resource_attributes + "resource_attributes.list": Case( + call=call("resource_attributes.list", "document"), + method="GET", + path=f"{DOCUMENT}/attributes", + query=DEFAULT_PAGE, + body=None, + response=[ATTRIBUTE], + model=ResourceAttributeRead, + ), + "resource_attributes.list-page": Case( + call=call("resource_attributes.list", "document", page=2, per_page=10), + method="GET", + path=f"{DOCUMENT}/attributes", + query=SECOND_PAGE, + body=None, + response=[], + model=ResourceAttributeRead, + ), + "resource_attributes.get": Case( + call=call("resource_attributes.get", "document", "owner"), + method="GET", + path=f"{DOCUMENT}/attributes/owner", + query=[], + body=None, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.get_by_key": Case( + call=call("resource_attributes.get_by_key", "document", "owner"), + method="GET", + path=f"{DOCUMENT}/attributes/owner", + query=[], + body=None, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.get_by_id": Case( + call=call("resource_attributes.get_by_id", RESOURCE_ID, ATTRIBUTE_ID), + method="GET", + path=f"{RESOURCES}/{RESOURCE_ID}/attributes/{ATTRIBUTE_ID}", + query=[], + body=None, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.create": Case( + call=call( + "resource_attributes.create", + "document", + ResourceAttributeCreate( + key="owner", type=AttributeType.string, description="Who owns the document" + ), + ), + method="POST", + path=f"{DOCUMENT}/attributes", + query=[], + body=NEW_ATTRIBUTE, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.create-dict": Case( + call=call("resource_attributes.create", "document", NEW_ATTRIBUTE), + method="POST", + path=f"{DOCUMENT}/attributes", + query=[], + body=NEW_ATTRIBUTE, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.update": Case( + call=call( + "resource_attributes.update", + "document", + "owner", + ResourceAttributeUpdate(type=AttributeType.array), + ), + method="PATCH", + path=f"{DOCUMENT}/attributes/owner", + query=[], + body={"type": "array"}, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.update-clears-a-field": Case( + call=call("resource_attributes.update", "document", "owner", {"description": None}), + method="PATCH", + path=f"{DOCUMENT}/attributes/owner", + query=[], + body={"description": None}, + response=ATTRIBUTE, + model=ResourceAttributeRead, + ), + "resource_attributes.delete": Case( + call=call("resource_attributes.delete", "document", "owner"), + method="DELETE", + path=f"{DOCUMENT}/attributes/owner", + query=[], + body=None, + response=None, + model=None, + ), + # resource_relations + "resource_relations.list": Case( + call=call("resource_relations.list", "document"), + method="GET", + path=f"{DOCUMENT}/relations", + query=DEFAULT_PAGE, + body=None, + response=RELATION_PAGE, + model=PaginatedResultRelationRead, + ), + "resource_relations.list-page": Case( + call=call("resource_relations.list", "document", page=2, per_page=10), + method="GET", + path=f"{DOCUMENT}/relations", + query=SECOND_PAGE, + body=None, + response={"data": [], "total_count": 1, "page_count": 1}, + model=PaginatedResultRelationRead, + ), + "resource_relations.get": Case( + call=call("resource_relations.get", "document", "parent"), + method="GET", + path=f"{DOCUMENT}/relations/parent", + query=[], + body=None, + response=RELATION, + model=RelationRead, + ), + "resource_relations.get_by_key": Case( + call=call("resource_relations.get_by_key", "document", "parent"), + method="GET", + path=f"{DOCUMENT}/relations/parent", + query=[], + body=None, + response=RELATION, + model=RelationRead, + ), + "resource_relations.get_by_id": Case( + call=call("resource_relations.get_by_id", RESOURCE_ID, RELATION_ID), + method="GET", + path=f"{RESOURCES}/{RESOURCE_ID}/relations/{RELATION_ID}", + query=[], + body=None, + response=RELATION, + model=RelationRead, + ), + "resource_relations.create": Case( + call=call("resource_relations.create", "document", RelationCreate(**NEW_RELATION)), + method="POST", + path=f"{DOCUMENT}/relations", + query=[], + body=NEW_RELATION, + response=RELATION, + model=RelationRead, + ), + "resource_relations.create-dict": Case( + call=call("resource_relations.create", "document", NEW_RELATION), + method="POST", + path=f"{DOCUMENT}/relations", + query=[], + body=NEW_RELATION, + response=RELATION, + model=RelationRead, + ), + "resource_relations.delete": Case( + call=call("resource_relations.delete", "document", "parent"), + method="DELETE", + path=f"{DOCUMENT}/relations/parent", + query=[], + body=None, + response=None, + model=None, + ), + # resource_roles + "resource_roles.list": Case( + call=call("resource_roles.list", "document"), + method="GET", + path=f"{DOCUMENT}/roles", + query=DEFAULT_PAGE, + body=None, + response=[RESOURCE_ROLE], + model=ResourceRoleRead, + ), + "resource_roles.list-page": Case( + call=call("resource_roles.list", "document", page=2, per_page=10), + method="GET", + path=f"{DOCUMENT}/roles", + query=SECOND_PAGE, + body=None, + response=[], + model=ResourceRoleRead, + ), + "resource_roles.get": Case( + call=call("resource_roles.get", "document", "editor"), + method="GET", + path=f"{DOCUMENT}/roles/editor", + query=[], + body=None, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.get_by_key": Case( + call=call("resource_roles.get_by_key", "document", "editor"), + method="GET", + path=f"{DOCUMENT}/roles/editor", + query=[], + body=None, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.get_by_id": Case( + call=call("resource_roles.get_by_id", RESOURCE_ID, ROLE_ID), + method="GET", + path=f"{RESOURCES}/{RESOURCE_ID}/roles/{ROLE_ID}", + query=[], + body=None, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.create": Case( + call=call("resource_roles.create", "document", ResourceRoleCreate(**NEW_RESOURCE_ROLE)), + method="POST", + path=f"{DOCUMENT}/roles", + query=[], + body=NEW_RESOURCE_ROLE, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.create-derived-dict": Case( + call=call("resource_roles.create", "document", DERIVED_RESOURCE_ROLE), + method="POST", + path=f"{DOCUMENT}/roles", + query=[], + body=DERIVED_RESOURCE_ROLE, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.update": Case( + call=call( + "resource_roles.update", + "document", + "editor", + ResourceRoleUpdate(permissions=["read"]), + ), + method="PATCH", + path=f"{DOCUMENT}/roles/editor", + query=[], + body={"permissions": ["read"]}, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.update-clears-a-field": Case( + call=call("resource_roles.update", "document", "editor", {"description": None}), + method="PATCH", + path=f"{DOCUMENT}/roles/editor", + query=[], + body={"description": None}, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.delete": Case( + call=call("resource_roles.delete", "document", "editor"), + method="DELETE", + path=f"{DOCUMENT}/roles/editor", + query=[], + body=None, + response=None, + model=None, + ), + "resource_roles.assign_permissions": Case( + call=call("resource_roles.assign_permissions", "document", "editor", ["read", "write"]), + method="POST", + path=f"{DOCUMENT}/roles/editor/permissions", + query=[], + body={"permissions": ["read", "write"]}, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.remove_permissions": Case( + call=call("resource_roles.remove_permissions", "document", "editor", ["write"]), + method="DELETE", + path=f"{DOCUMENT}/roles/editor/permissions", + query=[], + body={"permissions": ["write"]}, + response=RESOURCE_ROLE, + model=ResourceRoleRead, + ), + "resource_roles.create_role_derivation": Case( + call=call( + "resource_roles.create_role_derivation", + "document", + "editor", + DerivedRoleRuleCreate(**DERIVATION_RULE), + ), + method="POST", + path=f"{DOCUMENT}/roles/editor/implicit_grants", + query=[], + body=DERIVATION_RULE, + response=DERIVATION, + model=DerivedRoleRuleRead, + ), + "resource_roles.create_role_derivation-dict": Case( + call=call( + "resource_roles.create_role_derivation", + "document", + "editor", + {**DERIVATION_RULE, "when": DERIVATION_SETTINGS}, + ), + method="POST", + path=f"{DOCUMENT}/roles/editor/implicit_grants", + query=[], + body={**DERIVATION_RULE, "when": DERIVATION_SETTINGS}, + response=DERIVATION, + model=DerivedRoleRuleRead, + ), + "resource_roles.delete_role_derivation": Case( + call=call( + "resource_roles.delete_role_derivation", + "document", + "editor", + DerivedRoleRuleDelete(**DERIVATION_RULE), + ), + method="DELETE", + path=f"{DOCUMENT}/roles/editor/implicit_grants", + query=[], + body=DERIVATION_RULE, + response=None, + model=None, + ), + "resource_roles.delete_role_derivation-dict": Case( + call=call("resource_roles.delete_role_derivation", "document", "editor", DERIVATION_RULE), + method="DELETE", + path=f"{DOCUMENT}/roles/editor/implicit_grants", + query=[], + body=DERIVATION_RULE, + response=None, + model=None, + ), + "resource_roles.update_role_derivation_conditions": Case( + call=call( + "resource_roles.update_role_derivation_conditions", + "document", + "editor", + PermitBackendSchemasSchemaDerivedRoleRuleDerivationSettings( + no_direct_roles_on_object=True + ), + ), + method="PUT", + path=f"{DOCUMENT}/roles/editor/implicit_grants/conditions", + query=[], + body=DERIVATION_SETTINGS, + response=DERIVATION_SETTINGS, + model=PermitBackendSchemasSchemaDerivedRoleRuleDerivationSettings, + ), + "resource_roles.update_role_derivation_conditions-dict": Case( + call=call( + "resource_roles.update_role_derivation_conditions", + "document", + "editor", + {"no_direct_roles_on_object": False}, + ), + method="PUT", + path=f"{DOCUMENT}/roles/editor/implicit_grants/conditions", + query=[], + body={"no_direct_roles_on_object": False}, + response={"no_direct_roles_on_object": False}, + model=PermitBackendSchemasSchemaDerivedRoleRuleDerivationSettings, + ), + # roles + "roles.list": Case( + call=call("roles.list"), + method="GET", + path=ROLES, + query=DEFAULT_PAGE, + body=None, + response=[ROLE], + model=RoleRead, + ), + "roles.list-page": Case( + call=call("roles.list", page=2, per_page=10), + method="GET", + path=ROLES, + query=SECOND_PAGE, + body=None, + response=[], + model=RoleRead, + ), + "roles.get": Case( + call=call("roles.get", "admin"), + method="GET", + path=f"{ROLES}/admin", + query=[], + body=None, + response=ROLE, + model=RoleRead, + ), + "roles.get_by_key": Case( + call=call("roles.get_by_key", "admin"), + method="GET", + path=f"{ROLES}/admin", + query=[], + body=None, + response=ROLE, + model=RoleRead, + ), + "roles.get_by_id": Case( + call=call("roles.get_by_id", ROLE_ID), + method="GET", + path=f"{ROLES}/{ROLE_ID}", + query=[], + body=None, + response=ROLE, + model=RoleRead, + ), + "roles.create": Case( + call=call("roles.create", RoleCreate(**NEW_ROLE)), + method="POST", + path=ROLES, + query=[], + body=NEW_ROLE, + response=ROLE, + model=RoleRead, + ), + "roles.create-dict": Case( + call=call("roles.create", {**NEW_ROLE, "extends": ["viewer"]}), + method="POST", + path=ROLES, + query=[], + body={**NEW_ROLE, "extends": ["viewer"]}, + response=ROLE, + model=RoleRead, + ), + "roles.update": Case( + call=call("roles.update", "admin", RoleUpdate(name="Administrator")), + method="PATCH", + path=f"{ROLES}/admin", + query=[], + body={"name": "Administrator"}, + response=ROLE, + model=RoleRead, + ), + "roles.update-clears-a-field": Case( + call=call("roles.update", "admin", {"description": None}), + method="PATCH", + path=f"{ROLES}/admin", + query=[], + body={"description": None}, + response=ROLE, + model=RoleRead, + ), + "roles.delete": Case( + call=call("roles.delete", "admin"), + method="DELETE", + path=f"{ROLES}/admin", + query=[], + body=None, + response=None, + model=None, + ), + "roles.assign_permissions": Case( + call=call("roles.assign_permissions", "admin", ["document:read", "document:write"]), + method="POST", + path=f"{ROLES}/admin/permissions", + query=[], + body={"permissions": ["document:read", "document:write"]}, + response=ROLE, + model=RoleRead, + ), + "roles.remove_permissions": Case( + call=call("roles.remove_permissions", "admin", ["document:write"]), + method="DELETE", + path=f"{ROLES}/admin/permissions", + query=[], + body={"permissions": ["document:write"]}, + response=ROLE, + model=RoleRead, + ), + # condition_sets + "condition_sets.list": Case( + call=call("condition_sets.list"), + method="GET", + path=CONDITION_SETS, + query=DEFAULT_PAGE, + body=None, + response=[CONDITION_SET], + model=ConditionSetRead, + ), + "condition_sets.list-page": Case( + call=call("condition_sets.list", page=2, per_page=10), + method="GET", + path=CONDITION_SETS, + query=SECOND_PAGE, + body=None, + response=[], + model=ConditionSetRead, + ), + "condition_sets.get": Case( + call=call("condition_sets.get", "us_employees"), + method="GET", + path=f"{CONDITION_SETS}/us_employees", + query=[], + body=None, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.get_by_key": Case( + call=call("condition_sets.get_by_key", "us_employees"), + method="GET", + path=f"{CONDITION_SETS}/us_employees", + query=[], + body=None, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.get_by_id": Case( + call=call("condition_sets.get_by_id", CONDITION_SET_ID), + method="GET", + path=f"{CONDITION_SETS}/{CONDITION_SET_ID}", + query=[], + body=None, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.create": Case( + call=call( + "condition_sets.create", + ConditionSetCreate( + key="us_employees", + name="US employees", + type=ConditionSetType.userset, + conditions=CONDITIONS, + ), + ), + method="POST", + path=CONDITION_SETS, + query=[], + body={**NEW_USER_SET, "conditions": CONDITIONS}, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.create-resource-set-dict": Case( + call=call("condition_sets.create", NEW_RESOURCE_SET), + method="POST", + path=CONDITION_SETS, + query=[], + body=NEW_RESOURCE_SET, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.update": Case( + call=call( + "condition_sets.update", "us_employees", ConditionSetUpdate(conditions=CONDITIONS) + ), + method="PATCH", + path=f"{CONDITION_SETS}/us_employees", + query=[], + body={"conditions": CONDITIONS}, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.update-clears-a-field": Case( + call=call("condition_sets.update", "us_employees", {"description": None}), + method="PATCH", + path=f"{CONDITION_SETS}/us_employees", + query=[], + body={"description": None}, + response=CONDITION_SET, + model=ConditionSetRead, + ), + "condition_sets.delete": Case( + call=call("condition_sets.delete", "us_employees"), + method="DELETE", + path=f"{CONDITION_SETS}/us_employees", + query=[], + body=None, + response=None, + model=None, + ), + # condition_set_rules + "condition_set_rules.list": Case( + call=call("condition_set_rules.list"), + method="GET", + path=SET_RULES, + query=DEFAULT_PAGE, + body=None, + response=[RULE], + model=ConditionSetRuleRead, + ), + "condition_set_rules.list-filtered": Case( + call=call( + "condition_set_rules.list", + user_set_key="us_employees", + permission_key="document:read", + resource_set_key="private_docs", + ), + method="GET", + path=SET_RULES, + query=[ + ("page", "1"), + ("per_page", "100"), + ("permission", "document:read"), + ("resource_set", "private_docs"), + ("user_set", "us_employees"), + ], + body=None, + response=[RULE], + model=ConditionSetRuleRead, + ), + "condition_set_rules.list-page": Case( + call=call("condition_set_rules.list", page=2, per_page=10), + method="GET", + path=SET_RULES, + query=SECOND_PAGE, + body=None, + response=[], + model=ConditionSetRuleRead, + ), + "condition_set_rules.create": Case( + call=call("condition_set_rules.create", ConditionSetRuleCreate(**SET_RULE)), + method="POST", + path=SET_RULES, + query=[], + body=SET_RULE, + response=[RULE], + model=ConditionSetRuleRead, + ), + "condition_set_rules.create-dict": Case( + call=call("condition_set_rules.create", {**SET_RULE, "is_role": False}), + method="POST", + path=SET_RULES, + query=[], + body={**SET_RULE, "is_role": False}, + response=[RULE], + model=ConditionSetRuleRead, + ), + "condition_set_rules.delete": Case( + call=call("condition_set_rules.delete", ConditionSetRuleRemove(**SET_RULE)), + method="DELETE", + path=SET_RULES, + query=[], + body=SET_RULE, + response=None, + model=None, + ), + "condition_set_rules.delete-dict": Case( + call=call("condition_set_rules.delete", SET_RULE), + method="DELETE", + path=SET_RULES, + query=[], + body=SET_RULE, + response=None, + model=None, + ), +} + +# The case named after each method, for the tests that need one call per method. +BASIC_CASES = {name: case for name, case in CASES.items() if name == case.call.path} + + +def invoke(config: PermitConfig, flavour: str, target: Call) -> object: + """Call ``permit.api.`` on a new async or blocking client, then close it.""" + if flavour == "async": + + async def call_awaiting() -> object: + async with Permit(config) as permit: + method = attrgetter(f"api.{target.path}")(permit) + return await method(*target.args, **target.kwargs) + + return asyncio.run(call_awaiting()) + with SyncPermit(config) as permit: + result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + assert not inspect.isawaitable(result) + return result + + +def respond(server: HTTPServer, case: Case) -> None: + """Make ``server`` answer the request of ``case`` with the case's response.""" + handler = server.expect_request(case.path, method=case.method) + if case.response is None: + handler.respond_with_data("", status=204) + else: + handler.respond_with_json(case.response) + + +def sent_headers(request: Request) -> dict[str, str | None]: + return {name: request.headers.get(name) for name in HEADERS} + + +def test_every_public_method_has_a_case() -> None: + public = { + f"{api}.{name}" + for api, api_class in SCHEMA_APIS.items() + for name, value in vars(api_class).items() + if not name.startswith("_") and callable(value) + } + + assert {case.call.path for case in CASES.values()} == public + assert set(BASIC_CASES) == public + assert len(public) == 52 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) +def test_request_and_response( + httpserver: HTTPServer, config: PermitConfig, case: Case, flavour: str +) -> None: + respond(httpserver, case) + + result = invoke(config, flavour, case.call) + + assert [sent(request) for request, _ in httpserver.log] == [case.request] + assert [sent_headers(request) for request, _ in httpserver.log] == [JSON_HEADERS] + if case.model is None: + assert result is None + elif isinstance(case.response, list): + assert type(result) is list + assert [type(item) for item in result] == [case.model] * len(case.response) + assert result == [case.model.parse_obj(item) for item in case.response] + else: + assert type(result) is case.model + assert result == case.model.parse_obj(case.response) + + +# --- API errors ------------------------------------------------------------------------ + + +class ApiError(NamedTuple): + """An error status, the JSON body the API sends with it, and what the SDK raises.""" + + status: int + body: dict[str, Any] + raises: type[PermitApiError] + + +def error_details(status: int, error_code: str) -> dict[str, Any]: + """The API's body for an error other than a validation error.""" + return { + "id": "request-1", + "title": f"status {status}", + "error_code": error_code, + "message": f"status {status}", + } + + +NOT_FOUND = ApiError(404, error_details(404, "NOT_FOUND"), PermitNotFoundError) +DUPLICATE = ApiError(409, error_details(409, "DUPLICATE_ENTITY"), PermitAlreadyExistsError) +INVALID_PERMISSION = ApiError( + 400, error_details(400, "INVALID_PERMISSION_FORMAT"), PermitApiDetailedError +) + + +def validation_error(location: list[str], message: str, error_type: str) -> ApiError: + """The API's 422 for a request whose input at ``location`` is not valid.""" + body = {"detail": [{"loc": location, "msg": message, "type": error_type}]} + return ApiError(422, body, PermitValidationError) + + +INVALID_PAGE = validation_error( + ["query", "per_page"], "Input should be less than or equal to 100", "less_than_equal" +) +INVALID_BODY = validation_error(["body", "actions"], "Field required", "missing") + +# The error each method's test answers with, by the method's name, so that each error class +# the SDK raises for an API error response is checked. Every other method gets NOT_FOUND. +METHOD_ERRORS = { + "list": INVALID_PAGE, + "replace": INVALID_BODY, + "create": DUPLICATE, + "create_role_derivation": DUPLICATE, + "assign_permissions": INVALID_PERMISSION, + "remove_permissions": INVALID_PERMISSION, +} +ERROR_CASES = { + name: (case, METHOD_ERRORS.get(name.split(".")[1], NOT_FOUND)) + for name, case in BASIC_CASES.items() +} + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize(("case", "error"), ERROR_CASES.values(), ids=ERROR_CASES.keys()) +def test_an_api_error_raises_the_matching_permit_api_error( + httpserver: HTTPServer, config: PermitConfig, case: Case, error: ApiError, flavour: str +) -> None: + httpserver.expect_request(case.path, method=case.method).respond_with_json( + error.body, status=error.status + ) + + with pytest.raises(PermitApiError) as raised: + invoke(config, flavour, case.call) + + assert type(raised.value) is error.raises + assert raised.value.status_code == error.status + assert raised.value.details == error.body + assert [sent(request) for request, _ in httpserver.log] == [case.request] + + +# --- proxy_facts_via_pdp --------------------------------------------------------------- + + +@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 proxy_config(httpserver: HTTPServer, pdp_server: HTTPServer) -> PermitConfig: + """The offline config with ``proxy_facts_via_pdp`` on and the PDP on ``pdp_server``.""" + offline = offline_config(httpserver.url_for("").rstrip("/")) + return PermitConfig( + token=offline.token, + api_url=offline.api_url, + pdp=pdp_server.url_for("").rstrip("/"), + api_context=offline.api_context, + proxy_facts_via_pdp=True, + facts_sync_timeout=2.5, + facts_sync_timeout_policy="fail", + ) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("case", BASIC_CASES.values(), ids=BASIC_CASES.keys()) +def test_proxy_facts_via_pdp_leaves_the_request_on_the_api( + *, + httpserver: HTTPServer, + pdp_server: HTTPServer, + proxy_config: PermitConfig, + case: Case, + flavour: str, +) -> None: + """Only the facts methods send their requests to the PDP; these go to the API. + + The client's wait-for-sync headers go with them, as with every request a client with + ``proxy_facts_via_pdp`` on sends to the API (see ``test_facts_sync_offline.py``). + """ + respond(httpserver, case) + + invoke(proxy_config, flavour, case.call) + + assert [sent(request) for request, _ in httpserver.log] == [case.request] + assert [sent_headers(request) for request, _ in httpserver.log] == [ + {**JSON_HEADERS, "X-Wait-Timeout": "2.5", "X-Timeout-Policy": "fail"} + ] + assert pdp_server.log == [] From 569e33a7789ebe7d26858061f6c1be5cd0496122 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 06:52:42 +0300 Subject: [PATCH 02/13] Wire-test the bulk, instance, tuple, assignment and invite methods Add offline tests for the 22 facts operations the API coverage report listed as untested: the bulk user, tenant and resource instance operations, single and bulk relationship tuple and role assignment writes, resource instance CRUD, tenants.list_tenant_users() and the user invite methods. Each method is called on the async and the blocking client, with proxy_facts_via_pdp off (the API) and on (the PDP's /facts route, or the API for the user invite methods). The tests check the method, path, query, Authorization, Content-Type and wait-for-sync headers and JSON body, the parsed return type, and that the API's error response raises the matching PermitApiError. Their untested entries leave the allowlist (PER-16177). Co-Authored-By: Claude Opus 5.5 --- .github/scripts/api_coverage_allowlist.json | 176 ------ tests/test_facts_operations_offline.py | 612 ++++++++++++++++++++ 2 files changed, 612 insertions(+), 176 deletions(-) create mode 100644 tests/test_facts_operations_offline.py diff --git a/.github/scripts/api_coverage_allowlist.json b/.github/scripts/api_coverage_allowlist.json index 851b1729..ffcb5e28 100644 --- a/.github/scripts/api_coverage_allowlist.json +++ b/.github/scripts/api_coverage_allowlist.json @@ -416,54 +416,6 @@ "ticket": "PER-16337", "reason": "EAP Access Requests; add it once the feature leaves EAP." }, - { - "api": "control-plane", - "operation": "PUT /v2/facts/{proj_id}/{env_id}/bulk/resource_instances", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_instances.bulk_replace(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/bulk/resource_instances", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_instances.bulk_delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/bulk/tenants", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.tenants.bulk_delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PUT /v2/facts/{proj_id}/{env_id}/bulk/users", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.users.bulk_replace(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/bulk/users", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.users.bulk_create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/bulk/users", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.users.bulk_delete(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/facts/{proj_id}/{env_id}/email_configurations", @@ -560,94 +512,6 @@ "ticket": "PER-16337", "reason": "One-time setup (Permit proxy configs), which the CLI and Terraform cover; revisit together with check_url (PER-16737) if there is demand." }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/relationship_tuples", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.relationship_tuples.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/relationship_tuples/bulk", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.relationship_tuples.bulk_create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/relationship_tuples/bulk", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.relationship_tuples.bulk_delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/resource_instances", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_instances.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/facts/{proj_id}/{env_id}/resource_instances/{instance_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_instances.get(), permit.api.resource_instances.get_by_id(), permit.api.resource_instances.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/resource_instances/{instance_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_instances.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PATCH /v2/facts/{proj_id}/{env_id}/resource_instances/{instance_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.resource_instances.update(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/role_assignments", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.role_assignments.assign(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/role_assignments", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.role_assignments.unassign(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/role_assignments/bulk", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.role_assignments.bulk_assign(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/role_assignments/bulk", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.role_assignments.bulk_unassign(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/facts/{proj_id}/{env_id}/set_rules", @@ -672,38 +536,6 @@ "ticket": "PER-16177", "reason": "Called by permit.api.condition_set_rules.delete(); no offline test sends this request yet." }, - { - "api": "control-plane", - "operation": "GET /v2/facts/{proj_id}/{env_id}/tenants/{tenant_id}/users", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.tenants.list_tenant_users(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/facts/{proj_id}/{env_id}/user_invites", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.user_invites.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/user_invites", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.user_invites.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/facts/{proj_id}/{env_id}/user_invites/{user_invite_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.user_invites.delete(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "PATCH /v2/facts/{proj_id}/{env_id}/user_invites/{user_invite_id}", @@ -712,14 +544,6 @@ "ticket": "PER-16737", "reason": "P2: user_invites.update()." }, - { - "api": "control-plane", - "operation": "POST /v2/facts/{proj_id}/{env_id}/user_invites/{user_invite_id}/approve", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.user_invites.approve(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "GET /v2/history", diff --git a/tests/test_facts_operations_offline.py b/tests/test_facts_operations_offline.py new file mode 100644 index 00000000..edbb249b --- /dev/null +++ b/tests/test_facts_operations_offline.py @@ -0,0 +1,612 @@ +"""Offline wire tests for the bulk and single-object facts methods of ``permit.api`` (PER-16177). + +They cover the bulk user, tenant and resource instance methods, the resource instance, +relationship tuple and role assignment methods that write or read one object, the bulk +relationship tuple and role assignment methods, ``tenants.list_tenant_users()`` and the user +invite methods. Each is called through the async and the blocking client, each closed once +the call returns, on both routings: with ``proxy_facts_via_pdp`` off, which sends the +request to the API, and on, which sends it to the PDP's ``/facts`` route (the user invite +methods send it to the API either way). The test checks the request it puts on the wire +(method, path, query string, headers and JSON body), what the response parses into, and +that the API's error response raises the matching ``PermitApiError``. The API and the PDP +are each served by a local ``pytest_httpserver`` of their own 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 BaseModel +from pytest_httpserver import HTTPServer +from werkzeug import Request + +from permit import Permit +from permit.api.models import ( + BulkRoleAssignmentReport, + BulkRoleUnAssignmentReport, + ElementsUserInviteRead, + PaginatedResultElementsUserInviteRead, + PaginatedResultUserRead, + RelationshipTupleCreateBulkOperationResult, + RelationshipTupleDelete, + RelationshipTupleDeleteBulkOperationResult, + ResourceInstanceCreateBulkOperationResult, + ResourceInstanceDeleteBulkOperationResult, + ResourceInstanceRead, + RoleAssignmentRead, + RoleAssignmentRemove, + TenantDeleteBulkOperationResult, + UserCreate, + UserCreateBulkOperationResult, + UserDeleteBulkOperationResult, + UserRead, + UserReplaceBulkOperationResult, +) +from permit.api.user_invites import UserInvitesApi +from permit.config import PermitConfig +from permit.exceptions import ( + PermitAlreadyExistsError, + PermitApiDetailedError, + PermitApiError, + PermitNotFoundError, + PermitValidationError, +) +from permit.sync import Permit as SyncPermit +from tests.facts_methods import ( + ASSIGNMENT, + INSTANCE, + INSTANCE_ID, + RESOURCE_INSTANCE, + ROLE_ASSIGNMENT, + TENANT_ID, + TUPLE, + TUPLE_IDENT, + USER, + USER_ID, + read, +) +from tests.utils import FACTS, Call, call, offline_config, 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") + + +class Routing(NamedTuple): + """The facts options of a client, and the wait-for-sync headers its requests carry.""" + + options: dict[str, Any] + wait_timeout: str | None + timeout_policy: str | None + + +ROUTINGS = { + "api": Routing({}, None, None), + "pdp": Routing( + { + "proxy_facts_via_pdp": True, + "facts_sync_timeout": 2.5, + "facts_sync_timeout_policy": "fail", + }, + "2.5", + "fail", + ), +} + + +class ApiError(NamedTuple): + """An error status, the API's JSON body with it, and the error the SDK raises for it.""" + + status: int + body: dict[str, Any] + raises: type[PermitApiError] + + +def error_details(error_code: str, title: str) -> dict[str, Any]: + """The body the API sends with an error status other than 422.""" + return { + "id": "6a1b2c3d0000400080000000000000ee", + "title": title, + "error_code": error_code, + "message": f"{title}.", + "support_link": "https://docs.permit.io/errors", + } + + +NOT_FOUND = ApiError(404, error_details("NOT_FOUND", "Not found"), PermitNotFoundError) +DUPLICATE = ApiError( + 409, error_details("DUPLICATE_ENTITY", "Already exists"), PermitAlreadyExistsError +) +FORBIDDEN = ApiError(403, error_details("FORBIDDEN_ACCESS", "Forbidden"), PermitApiDetailedError) +INVALID = ApiError( + 422, + {"detail": [{"loc": ["body", "operations"], "msg": "field required", "type": "missing"}]}, + PermitValidationError, +) + + +class Case(NamedTuple): + """One method call, the request it sends on each routing, and what it returns. + + ``call`` is the method's dotted path under ``permit.api``. ``api_path`` is the request's + path on the API, and ``pdp_path`` its path on the PDP with ``proxy_facts_via_pdp`` on, or + None for a method that sends it to the API either way. ``response`` is the JSON the + server answers with, or None for an empty 204, and ``model`` is what the method returns, + or None when it returns nothing. ``error`` is the API error the error test answers with. + """ + + call: Call + method: str + api_path: str + pdp_path: str | None + query: list[tuple[str, str]] + body: Any + response: Any + model: type[BaseModel] | None + error: ApiError + + +DEFAULT_PAGE = [("page", "1"), ("per_page", "100")] +SECOND_PAGE = [("page", "2"), ("per_page", "10")] + +USERS_PAGE = {"data": [USER], "total_count": 1, "page_count": 1} +NEW_USERS = [UserCreate(key="alice", email="alice@example.com"), {"key": "bob"}] +NEW_USERS_SENT = [{"key": "alice", "email": "alice@example.com"}, {"key": "bob"}] +OTHER_ASSIGNMENT = {"user": "bob", "role": "viewer", "tenant": "default"} + +INVITE_ID = "6a1b2c3d-0000-4000-8000-000000000040" +ROLE_ID = "6a1b2c3d-0000-4000-8000-000000000050" +NEW_INVITE = { + "key": "bob", + "status": "pending", + "email": "bob@example.com", + "first_name": "Bob", + "last_name": "Smith", + "role_id": ROLE_ID, + "tenant_id": TENANT_ID, + "resource_instance_id": INSTANCE_ID, +} +INVITE = read(**NEW_INVITE, id=INVITE_ID) +INVITES_PAGE = {"data": [INVITE], "total_count": 1, "page_count": 1} +APPROVAL = {"email": "bob@example.com", "key": "bob", "attributes": {"dept": "eng"}} +INVITES = f"{FACTS}/user_invites" + +CASES = { + # bulk users and tenants + "users.bulk_create": Case( + call=call("users.bulk_create", NEW_USERS), + method="POST", + api_path=f"{FACTS}/bulk/users", + pdp_path="/facts/bulk/users", + query=[], + body={"operations": NEW_USERS_SENT}, + response={}, + model=UserCreateBulkOperationResult, + error=INVALID, + ), + "users.bulk_replace": Case( + call=call("users.bulk_replace", NEW_USERS), + method="PUT", + api_path=f"{FACTS}/bulk/users", + pdp_path="/facts/bulk/users", + query=[], + body={"operations": NEW_USERS_SENT}, + response={}, + model=UserReplaceBulkOperationResult, + error=INVALID, + ), + "users.bulk_delete": Case( + call=call("users.bulk_delete", ["alice", USER_ID]), + method="DELETE", + api_path=f"{FACTS}/bulk/users", + pdp_path="/facts/bulk/users", + query=[], + body={"idents": ["alice", USER_ID]}, + response={}, + model=UserDeleteBulkOperationResult, + error=INVALID, + ), + "tenants.bulk_delete": Case( + call=call("tenants.bulk_delete", ["acme", TENANT_ID]), + method="DELETE", + api_path=f"{FACTS}/bulk/tenants", + pdp_path="/facts/bulk/tenants", + query=[], + body={"idents": ["acme", TENANT_ID]}, + response={}, + model=TenantDeleteBulkOperationResult, + error=INVALID, + ), + # tenant users + "tenants.list_tenant_users": Case( + call=call("tenants.list_tenant_users", "acme"), + method="GET", + api_path=f"{FACTS}/tenants/acme/users", + pdp_path="/facts/tenants/acme/users", + query=DEFAULT_PAGE, + body=None, + response=USERS_PAGE, + model=PaginatedResultUserRead, + error=NOT_FOUND, + ), + "tenants.list_tenant_users-page": Case( + call=call("tenants.list_tenant_users", "acme", page=2, per_page=10), + method="GET", + api_path=f"{FACTS}/tenants/acme/users", + pdp_path="/facts/tenants/acme/users", + query=SECOND_PAGE, + body=None, + response={"data": [], "total_count": 1, "page_count": 1}, + model=PaginatedResultUserRead, + error=NOT_FOUND, + ), + # relationship tuples + "relationship_tuples.delete": Case( + call=call("relationship_tuples.delete", TUPLE_IDENT), + method="DELETE", + api_path=f"{FACTS}/relationship_tuples", + pdp_path="/facts/relationship_tuples", + query=[], + body=TUPLE_IDENT, + response=None, + model=None, + error=NOT_FOUND, + ), + "relationship_tuples.delete-model": Case( + call=call("relationship_tuples.delete", RelationshipTupleDelete(**TUPLE_IDENT)), + method="DELETE", + api_path=f"{FACTS}/relationship_tuples", + pdp_path="/facts/relationship_tuples", + query=[], + body=TUPLE_IDENT, + response=None, + model=None, + error=NOT_FOUND, + ), + "relationship_tuples.bulk_create": Case( + call=call("relationship_tuples.bulk_create", [TUPLE]), + method="POST", + api_path=f"{FACTS}/relationship_tuples/bulk", + pdp_path="/facts/relationship_tuples/bulk", + query=[], + body={"operations": [TUPLE]}, + response={}, + model=RelationshipTupleCreateBulkOperationResult, + error=INVALID, + ), + "relationship_tuples.bulk_delete": Case( + call=call("relationship_tuples.bulk_delete", [TUPLE_IDENT]), + method="DELETE", + api_path=f"{FACTS}/relationship_tuples/bulk", + pdp_path="/facts/relationship_tuples/bulk", + query=[], + body={"idents": [TUPLE_IDENT]}, + response={}, + model=RelationshipTupleDeleteBulkOperationResult, + error=INVALID, + ), + # resource instances + "resource_instances.create": Case( + call=call("resource_instances.create", INSTANCE), + method="POST", + api_path=f"{FACTS}/resource_instances", + pdp_path="/facts/resource_instances", + query=[], + body=INSTANCE, + response=RESOURCE_INSTANCE, + model=ResourceInstanceRead, + error=DUPLICATE, + ), + "resource_instances.get": Case( + call=call("resource_instances.get", "document:readme"), + method="GET", + api_path=f"{FACTS}/resource_instances/document:readme", + pdp_path="/facts/resource_instances/document:readme", + query=[], + body=None, + response=RESOURCE_INSTANCE, + model=ResourceInstanceRead, + error=NOT_FOUND, + ), + "resource_instances.get_by_key": Case( + call=call("resource_instances.get_by_key", "document:readme"), + method="GET", + api_path=f"{FACTS}/resource_instances/document:readme", + pdp_path="/facts/resource_instances/document:readme", + query=[], + body=None, + response=RESOURCE_INSTANCE, + model=ResourceInstanceRead, + error=NOT_FOUND, + ), + "resource_instances.get_by_id": Case( + call=call("resource_instances.get_by_id", INSTANCE_ID), + method="GET", + api_path=f"{FACTS}/resource_instances/{INSTANCE_ID}", + pdp_path=f"/facts/resource_instances/{INSTANCE_ID}", + query=[], + body=None, + response=RESOURCE_INSTANCE, + model=ResourceInstanceRead, + error=NOT_FOUND, + ), + "resource_instances.update": Case( + call=call("resource_instances.update", "document:readme", {"attributes": {"pages": 3}}), + method="PATCH", + api_path=f"{FACTS}/resource_instances/document:readme", + pdp_path="/facts/resource_instances/document:readme", + query=[], + body={"attributes": {"pages": 3}}, + response=RESOURCE_INSTANCE, + model=ResourceInstanceRead, + error=NOT_FOUND, + ), + "resource_instances.delete": Case( + call=call("resource_instances.delete", "document:readme"), + method="DELETE", + api_path=f"{FACTS}/resource_instances/document:readme", + pdp_path="/facts/resource_instances/document:readme", + query=[], + body=None, + response=None, + model=None, + error=NOT_FOUND, + ), + "resource_instances.bulk_replace": Case( + call=call("resource_instances.bulk_replace", [INSTANCE]), + method="PUT", + api_path=f"{FACTS}/bulk/resource_instances", + pdp_path="/facts/bulk/resource_instances", + query=[], + body={"operations": [INSTANCE]}, + response={}, + model=ResourceInstanceCreateBulkOperationResult, + error=INVALID, + ), + "resource_instances.bulk_delete": Case( + call=call("resource_instances.bulk_delete", ["document:readme", INSTANCE_ID]), + method="DELETE", + api_path=f"{FACTS}/bulk/resource_instances", + pdp_path="/facts/bulk/resource_instances", + query=[], + body={"idents": ["document:readme", INSTANCE_ID]}, + response={}, + model=ResourceInstanceDeleteBulkOperationResult, + error=INVALID, + ), + # role assignments + "role_assignments.assign": Case( + call=call("role_assignments.assign", ASSIGNMENT), + method="POST", + api_path=f"{FACTS}/role_assignments", + pdp_path="/facts/role_assignments", + query=[], + body=ASSIGNMENT, + response=ROLE_ASSIGNMENT, + model=RoleAssignmentRead, + error=DUPLICATE, + ), + "role_assignments.unassign": Case( + call=call("role_assignments.unassign", RoleAssignmentRemove(**ASSIGNMENT)), + method="DELETE", + api_path=f"{FACTS}/role_assignments", + pdp_path="/facts/role_assignments", + query=[], + body=ASSIGNMENT, + response=None, + model=None, + error=NOT_FOUND, + ), + "role_assignments.bulk_assign": Case( + call=call("role_assignments.bulk_assign", [ASSIGNMENT, OTHER_ASSIGNMENT]), + method="POST", + api_path=f"{FACTS}/role_assignments/bulk", + pdp_path="/facts/role_assignments/bulk", + query=[], + body=[ASSIGNMENT, OTHER_ASSIGNMENT], + response={"assignments_created": 2}, + model=BulkRoleAssignmentReport, + error=INVALID, + ), + "role_assignments.bulk_unassign": Case( + call=call("role_assignments.bulk_unassign", [ASSIGNMENT, OTHER_ASSIGNMENT]), + method="DELETE", + api_path=f"{FACTS}/role_assignments/bulk", + pdp_path="/facts/role_assignments/bulk", + query=[], + body=[ASSIGNMENT, OTHER_ASSIGNMENT], + response={"assignments_removed": 2}, + model=BulkRoleUnAssignmentReport, + error=INVALID, + ), + # user invites, which go to the API with or without proxy_facts_via_pdp + "user_invites.list": Case( + call=call("user_invites.list"), + method="GET", + api_path=INVITES, + pdp_path=None, + query=DEFAULT_PAGE, + body=None, + response=INVITES_PAGE, + model=PaginatedResultElementsUserInviteRead, + error=FORBIDDEN, + ), + "user_invites.list-page": Case( + call=call("user_invites.list", page=2, per_page=10), + method="GET", + api_path=INVITES, + pdp_path=None, + query=SECOND_PAGE, + body=None, + response={"data": [], "total_count": 1, "page_count": 1}, + model=PaginatedResultElementsUserInviteRead, + error=FORBIDDEN, + ), + "user_invites.get": Case( + call=call("user_invites.get", INVITE_ID), + method="GET", + api_path=f"{INVITES}/{INVITE_ID}", + pdp_path=None, + query=[], + body=None, + response=INVITE, + model=ElementsUserInviteRead, + error=NOT_FOUND, + ), + "user_invites.create": Case( + call=call("user_invites.create", NEW_INVITE), + method="POST", + api_path=INVITES, + pdp_path=None, + query=[], + body=NEW_INVITE, + response=INVITE, + model=ElementsUserInviteRead, + error=DUPLICATE, + ), + "user_invites.delete": Case( + call=call("user_invites.delete", INVITE_ID), + method="DELETE", + api_path=f"{INVITES}/{INVITE_ID}", + pdp_path=None, + query=[], + body=None, + response=None, + model=None, + error=NOT_FOUND, + ), + "user_invites.approve": Case( + call=call("user_invites.approve", INVITE_ID, APPROVAL), + method="POST", + api_path=f"{INVITES}/{INVITE_ID}/approve", + pdp_path=None, + query=[], + body=APPROVAL, + response={**USER, "key": "bob", "email": "bob@example.com", "attributes": {"dept": "eng"}}, + model=UserRead, + error=NOT_FOUND, + ), +} + + +@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 + + +def make_config(api: HTTPServer, pdp: HTTPServer, routing: Routing) -> PermitConfig: + """An offline config for the API on ``api`` and the PDP on ``pdp``, routed by ``routing``.""" + offline = offline_config(api.url_for("").rstrip("/")) + return PermitConfig( + token=offline.token, + api_url=offline.api_url, + pdp=pdp.url_for("").rstrip("/"), + api_context=offline.api_context, + **routing.options, + ) + + +async def _invoke_async(config: PermitConfig, target: Call) -> object: + async with Permit(config) as permit: + return await attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + + +def invoke(config: PermitConfig, flavour: str, target: Call) -> object: + """Call ``permit.api.`` on a new async or blocking client, then close it.""" + if flavour == "async": + return asyncio.run(_invoke_async(config, target)) + with SyncPermit(config) as permit: + result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + assert not inspect.isawaitable(result) + return result + + +def destination( + case: Case, routing: str, api: HTTPServer, pdp: HTTPServer +) -> tuple[HTTPServer, str, HTTPServer]: + """The server the request must reach, its path there, and the server it must not reach.""" + if routing == "pdp" and case.pdp_path is not None: + return pdp, case.pdp_path, api + return api, case.api_path, pdp + + +def sent_headers(request: Request) -> dict[str, str | None]: + return {name: request.headers.get(name) for name in HEADERS} + + +def expected_headers(routing: Routing) -> dict[str, str | None]: + return { + "Authorization": "Bearer test-token", + "Content-Type": "application/json", + "X-Wait-Timeout": routing.wait_timeout, + "X-Timeout-Policy": routing.timeout_policy, + } + + +def test_every_public_user_invites_method_has_a_case() -> None: + public = { + f"user_invites.{name}" + for name, value in vars(UserInvitesApi).items() + if not name.startswith("_") and callable(value) + } + + assert {case.call.path for case in CASES.values() if case.pdp_path is None} == public + assert len(public) == 5 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("routing", ROUTINGS) +@pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) +def test_request_and_response( + *, httpserver: HTTPServer, pdp_server: HTTPServer, case: Case, routing: str, flavour: str +) -> None: + config = make_config(httpserver, pdp_server, ROUTINGS[routing]) + server, path, other = destination(case, routing, httpserver, pdp_server) + handler = server.expect_request(path, method=case.method) + if case.response is None: + handler.respond_with_data("", status=204) + else: + handler.respond_with_json(case.response) + + result = invoke(config, flavour, case.call) + + assert [sent(request) for request, _ in server.log] == [ + {"method": case.method, "path": path, "query": case.query, "body": case.body} + ] + assert [sent_headers(request) for request, _ in server.log] == [ + expected_headers(ROUTINGS[routing]) + ] + assert other.log == [] + if case.model is None: + assert result is None + else: + assert type(result) is case.model + assert result == case.model.parse_obj(case.response) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("routing", ROUTINGS) +@pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) +def test_an_api_error_raises_the_matching_permit_api_error( + *, httpserver: HTTPServer, pdp_server: HTTPServer, case: Case, routing: str, flavour: str +) -> None: + """Through the PDP too: its ``/facts`` routes pass the API's error response on.""" + config = make_config(httpserver, pdp_server, ROUTINGS[routing]) + server, path, other = destination(case, routing, httpserver, pdp_server) + server.expect_request(path, method=case.method).respond_with_json( + case.error.body, status=case.error.status + ) + + with pytest.raises(PermitApiError) as raised: + invoke(config, flavour, case.call) + + assert type(raised.value) is case.error.raises + assert raised.value.status_code == case.error.status + assert raised.value.details == case.error.body + assert len(server.log) == 1 + assert other.log == [] From 9f78c47e8ac324f389e43ffd61d3ddaa08c8329b Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 06:52:50 +0300 Subject: [PATCH 03/13] Wire-test the projects and environments APIs Add offline tests for every public method of permit.api.projects and permit.api.environments, which covers the 11 project and environment operations the API coverage report listed as untested, the stats one included. Each method is called on the async and the blocking client with an API key of the narrowest level it accepts. The tests check the method, path, query, headers and JSON body, the parsed return type, that the API's error response raises the matching PermitApiError, and that a key one level narrower is refused before anything is sent. Their untested entries leave the allowlist (PER-16177). Co-Authored-By: Claude Opus 5.5 --- .github/scripts/api_coverage_allowlist.json | 88 ---- tests/test_projects_environments_offline.py | 504 ++++++++++++++++++++ 2 files changed, 504 insertions(+), 88 deletions(-) create mode 100644 tests/test_projects_environments_offline.py diff --git a/.github/scripts/api_coverage_allowlist.json b/.github/scripts/api_coverage_allowlist.json index ffcb5e28..26579b11 100644 --- a/.github/scripts/api_coverage_allowlist.json +++ b/.github/scripts/api_coverage_allowlist.json @@ -912,86 +912,6 @@ "ticket": "PER-16337", "reason": "EAP Policy Guards, organization-level policy setup." }, - { - "api": "control-plane", - "operation": "GET /v2/projects", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.projects.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/projects", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.projects.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/projects/{proj_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.projects.get(), permit.api.projects.get_by_id(), permit.api.projects.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/projects/{proj_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.projects.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PATCH /v2/projects/{proj_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.projects.update(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/projects/{proj_id}/envs", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.environments.list(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "POST /v2/projects/{proj_id}/envs", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.environments.create(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "GET /v2/projects/{proj_id}/envs/{env_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.environments.get(), permit.api.environments.get_by_id(), permit.api.environments.get_by_key(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "DELETE /v2/projects/{proj_id}/envs/{env_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.environments.delete(); no offline test sends this request yet." - }, - { - "api": "control-plane", - "operation": "PATCH /v2/projects/{proj_id}/envs/{env_id}", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.environments.update(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "POST /v2/projects/{proj_id}/envs/{env_id}/copy/async", @@ -1008,14 +928,6 @@ "ticket": "PER-16737", "reason": "P2: environments.copy_async() and get_copy_result()." }, - { - "api": "control-plane", - "operation": "GET /v2/projects/{proj_id}/envs/{env_id}/stats", - "stage": "GA", - "status": "untested", - "ticket": "PER-16177", - "reason": "Called by permit.api.environments.get_stats(); no offline test sends this request yet." - }, { "api": "control-plane", "operation": "POST /v2/projects/{proj_id}/envs/{env_id}/test_jwks", diff --git a/tests/test_projects_environments_offline.py b/tests/test_projects_environments_offline.py new file mode 100644 index 00000000..b7abe8d7 --- /dev/null +++ b/tests/test_projects_environments_offline.py @@ -0,0 +1,504 @@ +"""Offline wire tests for permit.api.projects and permit.api.environments (PER-16177). + +Every public method is called through the async and the blocking client, each closed once +the call returns, with an API key of the narrowest level the method accepts. The test checks +the request it puts on the wire (method, path, query string, headers and JSON body), what +the response parses into, that the API's error response raises the matching +``PermitApiError``, and that a key one level narrower is refused before anything is sent. +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 +import re +from operator import attrgetter +from typing import Any, NamedTuple + +import pytest +from pydantic.v1 import BaseModel +from pytest_httpserver import HTTPServer +from werkzeug import Request + +from permit import Permit +from permit.api.context import API_ACCESS_LEVELS, ApiKeyAccessLevel +from permit.api.environments import EnvironmentsApi +from permit.api.models import ( + APIKeyRead, + EnvironmentRead, + EnvironmentStats, + ProjectCreate, + ProjectRead, +) +from permit.api.projects import ProjectsApi +from permit.config import PermitConfig +from permit.exceptions import ( + PermitAlreadyExistsError, + PermitApiDetailedError, + PermitApiError, + PermitContextError, + PermitNotFoundError, +) +from permit.sync import Permit as SyncPermit +from tests.utils import ORG, PROJECT, Call, call, offline_config, sent + +FLAVOURS = ["async", "sync"] +ORGANIZATION_KEY = ApiKeyAccessLevel.ORGANIZATION_LEVEL_API_KEY +PROJECT_KEY = ApiKeyAccessLevel.PROJECT_LEVEL_API_KEY +ENVIRONMENT_KEY = ApiKeyAccessLevel.ENVIRONMENT_LEVEL_API_KEY + +# 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 ApiError(NamedTuple): + """An error status, the API's JSON body with it, and the error the SDK raises for it.""" + + status: int + body: dict[str, Any] + raises: type[PermitApiError] + + +def error_details(error_code: str, title: str) -> dict[str, Any]: + return { + "id": "6a1b2c3d0000400080000000000000ee", + "title": title, + "error_code": error_code, + "message": f"{title}.", + "support_link": "https://docs.permit.io/errors", + } + + +NOT_FOUND = ApiError(404, error_details("NOT_FOUND", "Not found"), PermitNotFoundError) +DUPLICATE = ApiError( + 409, error_details("DUPLICATE_ENTITY", "Already exists"), PermitAlreadyExistsError +) +FORBIDDEN = ApiError(403, error_details("FORBIDDEN_ACCESS", "Forbidden"), PermitApiDetailedError) + + +class Case(NamedTuple): + """One method call, the request it must send, and what it returns. + + ``call`` is the method's dotted path under ``permit.api``, and ``key`` the narrowest API + key level the method accepts. ``response`` is the JSON the server answers with, or None + for an empty 204; ``model`` is what it parses into (each item's, for a list), or None + when the method returns nothing. ``error`` is the API error the error test answers with. + """ + + call: Call + key: ApiKeyAccessLevel + method: str + path: str + query: list[tuple[str, str]] + body: Any + response: Any + model: type[BaseModel] | None + error: ApiError + + +TIMESTAMP = "2026-01-01T00:00:00+00:00" +ORGANIZATION_ID = "6a1b2c3d-0000-4000-8000-000000000001" +PROJECT_ID = "6a1b2c3d-0000-4000-8000-000000000002" +ENVIRONMENT_ID = "6a1b2c3d-0000-4000-8000-000000000003" +DEFAULT_PAGE = [("page", "1"), ("per_page", "100")] +SECOND_PAGE = [("page", "2"), ("per_page", "10")] + +PROJECTS = "/v2/projects" +ENVS = f"{PROJECTS}/web/envs" + +PROJECT_READ = { + "key": "web", + "name": "Web", + "id": PROJECT_ID, + "organization_id": ORGANIZATION_ID, + "created_at": TIMESTAMP, + "updated_at": TIMESTAMP, +} +ENVIRONMENT_READ = { + "key": "dev", + "name": "Development", + "id": ENVIRONMENT_ID, + "organization_id": ORGANIZATION_ID, + "project_id": PROJECT_ID, + "created_at": TIMESTAMP, + "updated_at": TIMESTAMP, +} +ENVIRONMENT_STATS = { + **ENVIRONMENT_READ, + "pdp_configs": [ + { + "id": "6a1b2c3d-0000-4000-8000-000000000060", + "organization_id": ORGANIZATION_ID, + "project_id": PROJECT_ID, + "environment_id": ENVIRONMENT_ID, + "client_secret": "test-client-secret", + } + ], + "stats": { + "roles": 2, + "users": 3, + "policies": 4, + "resources": 1, + "tenants": 1, + "has_decision_logs": True, + "members": [], + "mau": 3, + }, +} +API_KEY = { + "id": "6a1b2c3d-0000-4000-8000-000000000070", + "organization_id": ORGANIZATION_ID, + "project_id": PROJECT_ID, + "environment_id": ENVIRONMENT_ID, + "owner_type": "pdp_config", + "secret": "test-api-key-secret", + "created_at": TIMESTAMP, +} +COPY = {"target_env": {"existing": "staging"}, "conflict_strategy": "overwrite"} + +CASES = { + # projects + "projects.list": Case( + call=call("projects.list"), + key=ENVIRONMENT_KEY, + method="GET", + path=PROJECTS, + query=DEFAULT_PAGE, + body=None, + response=[PROJECT_READ], + model=ProjectRead, + error=FORBIDDEN, + ), + "projects.list-page": Case( + call=call("projects.list", page=2, per_page=10), + key=ENVIRONMENT_KEY, + method="GET", + path=PROJECTS, + query=SECOND_PAGE, + body=None, + response=[], + model=ProjectRead, + error=FORBIDDEN, + ), + "projects.get": Case( + call=call("projects.get", "web"), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{PROJECTS}/web", + query=[], + body=None, + response=PROJECT_READ, + model=ProjectRead, + error=NOT_FOUND, + ), + "projects.get_by_key": Case( + call=call("projects.get_by_key", "web"), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{PROJECTS}/web", + query=[], + body=None, + response=PROJECT_READ, + model=ProjectRead, + error=NOT_FOUND, + ), + "projects.get_by_id": Case( + call=call("projects.get_by_id", PROJECT_ID), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{PROJECTS}/{PROJECT_ID}", + query=[], + body=None, + response=PROJECT_READ, + model=ProjectRead, + error=NOT_FOUND, + ), + # The default initial environments are left for the API to create, not sent. + "projects.create": Case( + call=call("projects.create", {"key": "web", "name": "Web"}), + key=ORGANIZATION_KEY, + method="POST", + path=PROJECTS, + query=[], + body={"key": "web", "name": "Web"}, + response=PROJECT_READ, + model=ProjectRead, + error=DUPLICATE, + ), + "projects.create-without-environments": Case( + call=call("projects.create", ProjectCreate(key="web", name="Web", initial_environments=[])), + key=ORGANIZATION_KEY, + method="POST", + path=PROJECTS, + query=[], + body={"key": "web", "name": "Web", "initial_environments": []}, + response=PROJECT_READ, + model=ProjectRead, + error=DUPLICATE, + ), + "projects.update": Case( + call=call("projects.update", "web", {"description": "The storefront", "settings": None}), + key=PROJECT_KEY, + method="PATCH", + path=f"{PROJECTS}/web", + query=[], + body={"description": "The storefront", "settings": None}, + response={**PROJECT_READ, "description": "The storefront"}, + model=ProjectRead, + error=NOT_FOUND, + ), + "projects.delete": Case( + call=call("projects.delete", "web"), + key=PROJECT_KEY, + method="DELETE", + path=f"{PROJECTS}/web", + query=[], + body=None, + response=None, + model=None, + error=NOT_FOUND, + ), + # environments + "environments.list": Case( + call=call("environments.list", "web"), + key=ENVIRONMENT_KEY, + method="GET", + path=ENVS, + query=DEFAULT_PAGE, + body=None, + response=[ENVIRONMENT_READ], + model=EnvironmentRead, + error=NOT_FOUND, + ), + "environments.list-page": Case( + call=call("environments.list", "web", page=2, per_page=10), + key=ENVIRONMENT_KEY, + method="GET", + path=ENVS, + query=SECOND_PAGE, + body=None, + response=[], + model=EnvironmentRead, + error=NOT_FOUND, + ), + "environments.get": Case( + call=call("environments.get", "web", "dev"), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{ENVS}/dev", + query=[], + body=None, + response=ENVIRONMENT_READ, + model=EnvironmentRead, + error=NOT_FOUND, + ), + "environments.get_by_key": Case( + call=call("environments.get_by_key", "web", "dev"), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{ENVS}/dev", + query=[], + body=None, + response=ENVIRONMENT_READ, + model=EnvironmentRead, + error=NOT_FOUND, + ), + "environments.get_by_id": Case( + call=call("environments.get_by_id", PROJECT_ID, ENVIRONMENT_ID), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{PROJECTS}/{PROJECT_ID}/envs/{ENVIRONMENT_ID}", + query=[], + body=None, + response=ENVIRONMENT_READ, + model=EnvironmentRead, + error=NOT_FOUND, + ), + "environments.get_stats": Case( + call=call("environments.get_stats", "web", "dev"), + key=ENVIRONMENT_KEY, + method="GET", + path=f"{ENVS}/dev/stats", + query=[], + body=None, + response=ENVIRONMENT_STATS, + model=EnvironmentStats, + error=NOT_FOUND, + ), + "environments.get_api_key": Case( + call=call("environments.get_api_key", "web", "dev"), + key=ENVIRONMENT_KEY, + method="GET", + path="/v2/api-key/web/dev", + query=[], + body=None, + response=API_KEY, + model=APIKeyRead, + error=NOT_FOUND, + ), + "environments.create": Case( + call=call("environments.create", "web", {"key": "staging", "name": "Staging"}), + key=PROJECT_KEY, + method="POST", + path=ENVS, + query=[], + body={"key": "staging", "name": "Staging"}, + response={**ENVIRONMENT_READ, "key": "staging", "name": "Staging"}, + model=EnvironmentRead, + error=DUPLICATE, + ), + "environments.update": Case( + call=call("environments.update", "web", "dev", {"name": "Dev", "description": None}), + key=ENVIRONMENT_KEY, + method="PATCH", + path=f"{ENVS}/dev", + query=[], + body={"name": "Dev", "description": None}, + response={**ENVIRONMENT_READ, "name": "Dev"}, + model=EnvironmentRead, + error=NOT_FOUND, + ), + "environments.copy": Case( + call=call("environments.copy", "web", "dev", COPY), + key=PROJECT_KEY, + method="POST", + path=f"{ENVS}/dev/copy", + query=[], + body=COPY, + response={**ENVIRONMENT_READ, "key": "staging", "name": "Staging"}, + model=EnvironmentRead, + error=DUPLICATE, + ), + "environments.delete": Case( + call=call("environments.delete", "web", "dev"), + key=ENVIRONMENT_KEY, + method="DELETE", + path=f"{ENVS}/dev", + query=[], + body=None, + response=None, + model=None, + error=NOT_FOUND, + ), +} + +# The cases of the methods an environment-level key cannot call. +WIDER_KEY_CASES = {name: case for name, case in CASES.items() if case.key != ENVIRONMENT_KEY} + + +def scoped_config(base_url: str, key: ApiKeyAccessLevel) -> PermitConfig: + """An offline config whose API key and SDK context are at ``key``'s level. + + ``offline_config()`` gives an environment-level key in an environment context. + """ + config = offline_config(base_url) + context = config.api_context + if key is ORGANIZATION_KEY: + context._save_api_key_accessible_scope(org=ORG) + context.set_organization_level_context(ORG) + elif key is PROJECT_KEY: + context._save_api_key_accessible_scope(org=ORG, project=PROJECT) + context.set_project_level_context(ORG, PROJECT) + return config + + +async def _invoke_async(config: PermitConfig, target: Call) -> object: + async with Permit(config) as permit: + return await attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + + +def invoke(config: PermitConfig, flavour: str, target: Call) -> object: + """Call ``permit.api.`` on a new async or blocking client, then close it.""" + if flavour == "async": + return asyncio.run(_invoke_async(config, target)) + with SyncPermit(config) as permit: + result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + 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} + + +def public_methods(prefix: str, api: type) -> set[str]: + return { + f"{prefix}.{name}" + for name, value in vars(api).items() + if not name.startswith("_") and callable(value) + } + + +def test_every_public_method_has_a_case() -> None: + public = public_methods("projects", ProjectsApi) | public_methods( + "environments", EnvironmentsApi + ) + + assert {case.call.path for case in CASES.values()} == public + assert len(public) == 17 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) +def test_request_and_response(httpserver: HTTPServer, case: Case, flavour: str) -> None: + config = scoped_config(httpserver.url_for("").rstrip("/"), case.key) + handler = httpserver.expect_request(case.path, method=case.method) + if case.response is None: + handler.respond_with_data("", status=204) + else: + handler.respond_with_json(case.response) + + result = invoke(config, flavour, case.call) + + assert [sent(request) for request, _ in httpserver.log] == [ + {"method": case.method, "path": case.path, "query": case.query, "body": case.body} + ] + assert [sent_headers(request) for request, _ in httpserver.log] == [JSON_HEADERS] + if case.model is None: + assert result is None + elif isinstance(case.response, list): + assert isinstance(result, list) + assert [type(item) for item in result] == [case.model] * len(case.response) + assert result == [case.model.parse_obj(item) for item in case.response] + else: + assert type(result) is case.model + assert result == case.model.parse_obj(case.response) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) +def test_an_api_error_raises_the_matching_permit_api_error( + httpserver: HTTPServer, case: Case, flavour: str +) -> None: + config = scoped_config(httpserver.url_for("").rstrip("/"), case.key) + httpserver.expect_request(case.path, method=case.method).respond_with_json( + case.error.body, status=case.error.status + ) + + with pytest.raises(PermitApiError) as raised: + invoke(config, flavour, case.call) + + assert type(raised.value) is case.error.raises + assert raised.value.status_code == case.error.status + assert raised.value.details == case.error.body + assert len(httpserver.log) == 1 + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize("case", WIDER_KEY_CASES.values(), ids=WIDER_KEY_CASES.keys()) +def test_a_narrower_api_key_is_refused_before_sending( + httpserver: HTTPServer, case: Case, flavour: str +) -> None: + narrower = API_ACCESS_LEVELS[API_ACCESS_LEVELS.index(case.key) + 1] + config = scoped_config(httpserver.url_for("").rstrip("/"), narrower) + + with pytest.raises(PermitContextError, match=re.escape(f"access level: {case.key}")): + invoke(config, flavour, case.call) + + assert httpserver.log == [] From c5558e5226726ec52c7d80e3c9953bb08e0781ef Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 06:53:01 +0300 Subject: [PATCH 04/13] Check every thread's result in the sync client e2e tests test_sync_client_multithreading submitted work to a thread pool and never read the futures, so an exception in a thread was lost and the test passed. Its users had random keys out of 1000 and were deleted only when every step before the delete passed. Each thread now creates a user with a unique key, registers its deletion before creating it, reads it back, deletes it and checks it is gone. The main thread reads every thread's result with a time limit, so a thread that raises fails the test. Many threads sharing one client, whose calls all run on its one background loop, are tested as well as a client per thread. The threads are daemons and each client is closed with the same time limit, so a call that never returns fails the test instead of hanging the session (PER-16177). Co-Authored-By: Claude Opus 5.5 --- tests/test_sync_client.py | 122 +++++++++++++++++++++++++++++++------- 1 file changed, 101 insertions(+), 21 deletions(-) diff --git a/tests/test_sync_client.py b/tests/test_sync_client.py index 67a0c8ed..0cc4b34a 100644 --- a/tests/test_sync_client.py +++ b/tests/test_sync_client.py @@ -1,35 +1,115 @@ -import random -from concurrent.futures.thread import ThreadPoolExecutor +"""End-to-end tests of the blocking client, called from one thread and from many. + +The blocking client runs every call on an event loop in a background thread of its own, so +the calls of many threads that share one client all go through that one loop. Each thread +creates a user of its own, reads it back and deletes it; the main thread reads every +thread's result, so a call that raises in a thread fails the test. +""" + +import functools +import threading +from collections.abc import Callable +from concurrent.futures import Future +from contextlib import ExitStack +from typing import TypeVar import pytest -from permit import PermitConfig, UserCreate +from permit import PermitConfig, UserCreate, UserRead +from permit.exceptions import PermitNotFoundError from permit.sync import Permit +from tests.utils import delete_quietly_blocking, unique_key pytestmark = pytest.mark.e2e +THREADS = 10 +# How long to wait for a thread's result, or for a client's close(), which waits for the +# calls in flight. Long enough for the rate-limit retries of conftest.py, which can hold one +# call for about two minutes; short enough that a call that never returns fails the test. +TIMEOUT_S = 300 -@pytest.fixture -def permit(permit_config: PermitConfig) -> Permit: - return Permit(permit_config) +T = TypeVar("T") -def test_sync_client(permit: Permit) -> None: - user_key = f"user-{random.randint(0, 1000)}" # noqa: S311 - a test key, not a secret - permit.api.users.create( - UserCreate( - key=user_key, - email="test@example.com", +def create_read_delete(permit: Permit, user_key: str) -> UserRead: + """Create a user, read it back and delete it. It is deleted at the end whatever happens.""" + with ExitStack() as teardown: + teardown.callback( + delete_quietly_blocking, + functools.partial(permit.api.users.delete, user_key), + f"user '{user_key}'", ) - ) - user = permit.api.users.get(user_key) - assert user.key == user_key - permit.api.users.delete(user_key) + permit.api.users.create(UserCreate(key=user_key, email=f"{user_key}@example.com")) + user = permit.api.users.get(user_key) + permit.api.users.delete(user_key) + with pytest.raises(PermitNotFoundError): + permit.api.users.get(user_key) + return user + + +def run_in_threads(calls: list[Callable[[], T]]) -> list[T]: + """Run each call on a thread of its own, all at once, and return what each returned. + + Every result is read, with a time limit, so a call that raises in its thread raises + here, and one that never returns fails the test. The threads are daemons, unlike a + ThreadPoolExecutor's, which the interpreter waits for at exit, so a stuck one cannot + hold up the end of the test session either. + """ + futures: list[Future[T]] = [Future() for _ in calls] + + def run(work: Callable[[], T], future: Future[T]) -> None: + try: + future.set_result(work()) + # BaseException: a pytest failure, such as pytest.raises() not raising, is not an Exception. + except BaseException as error: + future.set_exception(error) + + for work, future in zip(calls, futures, strict=True): + threading.Thread(target=run, args=(work, future), daemon=True).start() + return [future.result(timeout=TIMEOUT_S) for future in futures] + + +def close_within(client: Permit) -> None: + """Close ``client``, failing instead of waiting forever for a call that never returns.""" + closing = threading.Thread(target=client.close, daemon=True) + closing.start() + closing.join(TIMEOUT_S) + assert not closing.is_alive(), "close() did not return: a call on the client is stuck" + + +def test_sync_client(permit_config: PermitConfig) -> None: + user_key = unique_key("sync-client") + + with Permit(permit_config) as permit: + user = create_read_delete(permit, user_key) + + assert type(user) is UserRead + assert (user.key, user.email) == (user_key, f"{user_key}@example.com") + + +def test_threads_sharing_one_sync_client(permit_config: PermitConfig) -> None: + """Every thread's calls go through the one background loop of the client they share.""" + keys = [unique_key("sync-shared") for _ in range(THREADS)] + + with ExitStack() as clients: + permit = Permit(permit_config) + clients.callback(close_within, permit) + users = run_in_threads([functools.partial(create_read_delete, permit, key) for key in keys]) + + assert [type(user) for user in users] == [UserRead] * THREADS + assert [user.key for user in users] == keys + +def test_threads_each_with_a_sync_client_of_their_own(permit_config: PermitConfig) -> None: + keys = [unique_key("sync-own") for _ in range(THREADS)] -def test_sync_client_multithreading(permit_config: PermitConfig) -> None: - instances = [Permit(permit_config) for _ in range(10)] + with ExitStack() as clients: + calls: list[Callable[[], UserRead]] = [] + for key in keys: + permit = Permit(permit_config) + clients.callback(close_within, permit) + calls.append(functools.partial(create_read_delete, permit, key)) + users = run_in_threads(calls) - with ThreadPoolExecutor() as executor: - for instance in instances: - executor.submit(test_sync_client, instance) + assert [type(user) for user in users] == [UserRead] * THREADS + assert [user.key for user in users] == keys From 25b2bd61f140c90eb16ce1e299604029b591c1d2 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 06:53:01 +0300 Subject: [PATCH 05/13] Check headers and API errors of resource actions and action groups The offline tests of permit.api.resource_actions and action_groups checked each request's method, path, query and body and the parsed result. They now also check its Authorization and Content-Type headers, check that a 404 with the API's error body raises PermitNotFoundError for every method, and close each client they create (PER-16177). Co-Authored-By: Claude Opus 5.5 --- tests/test_fix_resource_actions.py | 73 +++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/tests/test_fix_resource_actions.py b/tests/test_fix_resource_actions.py index ccc933cf..8242f592 100644 --- a/tests/test_fix_resource_actions.py +++ b/tests/test_fix_resource_actions.py @@ -1,10 +1,11 @@ """Offline tests for permit.api.resource_actions and permit.api.action_groups (PER-16177). -Every public method is called through the async and the blocking client, and the -test checks the request it puts on the wire (method, path, query string and JSON -body) and the model 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. +Every public method is called through the async and the blocking client, each closed +once the call returns, and the test checks the request it puts on the wire (method, +path, query string, headers and JSON body), the model the response parses into, and +that the API's error response raises the matching ``PermitApiError``. 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 @@ -14,6 +15,7 @@ import pytest from pytest_httpserver import HTTPServer +from werkzeug import Request from permit import Permit from permit.api.models import ( @@ -27,6 +29,7 @@ from permit.api.resource_action_groups import ResourceActionGroupsApi from permit.api.resource_actions import ResourceActionsApi from permit.config import PermitConfig +from permit.exceptions import PermitApiError, PermitNotFoundError from permit.sync import Permit as SyncPermit from permit.utils.pydantic_version import PYDANTIC_VERSION @@ -302,6 +305,27 @@ def test_every_public_method_has_a_case() -> None: assert len(expected) == 14 +async def _invoke_async(config: PermitConfig, target: Call) -> object: + async with Permit(config) as permit: + method = attrgetter(target.path.removeprefix("permit."))(permit) + return await method(*target.args, **target.kwargs) + + +def invoke(config: PermitConfig, flavour: str, target: Call) -> object: + """Call ``target`` on a new async or blocking client, then close the client.""" + if flavour == "async": + return asyncio.run(_invoke_async(config, target)) + with SyncPermit(config) as permit: + method = attrgetter(target.path.removeprefix("permit."))(permit) + result = method(*target.args, **target.kwargs) + 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 ("Authorization", "Content-Type")} + + @pytest.mark.parametrize("flavour", ["async", "sync"]) @pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) def test_request_and_response( @@ -313,22 +337,47 @@ def test_request_and_response( else: handler.respond_with_json(case.response) - permit = Permit(config) if flavour == "async" else SyncPermit(config) - method = attrgetter(case.call.path.removeprefix("permit."))(permit) - result = method(*case.call.args, **case.call.kwargs) - if flavour == "async": - result = asyncio.run(result) - else: - assert not inspect.isawaitable(result) + result = invoke(config, flavour, case.call) assert [sent(request) for request, _ in httpserver.log] == [ {"method": case.method, "path": case.path, "query": case.query, "body": case.body} ] + assert [sent_headers(request) for request, _ in httpserver.log] == [ + {"Authorization": "Bearer test-token", "Content-Type": "application/json"} + ] if case.model is None: assert result is None elif isinstance(case.response, list): + assert isinstance(result, list) assert [type(item) for item in result] == [case.model] * len(case.response) assert result == [case.model.parse_obj(item) for item in case.response] else: assert type(result) is case.model assert result == case.model.parse_obj(case.response) + + +# The API's answer for a resource, action or action group that does not exist. +NOT_FOUND = { + "id": "6a1b2c3d0000400080000000000000ee", + "title": "Not found", + "error_code": "NOT_FOUND", + "message": "The resource document was not found.", +} + + +@pytest.mark.parametrize("flavour", ["async", "sync"]) +@pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) +def test_an_api_error_raises_the_matching_permit_api_error( + httpserver: HTTPServer, config: PermitConfig, case: Case, flavour: str +) -> None: + httpserver.expect_request(case.path, method=case.method).respond_with_json( + NOT_FOUND, status=404 + ) + + with pytest.raises(PermitApiError) as raised: + invoke(config, flavour, case.call) + + assert type(raised.value) is PermitNotFoundError + assert raised.value.status_code == 404 + assert raised.value.details == NOT_FOUND + assert len(httpserver.log) == 1 From 8ac1cc0229eb34889fbe24cfe92275451caacea0 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 06:53:01 +0300 Subject: [PATCH 06/13] Wire-test elements.login_as on both clients Pin the request permit.elements.login_as() sends, from the async and the blocking client, for keys and for UUIDs: its path, query, headers and JSON body. Check what it returns, the API's ticket with the redirect URL in content, and that a 404 with the API's error body raises PermitNotFoundError (PER-16177). Co-Authored-By: Claude Opus 5.5 --- tests/test_elements_offline.py | 102 +++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 tests/test_elements_offline.py diff --git a/tests/test_elements_offline.py b/tests/test_elements_offline.py new file mode 100644 index 00000000..67c2f1a2 --- /dev/null +++ b/tests/test_elements_offline.py @@ -0,0 +1,102 @@ +"""Offline tests for permit.elements.login_as() (PER-16177). + +It is called through the async and the blocking client, each closed once the call returns, +and the test checks the request it puts on the wire (method, path, query string, headers +and JSON body), what the response parses into, and that the API's error response raises +the matching ``PermitApiError``. 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 uuid import UUID + +import pytest +from pytest_httpserver import HTTPServer + +from permit import Permit +from permit.api.elements import UserLoginAsResponse +from permit.config import PermitConfig +from permit.exceptions import PermitApiError, PermitNotFoundError +from permit.sync import Permit as SyncPermit +from tests.utils import sent + +FLAVOURS = ["async", "sync"] +LOGIN_AS = "/v2/auth/elements_login_as" +USER_ID = "01234567-89ab-cdef-0123-456789abcdef" +TENANT_ID = "fedcba98-7654-3210-fedc-ba9876543210" +TICKET = {"redirect_url": "https://app.example.com/login?token=abc", "token": "abc"} + +# The ids login_as() is called with, and the ids it sends. +IDS: dict[str, tuple[str | UUID, str | UUID, dict[str, str]]] = { + "keys": ("alice", "acme", {"user_id": "alice", "tenant_id": "acme"}), + "uuids": (UUID(USER_ID), UUID(TENANT_ID), {"user_id": USER_ID, "tenant_id": TENANT_ID}), +} + + +async def _login_as_async( + config: PermitConfig, user: str | UUID, tenant: str | UUID +) -> UserLoginAsResponse: + async with Permit(config) as permit: + return await permit.elements.login_as(user, tenant) + + +def login_as( + config: PermitConfig, flavour: str, user: str | UUID, tenant: str | UUID +) -> UserLoginAsResponse: + """Call ``permit.elements.login_as()`` on a new async or blocking client, then close it.""" + if flavour == "async": + return asyncio.run(_login_as_async(config, user, tenant)) + with SyncPermit(config) as permit: + result = permit.elements.login_as(user, tenant) + assert not inspect.isawaitable(result) + return result + + +@pytest.mark.parametrize("flavour", FLAVOURS) +@pytest.mark.parametrize(("user", "tenant", "body"), IDS.values(), ids=IDS.keys()) +def test_login_as_request_and_response( + *, + httpserver: HTTPServer, + config: PermitConfig, + user: str | UUID, + tenant: str | UUID, + body: dict[str, str], + flavour: str, +) -> None: + """The response gets ``content``, which holds the redirect URL for a header login.""" + httpserver.expect_request(LOGIN_AS, method="POST").respond_with_json(TICKET) + + result = login_as(config, flavour, user, tenant) + + assert [sent(request) for request, _ in httpserver.log] == [ + {"method": "POST", "path": LOGIN_AS, "query": [], "body": body} + ] + assert [ + (request.headers.get("Authorization"), request.headers.get("Content-Type")) + for request, _ in httpserver.log + ] == [("Bearer test-token", "application/json")] + assert type(result) is UserLoginAsResponse + assert result == UserLoginAsResponse(**TICKET, content={"url": TICKET["redirect_url"]}) + + +@pytest.mark.parametrize("flavour", FLAVOURS) +def test_login_as_raises_the_api_error_for_an_unknown_user( + httpserver: HTTPServer, config: PermitConfig, flavour: str +) -> None: + detail = { + "id": "6a1b2c3d0000400080000000000000ee", + "title": "Not found", + "error_code": "NOT_FOUND", + "message": "The user alice was not found.", + } + httpserver.expect_request(LOGIN_AS, method="POST").respond_with_json(detail, status=404) + + with pytest.raises(PermitApiError) as raised: + login_as(config, flavour, "alice", "acme") + + assert type(raised.value) is PermitNotFoundError + assert raised.value.status_code == 404 + assert raised.value.details == detail + assert len(httpserver.log) == 1 From 076dd9484c4c5f9559ebf6e15a326f6229071949 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:09:38 +0300 Subject: [PATCH 07/13] Say where a method's wire test goes in CONTRIBUTING.md The API coverage section explains the untested status but not where the wire test that retires one lives. Name the offline modules that hold the wire tests, what they check, and that a new method gets its wire test in the change that adds it. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 90c77b58..2b24cdd1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -371,6 +371,17 @@ sends with the proxy on, the tests of the PDP's waits and of the cloud PDP's 404 and a new facts method fails `test_every_public_facts_method_has_a_case` until it has a case there. +A method's wire test is in the offline module of its API, for example +`tests/test_schema_offline.py` (resources, their attributes, relations and roles, roles, +condition sets and condition set rules), `tests/test_facts_operations_offline.py` (the bulk +and single-object facts methods with the proxy off and on, and user invites) or +`tests/test_projects_environments_offline.py`. Such a module calls the method on the async +and the blocking client and checks the request, its headers, what the response parses into +and the error an API error response raises, and most of them fail a +`test_every_public_method_has_a_case` test until every public method of their APIs has a +case. Add a new method's wire test there in the same change, so that its operation never +needs an `untested` entry. + CI runs it in two places: - The `API Coverage` job in `.github/workflows/test.yml`, on every pull request, against From 38fa681c7e25feb938f94b6aa8bbd24fc5afc7d0 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:38:55 +0300 Subject: [PATCH 08/13] Send one delete per user in the sync client e2e tests create_read_delete kept its teardown after deleting the user and checking it was gone, so every passing run sent a second DELETE per user (21 across the three tests), each answered 404 and logged as a tolerated cleanup error. Drop the teardown once the user is confirmed gone; until then it still deletes the user whatever happens. Co-Authored-By: Claude Opus 5.5 --- tests/test_sync_client.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_sync_client.py b/tests/test_sync_client.py index 0cc4b34a..30d3bd62 100644 --- a/tests/test_sync_client.py +++ b/tests/test_sync_client.py @@ -32,7 +32,11 @@ def create_read_delete(permit: Permit, user_key: str) -> UserRead: - """Create a user, read it back and delete it. It is deleted at the end whatever happens.""" + """Create a user, read it back, delete it and check it is gone. + + Until the user is gone, a teardown deletes it whatever happens. Once it is, the + teardown is dropped, so that a passing run sends one delete per user, not two. + """ with ExitStack() as teardown: teardown.callback( delete_quietly_blocking, @@ -44,6 +48,7 @@ def create_read_delete(permit: Permit, user_key: str) -> UserRead: permit.api.users.delete(user_key) with pytest.raises(PermitNotFoundError): permit.api.users.get(user_key) + teardown.pop_all() return user From 718cf64923c142aeca570ed5c9bdecf65903faee Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:40:13 +0300 Subject: [PATCH 09/13] Wait for every thread before reading the threaded test's results run_in_threads read the futures one at a time, each with its own time limit, so a thread that hung hid the error a later thread raised: the test failed with a bare TimeoutError. Wait for all the threads together under one limit, then raise the first error any call raised, or a TimeoutError that says how many threads did not return. Waiting for every thread also lets each one clean up before the clients close. Co-Authored-By: Claude Opus 5.5 --- tests/test_sync_client.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/test_sync_client.py b/tests/test_sync_client.py index 30d3bd62..0a673265 100644 --- a/tests/test_sync_client.py +++ b/tests/test_sync_client.py @@ -9,7 +9,7 @@ import functools import threading from collections.abc import Callable -from concurrent.futures import Future +from concurrent.futures import Future, wait from contextlib import ExitStack from typing import TypeVar @@ -23,9 +23,9 @@ pytestmark = pytest.mark.e2e THREADS = 10 -# How long to wait for a thread's result, or for a client's close(), which waits for the -# calls in flight. Long enough for the rate-limit retries of conftest.py, which can hold one -# call for about two minutes; short enough that a call that never returns fails the test. +# How long to wait for all the threads to finish, or for a client's close(), which waits for +# the calls in flight. Long enough for the rate-limit retries of conftest.py, which can hold +# one call for about two minutes; short enough that a call that never returns fails the test. TIMEOUT_S = 300 T = TypeVar("T") @@ -55,8 +55,10 @@ def create_read_delete(permit: Permit, user_key: str) -> UserRead: def run_in_threads(calls: list[Callable[[], T]]) -> list[T]: """Run each call on a thread of its own, all at once, and return what each returned. - Every result is read, with a time limit, so a call that raises in its thread raises - here, and one that never returns fails the test. The threads are daemons, unlike a + It waits for every thread, for TIMEOUT_S in all, so that each one has cleaned up before + the caller closes its clients. Then the error of the first call in ``calls`` that + raised is raised here, even when another call has not returned; when none raised but + one has not returned, a TimeoutError says how many. The threads are daemons, unlike a ThreadPoolExecutor's, which the interpreter waits for at exit, so a stuck one cannot hold up the end of the test session either. """ @@ -71,7 +73,14 @@ def run(work: Callable[[], T], future: Future[T]) -> None: for work, future in zip(calls, futures, strict=True): threading.Thread(target=run, args=(work, future), daemon=True).start() - return [future.result(timeout=TIMEOUT_S) for future in futures] + _, pending = wait(futures, timeout=TIMEOUT_S) + for future in futures: + if future.done() and (error := future.exception()) is not None: + raise error + if pending: + msg = f"{len(pending)} of {len(futures)} threads did not return within {TIMEOUT_S}s" + raise TimeoutError(msg) + return [future.result() for future in futures] def close_within(client: Permit) -> None: From f0d577bf71f4823350e0d7a1eafcce54f81f38d7 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:40:57 +0300 Subject: [PATCH 10/13] Answer environment reads with an email configuration, as the API does The API answers a get or a list of environments with EnvironmentReadWithEmailConfig, which requires email_configuration. The wire tests' mock answers for environments.list, get, get_by_key and get_by_id left it out. Add it to those four, and leave the create, update and copy answers, whose schema is EnvironmentRead, as they are. Co-Authored-By: Claude Opus 5.5 --- tests/test_projects_environments_offline.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_projects_environments_offline.py b/tests/test_projects_environments_offline.py index b7abe8d7..815eeff1 100644 --- a/tests/test_projects_environments_offline.py +++ b/tests/test_projects_environments_offline.py @@ -129,6 +129,12 @@ class Case(NamedTuple): "created_at": TIMESTAMP, "updated_at": TIMESTAMP, } +# What the API answers a get or a list of environments with: an environment and its email +# configuration. +ENVIRONMENT_WITH_EMAIL_CONFIG = { + **ENVIRONMENT_READ, + "email_configuration": "6a1b2c3d-0000-4000-8000-000000000080", +} ENVIRONMENT_STATS = { **ENVIRONMENT_READ, "pdp_configs": [ @@ -272,7 +278,7 @@ class Case(NamedTuple): path=ENVS, query=DEFAULT_PAGE, body=None, - response=[ENVIRONMENT_READ], + response=[ENVIRONMENT_WITH_EMAIL_CONFIG], model=EnvironmentRead, error=NOT_FOUND, ), @@ -294,7 +300,7 @@ class Case(NamedTuple): path=f"{ENVS}/dev", query=[], body=None, - response=ENVIRONMENT_READ, + response=ENVIRONMENT_WITH_EMAIL_CONFIG, model=EnvironmentRead, error=NOT_FOUND, ), @@ -305,7 +311,7 @@ class Case(NamedTuple): path=f"{ENVS}/dev", query=[], body=None, - response=ENVIRONMENT_READ, + response=ENVIRONMENT_WITH_EMAIL_CONFIG, model=EnvironmentRead, error=NOT_FOUND, ), @@ -316,7 +322,7 @@ class Case(NamedTuple): path=f"{PROJECTS}/{PROJECT_ID}/envs/{ENVIRONMENT_ID}", query=[], body=None, - response=ENVIRONMENT_READ, + response=ENVIRONMENT_WITH_EMAIL_CONFIG, model=EnvironmentRead, error=NOT_FOUND, ), From ee35daa0718801f8961bf763bc3f87db57784315 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:44:56 +0300 Subject: [PATCH 11/13] Share the wire tests' helpers through tests/utils.py The schema, facts operations, projects and environments, resource actions and elements wire tests each defined their own invoke, sent_headers, HEADERS, JSON_HEADERS, ApiError and error_details, and two error_details had already drifted to different arguments and bodies. Define them once in tests/utils.py, with the NOT_FOUND, DUPLICATE and FORBIDDEN errors, and import them in those five modules. error_details now returns the API's ErrorDetails shape everywhere. The resource action cases name their methods under permit.api, as the other modules do, so they can use the shared invoke. The resource action and login_as tests now also check that no wait-for-sync header is sent. The collected test ids are unchanged. Co-Authored-By: Claude Opus 5.5 --- tests/test_elements_offline.py | 25 +++---- tests/test_facts_operations_offline.py | 79 ++++---------------- tests/test_fix_resource_actions.py | 61 +++------------ tests/test_projects_environments_offline.py | 81 ++++---------------- tests/test_schema_offline.py | 82 +++++---------------- tests/utils.py | 78 +++++++++++++++++++- 6 files changed, 147 insertions(+), 259 deletions(-) diff --git a/tests/test_elements_offline.py b/tests/test_elements_offline.py index 67c2f1a2..aa82fb5c 100644 --- a/tests/test_elements_offline.py +++ b/tests/test_elements_offline.py @@ -18,9 +18,9 @@ from permit import Permit from permit.api.elements import UserLoginAsResponse from permit.config import PermitConfig -from permit.exceptions import PermitApiError, PermitNotFoundError +from permit.exceptions import PermitApiError from permit.sync import Permit as SyncPermit -from tests.utils import sent +from tests.utils import JSON_HEADERS, NOT_FOUND, sent, sent_headers FLAVOURS = ["async", "sync"] LOGIN_AS = "/v2/auth/elements_login_as" @@ -73,10 +73,7 @@ def test_login_as_request_and_response( assert [sent(request) for request, _ in httpserver.log] == [ {"method": "POST", "path": LOGIN_AS, "query": [], "body": body} ] - assert [ - (request.headers.get("Authorization"), request.headers.get("Content-Type")) - for request, _ in httpserver.log - ] == [("Bearer test-token", "application/json")] + assert [sent_headers(request) for request, _ in httpserver.log] == [JSON_HEADERS] assert type(result) is UserLoginAsResponse assert result == UserLoginAsResponse(**TICKET, content={"url": TICKET["redirect_url"]}) @@ -85,18 +82,14 @@ def test_login_as_request_and_response( def test_login_as_raises_the_api_error_for_an_unknown_user( httpserver: HTTPServer, config: PermitConfig, flavour: str ) -> None: - detail = { - "id": "6a1b2c3d0000400080000000000000ee", - "title": "Not found", - "error_code": "NOT_FOUND", - "message": "The user alice was not found.", - } - httpserver.expect_request(LOGIN_AS, method="POST").respond_with_json(detail, status=404) + httpserver.expect_request(LOGIN_AS, method="POST").respond_with_json( + NOT_FOUND.body, status=NOT_FOUND.status + ) with pytest.raises(PermitApiError) as raised: login_as(config, flavour, "alice", "acme") - assert type(raised.value) is PermitNotFoundError - assert raised.value.status_code == 404 - assert raised.value.details == detail + assert type(raised.value) is NOT_FOUND.raises + assert raised.value.status_code == NOT_FOUND.status + assert raised.value.details == NOT_FOUND.body assert len(httpserver.log) == 1 diff --git a/tests/test_facts_operations_offline.py b/tests/test_facts_operations_offline.py index edbb249b..236726f5 100644 --- a/tests/test_facts_operations_offline.py +++ b/tests/test_facts_operations_offline.py @@ -13,17 +13,12 @@ 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 BaseModel from pytest_httpserver import HTTPServer -from werkzeug import Request -from permit import Permit from permit.api.models import ( BulkRoleAssignmentReport, BulkRoleUnAssignmentReport, @@ -47,14 +42,7 @@ ) from permit.api.user_invites import UserInvitesApi from permit.config import PermitConfig -from permit.exceptions import ( - PermitAlreadyExistsError, - PermitApiDetailedError, - PermitApiError, - PermitNotFoundError, - PermitValidationError, -) -from permit.sync import Permit as SyncPermit +from permit.exceptions import PermitApiError, PermitValidationError from tests.facts_methods import ( ASSIGNMENT, INSTANCE, @@ -68,13 +56,23 @@ USER_ID, read, ) -from tests.utils import FACTS, Call, call, offline_config, sent +from tests.utils import ( + DUPLICATE, + FACTS, + FORBIDDEN, + JSON_HEADERS, + NOT_FOUND, + ApiError, + Call, + call, + invoke, + offline_config, + sent, + sent_headers, +) 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") - class Routing(NamedTuple): """The facts options of a client, and the wait-for-sync headers its requests carry.""" @@ -97,31 +95,6 @@ class Routing(NamedTuple): ), } - -class ApiError(NamedTuple): - """An error status, the API's JSON body with it, and the error the SDK raises for it.""" - - status: int - body: dict[str, Any] - raises: type[PermitApiError] - - -def error_details(error_code: str, title: str) -> dict[str, Any]: - """The body the API sends with an error status other than 422.""" - return { - "id": "6a1b2c3d0000400080000000000000ee", - "title": title, - "error_code": error_code, - "message": f"{title}.", - "support_link": "https://docs.permit.io/errors", - } - - -NOT_FOUND = ApiError(404, error_details("NOT_FOUND", "Not found"), PermitNotFoundError) -DUPLICATE = ApiError( - 409, error_details("DUPLICATE_ENTITY", "Already exists"), PermitAlreadyExistsError -) -FORBIDDEN = ApiError(403, error_details("FORBIDDEN_ACCESS", "Forbidden"), PermitApiDetailedError) INVALID = ApiError( 422, {"detail": [{"loc": ["body", "operations"], "msg": "field required", "type": "missing"}]}, @@ -511,21 +484,6 @@ def make_config(api: HTTPServer, pdp: HTTPServer, routing: Routing) -> PermitCon ) -async def _invoke_async(config: PermitConfig, target: Call) -> object: - async with Permit(config) as permit: - return await attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) - - -def invoke(config: PermitConfig, flavour: str, target: Call) -> object: - """Call ``permit.api.`` on a new async or blocking client, then close it.""" - if flavour == "async": - return asyncio.run(_invoke_async(config, target)) - with SyncPermit(config) as permit: - result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) - assert not inspect.isawaitable(result) - return result - - def destination( case: Case, routing: str, api: HTTPServer, pdp: HTTPServer ) -> tuple[HTTPServer, str, HTTPServer]: @@ -535,14 +493,9 @@ def destination( return api, case.api_path, pdp -def sent_headers(request: Request) -> dict[str, str | None]: - return {name: request.headers.get(name) for name in HEADERS} - - def expected_headers(routing: Routing) -> dict[str, str | None]: return { - "Authorization": "Bearer test-token", - "Content-Type": "application/json", + **JSON_HEADERS, "X-Wait-Timeout": routing.wait_timeout, "X-Timeout-Policy": routing.timeout_policy, } diff --git a/tests/test_fix_resource_actions.py b/tests/test_fix_resource_actions.py index 8242f592..5deecb5d 100644 --- a/tests/test_fix_resource_actions.py +++ b/tests/test_fix_resource_actions.py @@ -8,16 +8,11 @@ key and no ``/v2/api-key/scope`` lookup are needed. """ -import asyncio -import inspect -from operator import attrgetter from typing import TYPE_CHECKING, Any, NamedTuple import pytest from pytest_httpserver import HTTPServer -from werkzeug import Request -from permit import Permit from permit.api.models import ( ResourceActionCreate, ResourceActionGroupCreate, @@ -29,8 +24,7 @@ from permit.api.resource_action_groups import ResourceActionGroupsApi from permit.api.resource_actions import ResourceActionsApi from permit.config import PermitConfig -from permit.exceptions import PermitApiError, PermitNotFoundError -from permit.sync import Permit as SyncPermit +from permit.exceptions import PermitApiError from permit.utils.pydantic_version import PYDANTIC_VERSION if TYPE_CHECKING: @@ -40,7 +34,7 @@ from pydantic import BaseModel else: from pydantic.v1 import BaseModel -from tests.utils import SCHEMA, Call, call, sent +from tests.utils import JSON_HEADERS, NOT_FOUND, SCHEMA, Call, call, invoke, sent, sent_headers RESOURCES = f"{SCHEMA}/resources" TIMESTAMP = "2024-01-01T00:00:00+00:00" @@ -76,8 +70,9 @@ def group(key: str) -> dict[str, Any]: class Case(NamedTuple): """One SDK call and the request it must send. - ``response`` is the JSON the server answers with, or None for an empty 204; - ``model`` is what it parses into, or None when the method returns nothing. + ``call`` is the method's dotted path under ``permit.api``. ``response`` is the JSON + the server answers with, or None for an empty 204; ``model`` is what it parses into, + or None when the method returns nothing. """ call: Call @@ -89,8 +84,8 @@ class Case(NamedTuple): model: type[BaseModel] | None -ACTIONS = "permit.api.resource_actions" -GROUPS = "permit.api.action_groups" +ACTIONS = "resource_actions" +GROUPS = "action_groups" CASES = { "actions.list": Case( @@ -305,27 +300,6 @@ def test_every_public_method_has_a_case() -> None: assert len(expected) == 14 -async def _invoke_async(config: PermitConfig, target: Call) -> object: - async with Permit(config) as permit: - method = attrgetter(target.path.removeprefix("permit."))(permit) - return await method(*target.args, **target.kwargs) - - -def invoke(config: PermitConfig, flavour: str, target: Call) -> object: - """Call ``target`` on a new async or blocking client, then close the client.""" - if flavour == "async": - return asyncio.run(_invoke_async(config, target)) - with SyncPermit(config) as permit: - method = attrgetter(target.path.removeprefix("permit."))(permit) - result = method(*target.args, **target.kwargs) - 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 ("Authorization", "Content-Type")} - - @pytest.mark.parametrize("flavour", ["async", "sync"]) @pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) def test_request_and_response( @@ -342,9 +316,7 @@ def test_request_and_response( assert [sent(request) for request, _ in httpserver.log] == [ {"method": case.method, "path": case.path, "query": case.query, "body": case.body} ] - assert [sent_headers(request) for request, _ in httpserver.log] == [ - {"Authorization": "Bearer test-token", "Content-Type": "application/json"} - ] + assert [sent_headers(request) for request, _ in httpserver.log] == [JSON_HEADERS] if case.model is None: assert result is None elif isinstance(case.response, list): @@ -356,28 +328,19 @@ def test_request_and_response( assert result == case.model.parse_obj(case.response) -# The API's answer for a resource, action or action group that does not exist. -NOT_FOUND = { - "id": "6a1b2c3d0000400080000000000000ee", - "title": "Not found", - "error_code": "NOT_FOUND", - "message": "The resource document was not found.", -} - - @pytest.mark.parametrize("flavour", ["async", "sync"]) @pytest.mark.parametrize("case", CASES.values(), ids=CASES.keys()) def test_an_api_error_raises_the_matching_permit_api_error( httpserver: HTTPServer, config: PermitConfig, case: Case, flavour: str ) -> None: httpserver.expect_request(case.path, method=case.method).respond_with_json( - NOT_FOUND, status=404 + NOT_FOUND.body, status=NOT_FOUND.status ) with pytest.raises(PermitApiError) as raised: invoke(config, flavour, case.call) - assert type(raised.value) is PermitNotFoundError - assert raised.value.status_code == 404 - assert raised.value.details == NOT_FOUND + assert type(raised.value) is NOT_FOUND.raises + assert raised.value.status_code == NOT_FOUND.status + assert raised.value.details == NOT_FOUND.body assert len(httpserver.log) == 1 diff --git a/tests/test_projects_environments_offline.py b/tests/test_projects_environments_offline.py index 815eeff1..6f32d38d 100644 --- a/tests/test_projects_environments_offline.py +++ b/tests/test_projects_environments_offline.py @@ -9,18 +9,13 @@ pre-populated, so no API key and no ``/v2/api-key/scope`` lookup are needed. """ -import asyncio -import inspect import re -from operator import attrgetter from typing import Any, NamedTuple import pytest from pydantic.v1 import BaseModel from pytest_httpserver import HTTPServer -from werkzeug import Request -from permit import Permit from permit.api.context import API_ACCESS_LEVELS, ApiKeyAccessLevel from permit.api.environments import EnvironmentsApi from permit.api.models import ( @@ -32,55 +27,28 @@ ) from permit.api.projects import ProjectsApi from permit.config import PermitConfig -from permit.exceptions import ( - PermitAlreadyExistsError, - PermitApiDetailedError, - PermitApiError, - PermitContextError, - PermitNotFoundError, +from permit.exceptions import PermitApiError, PermitContextError +from tests.utils import ( + DUPLICATE, + FORBIDDEN, + JSON_HEADERS, + NOT_FOUND, + ORG, + PROJECT, + ApiError, + Call, + call, + invoke, + offline_config, + sent, + sent_headers, ) -from permit.sync import Permit as SyncPermit -from tests.utils import ORG, PROJECT, Call, call, offline_config, sent FLAVOURS = ["async", "sync"] ORGANIZATION_KEY = ApiKeyAccessLevel.ORGANIZATION_LEVEL_API_KEY PROJECT_KEY = ApiKeyAccessLevel.PROJECT_LEVEL_API_KEY ENVIRONMENT_KEY = ApiKeyAccessLevel.ENVIRONMENT_LEVEL_API_KEY -# 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 ApiError(NamedTuple): - """An error status, the API's JSON body with it, and the error the SDK raises for it.""" - - status: int - body: dict[str, Any] - raises: type[PermitApiError] - - -def error_details(error_code: str, title: str) -> dict[str, Any]: - return { - "id": "6a1b2c3d0000400080000000000000ee", - "title": title, - "error_code": error_code, - "message": f"{title}.", - "support_link": "https://docs.permit.io/errors", - } - - -NOT_FOUND = ApiError(404, error_details("NOT_FOUND", "Not found"), PermitNotFoundError) -DUPLICATE = ApiError( - 409, error_details("DUPLICATE_ENTITY", "Already exists"), PermitAlreadyExistsError -) -FORBIDDEN = ApiError(403, error_details("FORBIDDEN_ACCESS", "Forbidden"), PermitApiDetailedError) - class Case(NamedTuple): """One method call, the request it must send, and what it returns. @@ -414,25 +382,6 @@ def scoped_config(base_url: str, key: ApiKeyAccessLevel) -> PermitConfig: return config -async def _invoke_async(config: PermitConfig, target: Call) -> object: - async with Permit(config) as permit: - return await attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) - - -def invoke(config: PermitConfig, flavour: str, target: Call) -> object: - """Call ``permit.api.`` on a new async or blocking client, then close it.""" - if flavour == "async": - return asyncio.run(_invoke_async(config, target)) - with SyncPermit(config) as permit: - result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) - 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} - - def public_methods(prefix: str, api: type) -> set[str]: return { f"{prefix}.{name}" diff --git a/tests/test_schema_offline.py b/tests/test_schema_offline.py index 3aa90f34..a52098f1 100644 --- a/tests/test_schema_offline.py +++ b/tests/test_schema_offline.py @@ -13,17 +13,12 @@ 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 BaseModel from pytest_httpserver import HTTPServer -from werkzeug import Request -from permit import Permit from permit.api.condition_set_rules import ConditionSetRulesApi from permit.api.condition_sets import ConditionSetsApi from permit.api.models import ( @@ -62,15 +57,22 @@ from permit.api.resources import ResourcesApi from permit.api.roles import RolesApi from permit.config import PermitConfig -from permit.exceptions import ( - PermitAlreadyExistsError, - PermitApiDetailedError, - PermitApiError, - PermitNotFoundError, - PermitValidationError, +from permit.exceptions import PermitApiDetailedError, PermitApiError, PermitValidationError +from tests.utils import ( + DUPLICATE, + FACTS, + JSON_HEADERS, + NOT_FOUND, + SCHEMA, + ApiError, + Call, + call, + error_details, + invoke, + offline_config, + sent, + sent_headers, ) -from permit.sync import Permit as SyncPermit -from tests.utils import FACTS, SCHEMA, Call, call, offline_config, sent FLAVOURS = ["async", "sync"] @@ -104,15 +106,6 @@ DEFAULT_PAGE = [("page", "1"), ("per_page", "100")] SECOND_PAGE = [("page", "2"), ("per_page", "10")] -# 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, -} - def read(object_id: str, **fields: Any) -> dict[str, Any]: """A read model's JSON: its ``object_id``, ``fields``, and what every read model has.""" @@ -1021,22 +1014,6 @@ def request(self) -> dict[str, Any]: BASIC_CASES = {name: case for name, case in CASES.items() if name == case.call.path} -def invoke(config: PermitConfig, flavour: str, target: Call) -> object: - """Call ``permit.api.`` on a new async or blocking client, then close it.""" - if flavour == "async": - - async def call_awaiting() -> object: - async with Permit(config) as permit: - method = attrgetter(f"api.{target.path}")(permit) - return await method(*target.args, **target.kwargs) - - return asyncio.run(call_awaiting()) - with SyncPermit(config) as permit: - result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) - assert not inspect.isawaitable(result) - return result - - def respond(server: HTTPServer, case: Case) -> None: """Make ``server`` answer the request of ``case`` with the case's response.""" handler = server.expect_request(case.path, method=case.method) @@ -1046,10 +1023,6 @@ def respond(server: HTTPServer, case: Case) -> None: handler.respond_with_json(case.response) -def sent_headers(request: Request) -> dict[str, str | None]: - return {name: request.headers.get(name) for name in HEADERS} - - def test_every_public_method_has_a_case() -> None: public = { f"{api}.{name}" @@ -1087,29 +1060,10 @@ def test_request_and_response( # --- API errors ------------------------------------------------------------------------ - -class ApiError(NamedTuple): - """An error status, the JSON body the API sends with it, and what the SDK raises.""" - - status: int - body: dict[str, Any] - raises: type[PermitApiError] - - -def error_details(status: int, error_code: str) -> dict[str, Any]: - """The API's body for an error other than a validation error.""" - return { - "id": "request-1", - "title": f"status {status}", - "error_code": error_code, - "message": f"status {status}", - } - - -NOT_FOUND = ApiError(404, error_details(404, "NOT_FOUND"), PermitNotFoundError) -DUPLICATE = ApiError(409, error_details(409, "DUPLICATE_ENTITY"), PermitAlreadyExistsError) INVALID_PERMISSION = ApiError( - 400, error_details(400, "INVALID_PERMISSION_FORMAT"), PermitApiDetailedError + 400, + error_details("INVALID_PERMISSION_FORMAT", "Invalid permission format"), + PermitApiDetailedError, ) diff --git a/tests/utils.py b/tests/utils.py index 66229e69..84f76d4f 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,17 +1,26 @@ import asyncio +import inspect import json import time import uuid from collections.abc import Awaitable, Callable +from operator import attrgetter from typing import Any, NamedTuple, TypeVar import pytest from loguru import logger from werkzeug import Request +from permit import Permit from permit.api.context import ApiContext from permit.config import PermitConfig -from permit.exceptions import PermitApiError +from permit.exceptions import ( + PermitAlreadyExistsError, + PermitApiDetailedError, + PermitApiError, + PermitNotFoundError, +) +from permit.sync import Permit as SyncPermit # --- offline tests ------------------------------------------------------------ # @@ -61,6 +70,73 @@ def sent(request: Request) -> dict[str, Any]: } +# --- offline wire tests of a permit.api method -------------------------------- +# +# A wire test calls a method through the async and the blocking client, each closed once +# the call returns, and checks the request, its headers, what the response parses into +# and the error an API error response raises. + +# 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") +# HEADERS on a request with a JSON body from a client of offline_config() that has no +# facts sync timeout. +JSON_HEADERS: dict[str, str | None] = { + "Authorization": "Bearer test-token", + "Content-Type": "application/json", + "X-Wait-Timeout": None, + "X-Timeout-Policy": None, +} + + +def sent_headers(request: Request) -> dict[str, str | None]: + """The value of each of ``HEADERS`` on ``request``, None for one it does not carry.""" + return {name: request.headers.get(name) for name in HEADERS} + + +async def _invoke_async(config: PermitConfig, target: Call) -> object: + async with Permit(config) as permit: + return await attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + + +def invoke(config: PermitConfig, flavour: str, target: Call) -> object: + """Call ``permit.api.`` on a new async or blocking client, then close it. + + ``flavour`` is "async" for ``permit.Permit`` or "sync" for ``permit.sync.Permit``. + """ + if flavour == "async": + return asyncio.run(_invoke_async(config, target)) + with SyncPermit(config) as permit: + result = attrgetter(f"api.{target.path}")(permit)(*target.args, **target.kwargs) + assert not inspect.isawaitable(result) + return result + + +class ApiError(NamedTuple): + """An error status, the API's JSON body with it, and the error the SDK raises for it.""" + + status: int + body: dict[str, Any] + raises: type[PermitApiError] + + +def error_details(error_code: str, title: str) -> dict[str, Any]: + """The body the API sends with an error status other than 422: its ``ErrorDetails``.""" + return { + "id": "6a1b2c3d0000400080000000000000ee", + "title": title, + "error_code": error_code, + "message": f"{title}.", + "support_link": "https://docs.permit.io/errors", + } + + +NOT_FOUND = ApiError(404, error_details("NOT_FOUND", "Not found"), PermitNotFoundError) +DUPLICATE = ApiError( + 409, error_details("DUPLICATE_ENTITY", "Already exists"), PermitAlreadyExistsError +) +FORBIDDEN = ApiError(403, error_details("FORBIDDEN_ACCESS", "Forbidden"), PermitApiDetailedError) + + # --- end-to-end tests --------------------------------------------------------- # The hosted cloud PDP. conftest.py's fixtures can default to it, and the e2e tests that From b198434b6de6106b076dcba88d12cee01b43507b Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:45:46 +0300 Subject: [PATCH 12/13] Drop the async login_as tests that test_elements_offline.py covers test_elements_login_as_sends_canonical_uuid_strings and test_elements_login_as_passes_string_ids_through checked only the JSON body of an async login_as call with UUID ids and with string ids, on a client they never closed. test_login_as_request_and_response checks the same bodies on both clients, with the path, headers and return value. Remove the two, and say next to its ids that a UUID is sent in its canonical hyphenated form, not UUID.hex. Co-Authored-By: Claude Opus 5.5 --- tests/test_elements_offline.py | 3 ++- tests/test_offline_regressions.py | 34 +------------------------------ 2 files changed, 3 insertions(+), 34 deletions(-) diff --git a/tests/test_elements_offline.py b/tests/test_elements_offline.py index aa82fb5c..fb85901d 100644 --- a/tests/test_elements_offline.py +++ b/tests/test_elements_offline.py @@ -28,7 +28,8 @@ TENANT_ID = "fedcba98-7654-3210-fedc-ba9876543210" TICKET = {"redirect_url": "https://app.example.com/login?token=abc", "token": "abc"} -# The ids login_as() is called with, and the ids it sends. +# The ids login_as() is called with, and the ids it sends: a UUID in its canonical +# hyphenated form, not UUID.hex. IDS: dict[str, tuple[str | UUID, str | UUID, dict[str, str]]] = { "keys": ("alice", "acme", {"user_id": "alice", "tenant_id": "acme"}), "uuids": (UUID(USER_ID), UUID(TENANT_ID), {"user_id": USER_ID, "tenant_id": TENANT_ID}), diff --git a/tests/test_offline_regressions.py b/tests/test_offline_regressions.py index 09a503d4..ea7c57a1 100644 --- a/tests/test_offline_regressions.py +++ b/tests/test_offline_regressions.py @@ -17,7 +17,7 @@ from operator import attrgetter from pathlib import Path from typing import Any, get_type_hints -from uuid import UUID, uuid4 +from uuid import uuid4 import aiohttp import pydantic @@ -31,7 +31,6 @@ import permit from permit import Permit, Resource, User, exceptions from permit.api.context import ApiKeyAccessLevel -from permit.api.elements import ElementsApi from permit.api.encoders import jsonable_encoder from permit.api.environments import EnvironmentsApi from permit.api.models import ( @@ -401,37 +400,6 @@ async def test_every_sdk_client_sends_the_standard_bearer_scheme( } -async def test_elements_login_as_sends_canonical_uuid_strings( - httpserver: HTTPServer, config: PermitConfig -) -> None: - """UUID ids must be sent in canonical hyphenated form, not UUID.hex.""" - httpserver.expect_request("/v2/auth/elements_login_as", method="POST").respond_with_json( - {"redirect_url": "http://elements.permit.test/login"} - ) - - await ElementsApi(config).login_as( - UUID("01234567-89ab-cdef-0123-456789abcdef"), - UUID("fedcba98-7654-3210-fedc-ba9876543210"), - ) - - assert single_request(httpserver).get_json() == { - "user_id": "01234567-89ab-cdef-0123-456789abcdef", - "tenant_id": "fedcba98-7654-3210-fedc-ba9876543210", - } - - -async def test_elements_login_as_passes_string_ids_through( - httpserver: HTTPServer, config: PermitConfig -) -> None: - httpserver.expect_request("/v2/auth/elements_login_as", method="POST").respond_with_json( - {"redirect_url": "http://elements.permit.test/login"} - ) - - await ElementsApi(config).login_as("user-1", "tenant-1") - - assert single_request(httpserver).get_json() == {"user_id": "user-1", "tenant_id": "tenant-1"} - - async def test_tenants_delete_tenant_user_targets_the_tenant_membership( httpserver: HTTPServer, config: PermitConfig ) -> None: From 628309832966345636b72614832b5c2646006d36 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Fri, 2 Oct 2026 07:46:19 +0300 Subject: [PATCH 13/13] Name each wire-test module's guard in CONTRIBUTING.md The paragraph on where a method's wire test goes said most of the modules fail test_every_public_method_has_a_case, but the facts operations module holds only the user invite methods to its guard. Name the modules that have that test and what the facts operations module checks instead, mention the shared helpers in tests/utils.py, and say "CI runs the report" so the next paragraph no longer reads as if CI ran a wire test in two places. Co-Authored-By: Claude Opus 5.5 --- CONTRIBUTING.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2b24cdd1..5160d30c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -376,13 +376,16 @@ A method's wire test is in the offline module of its API, for example condition sets and condition set rules), `tests/test_facts_operations_offline.py` (the bulk and single-object facts methods with the proxy off and on, and user invites) or `tests/test_projects_environments_offline.py`. Such a module calls the method on the async -and the blocking client and checks the request, its headers, what the response parses into -and the error an API error response raises, and most of them fail a -`test_every_public_method_has_a_case` test until every public method of their APIs has a -case. Add a new method's wire test there in the same change, so that its operation never -needs an `untested` entry. - -CI runs it in two places: +and the blocking client with the helpers of `tests/utils.py` (`invoke`, `sent_headers`, +`ApiError`), and checks the request, its headers, what the response parses into and the +error an API error response raises. The schema and the projects and environments modules, +and `tests/test_fix_resource_actions.py`, fail `test_every_public_method_has_a_case` until +every public method of their APIs has a case; the facts operations module holds only the +user invite methods to that, with `test_every_public_user_invites_method_has_a_case`. Add a +new method's wire test there in the same change, so that its operation never needs an +`untested` entry. + +CI runs the report in two places: - The `API Coverage` job in `.github/workflows/test.yml`, on every pull request, against the committed snapshots. The `pytest` jobs record their requests too, and the report's