Skip to content

feat(authorization): align roles and permissions with the spec (PATCH, 409 conflicts, resource_type_slug) - #97

Merged
gjtorikian merged 21 commits into
mainfrom
fix/permission-response-shape
Sep 3, 2026
Merged

feat(authorization): align roles and permissions with the spec (PATCH, 409 conflicts, resource_type_slug)#97
gjtorikian merged 21 commits into
mainfrom
fix/permission-response-shape

Conversation

@workos-tars

@workos-tars workos-tars Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add system and resource_type_slug to every formatted permission response and event
  • preserve resource types supplied through both the API and seed config, defaulting omitted scopes to organization
  • validate custom resource types and remove the permission response-shape exception

Test plan

  • bun test src/workos/response-shapes.spec.ts src/workos/routes/authorization-permissions.spec.ts (42 passed)
  • bun run typecheck
  • bun run lint
  • bun run fmt:check

Requested by @alidacodes

@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR aligns authorization role and permission behavior with the updated WorkOS specification.

  • Adds resource_type_slug, permission arrays, and system fields to formatted authorization objects and lifecycle events.
  • Changes update routes to PATCH, introduces distinct permission replacement and attachment operations, and returns specification-compatible conflict responses.
  • Preserves authorization resource types through API and seed paths with validation and organization defaults.
  • Updates generated contracts, endpoint coverage metadata, and route-discovery tooling for the newer OpenAPI specification.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
src/workos/role-helpers.ts Aligns role mutation verbs and permission-management responses with the specification while emitting updated role payloads after join changes.
src/workos/routes/authorization-permissions.ts Adds permission resource-type persistence and validation, PATCH updates, and specification-compatible slug-conflict responses.
src/workos/helpers.ts Formats roles with current permission slugs and formats permissions with required scope and system fields.
src/workos/index.ts Preserves resource types during seeding and enriches emitted role lifecycle payloads.
scripts/gen-supported-lib.ts Derives shared role routes from their actual helper source to keep endpoint coverage metadata aligned with runtime registration.
src/workos/generated/events.ts Regenerates the event catalog and payload requirements for the upgraded OpenAPI specification.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Input[API request or seed config] --> Validate[Validate resource_type_slug]
  Validate --> Store[Store role or permission]
  Store --> Join[Role-permission joins]
  Join --> Format[Format role and permission responses]
  Format --> API[Authorization API response]
  Join --> Events[Role lifecycle events]
  Store --> Checks[Authorization checks and token permissions]
Loading

Reviews (13): Last reviewed commit: "fix(roles): emit permission changes as p..." | Re-trigger Greptile

