Skip to content

Add the Groups API to the async and sync clients - #138

Draft
zeevmoney wants to merge 19 commits into
per-16680/stop-logging-api-keyfrom
per-16677/groups-api
Draft

Add the Groups API to the async and sync clients#138
zeevmoney wants to merge 19 commits into
per-16680/stop-logging-api-keyfrom
per-16677/groups-api

Conversation

@zeevmoney

@zeevmoney zeevmoney commented Oct 1, 2026

Copy link
Copy Markdown
Member

Linear issues

  • PER-16677: Add permit.api.groups with the 10 GA group operations to both clients.
  • PER-16337: Covers P1 item 1 and the P2 group-to-group routes of the SDK gap list.
  • PER-15117: The docstrings state which group identifier forms each method accepts.
  • PER-16235: The docstrings state that a group's roles reach its members through ReBAC.

Why

The SDK had no way to manage groups. Callers had to send raw requests to the /groups routes, and it was unclear which group identifier each route takes.

What changed

  • permit/api/groups.py: new GroupsApi, under /v2/schema/{proj}/{env}:

    Method Route
    list(page, per_page) GET /groups/direct
    get(group_instance_key) GET /groups/direct/{g}
    create(group_data) POST /groups
    delete(group_instance_key) DELETE /groups/{g}
    assign_user(g, user_key, tenant) / remove_user(...) PUT / DELETE /groups/{g}/users/{user}
    assign_role(g, role_data) / remove_role(...) POST / DELETE /groups/{g}/roles
    assign_group(g, assignment) / remove_group(...) PUT / DELETE /groups/{g}/assign_group

    The reads use the /groups/direct routes, not the deprecated GET /groups and GET /groups/{key}. Model arguments take the model or an equivalent dict, like the other modules.

  • The docstrings say which API key each method needs (environment-level, or a broader key with the API context set to the environment). They also say that a role granted to a group is a resource role on one instance, which reaches members through ReBAC role derivation and is not a tenant-wide role. For identifiers:

    Argument Accepts
    group_instance_key (first argument) instance id; "<type>:<key>"; a bare key, read as "group:<key>", so only for groups of the group type
    GroupAssignment.group_instance_key instance id or bare key, of a group of the same resource type; "<type>:<key>" answers 404
    GroupCreate.group_instance_key bare key
    GroupAddRole.resource_instance instance key in the group's tenant; an instance in another tenant answers 409

    assign_group(P, {"group_instance_key": C}) makes P's members members of C, so they get C's roles; C's members get nothing from P.

  • permit.api.groups is wired into PermitApiClient and SyncPermitApiClient (SyncGroupsApi), and permit/_sync_types.pyi was regenerated. API_SUB_API_COUNT goes from 17 to 18, and tests/type_check/consumer.py calls the groups API on both clients.

  • README: a short Groups section.

  • Tests: tests/test_groups_offline.py (offline wire tests) and tests/test_groups_e2e.py (e2e). tests/utils.py now holds delete_quietly and poll_for, which tests/test_cloud_pdp_e2e.py and tests/test_groups_e2e.py share. Each module keeps its own polling bounds.

Behaviour changes

  • New public API, additive only: permit.api.groups on permit.Permit and permit.sync.Permit, plus the importable names permit.api.groups.GroupsApi and permit.api.sync_api_client.SyncGroupsApi. No existing behaviour changes.

How it was tested

  • Offline suite: 472 passed, 3 skipped, 0 warnings on both pydantic v2 and v1 (361 before, plus 111 groups tests). The groups tests cover all 10 methods on both clients:
    • exact method, path, query, Authorization and Content-Type headers, JSON body, and the parsed return type;
    • 404 and 409 with the API's error body, raising PermitNotFoundError / PermitAlreadyExistsError with the status;
    • a project-level API context refused with PermitContextError before anything is sent;
    • an invalid dict rejected before sending.
  • Mutation check on the offline tests: 33 of 33 non-equivalent mutants of groups.py, the client wiring and the 404/409 mapping fail the tests. The one survivor, dropping the access-level check, is equivalent: every key level meets an environment-level requirement.
  • strict mypy: clean on 94 files on both pydantic lanes. The typing-surface and stub-drift tests pass on both lanes, and so do the sync/async parity tests.
  • e2e: tests/test_groups_e2e.py collects 7 tests, which CI runs. They cover create/get/list (and a duplicate create answering 409), both identifier forms, the bare key of the default type, a group role reaching a member through permit.check() and being revoked, group-to-group direction, and the blocking client. Cleanup is registered at creation, and a 404 counts as success. They have not run against the API yet. Against a local fake of the API and PDP, all 7 pass on both lanes, and each of 14 deliberate fake behaviour changes makes the suite fail.
  • pre-commit (all hooks), uv lock --check, actionlint and zizmor are clean.

Owner actions before merge

  • Check the e2e jobs, which run tests/test_groups_e2e.py against the API for the first time. In particular, test_assign_group_makes_the_group_a_member_of_the_other confirms the assign_group direction the docstrings and README state. The e2e (cloud PDP) job also runs tests/test_cloud_pdp_e2e.py with the shared helpers.

🤖 Generated with Claude Code

zeevmoney and others added 14 commits October 1, 2026 14:42
@zeevmoney @claude
Add the Groups API to the async and sync clients
f52d913 
permit.api.groups covers the ten GA group operations (PER-16677): list
and get through the /groups/direct reads, create, delete, adding and
removing users, granting and revoking roles, and making one group's
members members of another. Model arguments take the model or a dict.

The docstrings state which identifier forms each call accepts, the API
key it needs, the direction of assign_group, and that a group's roles
reach its members through ReBAC role derivation.

The sync stub is regenerated, the sub-API count sentinel goes to 18,
and the type-check consumer calls the new API on both clients.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Test the Groups API requests offline on both clients
90341bb 
Each of the ten methods is called through the async and the blocking
client against pytest-httpserver. The tests check the method, path,
query, headers and JSON body sent, the model the response parses into,
the identifier forms passed through the path, model and dict arguments
sending the same body, 404 and 409 raising PermitApiError with that
status, and an invalid dict being rejected before anything is sent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Document the Groups API in the README
b7bf938 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Add end-to-end tests for the Groups API
d7a9860 
Cover the ten GA group operations against the Permit API and a PDP
(PER-16677): create, get and list; assign and remove a user; grant and
revoke a role, checking through permit.check() that a member gets the
group's role on that resource instance only; nest one group in another,
checking which group's members gain the other's roles; and the 404 and
409 errors. One test drives a group through the blocking client.

The tests pin which identifier forms each call takes: the resource
instance id or <resource_key>:<instance_key> in the path, a bare key
only for a group of the default "group" type, and the instance key or
id (not the qualified form) for the group in the assign_group body.

Each test creates its own tenant, resource types, users and groups
with unique keys and registers every delete before the create it
undoes; a 404 at teardown counts as success.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Merge the Groups API SDK branch into the Groups API PR
ba8fc14 
* per-16677/impl-sdk:
  Document the Groups API in the README
  Test the Groups API requests offline on both clients
  Add the Groups API to the async and sync clients

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Merge the Groups API e2e test branch into the Groups API PR
8b87997 
* per-16677/impl-e2e:
  Add end-to-end tests for the Groups API

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Check in e2e that creating an existing group answers 409
3469348 
GroupsApi.create documents a 409 for a group that already exists. The
e2e test now creates the same group a second time and expects that
status, so CI confirms the documented error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Name the group identifier forms in e2e as the docstrings do
14036da 
The GroupsApi docstrings and the README call the qualified identifier
"<type>:<key>". The e2e module now uses the same name in its parameter
ids, helper docstring and comments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Say in the README which group forms assign_group's body takes
fd17a99 
The README's identifier bullet read as if every group_instance_key in
the Groups API took "<type>:<key>". It applies to the first argument
only. The group in the assign_group()/remove_group() body is named by
its instance id or its key alone, and both groups must be of the same
resource type; "<type>:<key>" there answers 404. The docstrings and the
e2e tests already say this.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Document that assign_role answers 409 for another tenant's instance
96ee014 
assign_role() looks up the resource instance in the group's tenant and
creates it there when the lookup misses. An instance key is unique
within its resource type regardless of tenant, so an instance with that
key in another tenant makes the create fail with 409. The docstring now
says the instance must be in the group's tenant and lists the 409.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Say what makes a resource type a group type in GroupsApi docs
e4dc8ed 
The API treats any resource type with a "member" role as a group
resource type, so groups.list() also returns the instances of types
that were never meant as groups. The class docstring now defines a
group resource type that way, and list() says what it returns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Test groups API errors with the error body the API sends
6aeefba 
The 404/409 test answered with a body that is not an ErrorDetails, so
the SDK raised its plain PermitApiError fallback, which a real API
error never reaches. The test now answers with an API-shaped body
(id, title, NOT_FOUND or DUPLICATE_ENTITY) and checks that every
groups method raises PermitNotFoundError or PermitAlreadyExistsError,
with the status and the body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Test that groups calls need an environment API context
685603e 
Every GroupsApi docstring says it needs an environment-level key or a
broader key with the API context set to an environment, but no test
pinned it. A project-level key whose context is the project now gets
PermitContextError from every groups method, on both clients, and
nothing is sent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Share the e2e polling and quiet-delete helpers in tests/utils
7f2c8ff 
test_cloud_pdp_e2e.py and test_groups_e2e.py each had the same
delete_quietly and the same bounded poll loop, differing only in their
timeout and interval. Both now live in tests/utils.py: delete_quietly
as it was, and poll_for, which takes the timeout and interval. Each
module binds its own values as settled, so the call sites and the
polling bounds are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 1, 2026

Copy link
Copy Markdown

PER-16677

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Dependency Security Audit

Scanned: pyproject.toml dependencies + dev group, resolved at Python 3.10 (the current resolution, and the lowest versions the published specs permit under each pydantic major)

✅ No known vulnerabilities found.

Both the resolved dependency set and the lowest versions the published specs permit are clean at HIGH and CRITICAL.

zeevmoney and others added 5 commits October 1, 2026 20:20
@zeevmoney
Merge the invites e2e fix from the base branch
94f2b2b 
* origin/per-16680/stop-logging-api-key:
  Invite with a role of the invited resource in the invites e2e test
@zeevmoney
Merge the RBAC e2e polling fix from the base branch
5de466c 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* origin/per-16680/stop-logging-api-key:
  Poll for the PDP's role assignment list in the RBAC e2e tests
@zeevmoney @claude
Merge the base branch's RBAC e2e polling fix
ff76d5b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Merge the review fixes from the base branch
9636773 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney @claude
Merge the base branch's backported e2e and schema fixes
fcb932e 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zeevmoney
zeevmoney added this pull request to stack #145 October 2, 2026 18:24

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zeevmoney