Comment thread src/workos/helpers.ts Outdated
return {
...formatEntity(p),
system: false,
resource_type_slug: p.resource_type_slug ?? 'organization',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the first commit, this was explicitly passing 'organization'; now allows supporting other resource type slugs

@alidacodes alidacodes self-assigned this Sep 2, 2026
@workos workos deleted a comment from workos-tars Bot Sep 2, 2026
The `organization` fallback was spelled out independently at both
persistence sites and again in the formatter, so changing it (or
making it configurable) would have to land in three places. Review
of #97 also found the new seed key undocumented and the empty-string
rejection path untested; this closes those gaps in the same pass.
@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

The reported failure belongs to superseded head d3b13b3. Current PR #97 is green, including x64 Docker, ARM64 Docker, tests, lint, Node 22/24, and security checks.

The image behavior was unchanged; CI now allows enough time for pre-bind RSA key generation under variable runner load.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Folded this check result into the active TARS task.

@gjtorikian gjtorikian changed the title fix(permissions): emit SDK-required response fields feat(permissions): emit spec-required fields and honor resource_type_slug Sep 3, 2026
The pin had fallen 21 minor versions behind the published spec, so
the committed event catalog knew nothing of the agent and
resource-export events and SUPPORTED.md measured coverage against a
stale endpoint list. 0.80 also retags the agents endpoints and adds
IT contacts, waitlists, and platform teams; the support-matrix
generator refuses unassigned tags, so those are mapped to features
here.
The spec and every SDK update a permission with PATCH, but the
emulator only registered PUT, so the Node SDK's updatePermission
never reached the handler. Nothing outside the emulator ever sent
PUT, so it is dropped rather than kept as an alias. A taken slug
now answers 409 permission_slug_conflict as production does; the
former 422 field error never carried the code clients key on.
A loop inside one `it` reports a bare mismatch without saying which
input tripped; `it.each` names the failing value in the test title.
Same divergence as permissions: the spec and the Node SDK update
environment and organization roles with PATCH, so updateRole and
updateOrganizationRole never reached the emulator's PUT handler. A
taken slug now answers 409 with the spec's role_slug_conflict or
organization_role_slug_conflict code instead of a 422 field error,
which never carried the code clients key on.
@workos-tars

workos-tars Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Flagged: ARM64 failed only during the 10-second container health probe with repeated curl: (56) Connection reset by peer. The equivalent x64 Docker job passed.

This matches the previously observed ARM runner/startup flake; there is no evidence here of a permission or image-build regression. Per the agreed scope, TARS is reporting it rather than modifying PR #97.

The spec and the Node SDK replace a role's permissions with PUT and
attach a single one with POST, both answering with the role, which
production returns with its permission slugs inlined. The emulator
gave POST replace-all semantics, had no PUT, and answered DELETE with
an empty 204, so setEnvironmentRolePermissions had no handler and
addEnvironmentRolePermission was rejected as malformed. Roles now
carry `permissions` wherever they are formatted, which also closes
that tracked gap in the response-shape ledger.
The matrix expanded registerRoleRoutes from a hand-kept mirror of the
helper's routes, which had already drifted: it still listed PUT for
role updates after the switch to PATCH, so SUPPORTED.md kept scoring
FGA at 14/26 writes. Reading the registrations out of role-helpers.ts
removes the second copy, and the generator now fails if it finds none
rather than silently dropping every role endpoint.
The spec requires `resource_type_slug` on every role and on the role
lifecycle events, and both create DTOs accept it, but the emulator had
no notion of a role's scope and carried it as a tracked gap in the
response-shape ledger. Roles now take the scope on create and in the
seed file, default to `organization` as production does, and keep it
through updates, which the update DTO cannot change. Resource types
are still not modeled, so a scope is not checked against a registry
and a role's permissions are not checked against its scope; production
enforces both.
@gjtorikian gjtorikian changed the title feat(permissions): emit spec-required fields and honor resource_type_slug feat(authorization): align roles and permissions with the spec (PATCH, 409 conflicts, resource_type_slug) Sep 3, 2026
Production rejects a `resource_type_slug` that names no resource
type and refuses permissions whose scope does not match the role's;
the emulator accepts any non-empty slug and never cross-checks. That
was stated only in code comments, so a reader of SUPPORTED.md had no
way to know a scope typo passes here and fails against WorkOS.
The same optional-but-non-empty rule was spelled out at both create
routes and both seed validators, so tightening it later (a registry,
once resource types are modeled) would take four coordinated edits.
`PUT …/permissions` deleted the role's join rows and then inserted
slug by slug, throwing 404 at the first unknown one, so a single typo
left the role with a partial set. Production validates the whole list
before writing, and this is the path the SDKs'
setEnvironmentRolePermissions and setOrganizationRolePermissions
call.
Production emits role.updated or organization_role.updated when a
permission set actually changes through the permissions endpoints,
without touching the role row; the emulator emitted nothing there,
because only ws.roles.update fires the collection hook. Its
role.deleted also never carries permissions while
organization_role.deleted does, but the delete route cascaded the
joins before the row, so the hook always saw an empty list. Both
behaviors were read from the production roles service and the
role-permissions controllers.
@gjtorikian
gjtorikian merged commit 1d1c84c into main Sep 3, 2026
9 checks passed
@gjtorikian
gjtorikian deleted the fix/permission-response-shape branch September 3, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants