fix: align M2M tokens, API key validation, and JWKS with the spec - #29
Merged
Merged
Conversation
A real WorkOS M2M token carries its granted scopes as a space-delimited `scope` string and always carries a `jti`. The emulator emitted a `scp` array and no `jti` at all, so a service that authorized on scopes — or verified the token through a WorkOS SDK, which reads `payload.scope` and rejects an M2M token missing `jti` — passed locally and failed against the real API. That is the failure mode an emulator exists to prevent. Reported in #26.
Validation read `key` from the request body and answered
`{ valid: boolean }`. The spec sends the secret as `value` and
returns `{ api_key: ApiKey | null }`, which is what every SDK reads
— so the request field never matched, the emulator saw no key at
all, and callers got a shape they do not parse. The net effect was
that no API key could be validated locally, and a key's permissions
were unreachable even when it was.
Returning the resource rather than a boolean also gives callers the
`permissions` array, so permission-based authorization can be
exercised against the emulator.
A value registered only through the legacy allow-list map has no
resource behind it and now validates as `api_key: null` rather than
`true`: it can authenticate requests, but the emulator does not know
its owner or permissions, and inventing them would report
privileges that do not exist.
Reported in #26.
Only a bare `/sso/jwks` was registered, and Hono does not match that
against `/sso/jwks/{clientId}` — the path the spec documents and the
one the SDKs fetch when verifying a session or M2M access token. So
every SDK-based verification against the emulator got a 404 while
the key it needed was sitting one path segment away. The README has
documented the per-client path all along; the route simply never
existed.
The existing conformance loop keys on a resource's `object`
discriminator, which means the envelope a route wraps a resource in
was never checked — envelopes have no discriminator, and they are
assembled inline in the handler rather than by a format* helper.
That blind spot is exactly how `/api_keys/validations` shipped
`{ valid }` for several releases against a spec saying
`{ api_key }`: the resource catalog had nothing to say about it.
Envelope requirements are extracted per operation from the spec's
own `paths` entry, and extraction fails loudly when an operation's
declared `$ref` stops matching the curated schema name — the
counterpart to the discriminator guard, so a spec rename cannot
leave the test asserting a contract that no longer exists.
Coverage is curated because each case has to issue a real request,
so the catalog asserts it is exercised exactly, and divergences live
in ledgers as exact field sets rather than being tolerated
silently. Adding `api_key` to the resource catalog closes a second
gap of the same kind: a credential-bearing resource that had no
shape or secret-leak coverage.
The spec-derived catalogs under src/workos/generated are committed so consumers of the package never need the spec — which also means a @workos/openapi-spec bump does not regenerate them, and the conformance tests go on asserting the previous spec's contract. A dependency bump could turn green while the emulator had silently drifted, which is the whole class of problem those tests exist to catch. Added as a step in the existing `test` job rather than its own job: the repo ruleset pins required check contexts by name, so a new job would not be enforced until someone updated the ruleset, while a step is enforced immediately.
Seeding a catch-all webhook endpoint made the suite POST every resource it then created to a port nothing is listening on. Those fetches are async and outlive the file, so they land in whichever suite runs next — and one of those, event-bus.spec.ts, counts calls on a global fetch spy. Subscribing to an event the suite never triggers keeps the endpoint listable without generating traffic.
The assertion counted all calls on a spy installed over the global `fetch`, so a webhook delivery still in flight from another suite was counted as if this test had made it — reliably enough to fail roughly one full-suite run in ten, with concurrent-webhooks.spec.ts's ten concurrent user.created deliveries as the usual source. Indexing calls[0] had the same flaw more quietly: a foreign call arriving first would have sent the HMAC and signature-format assertions off to verify someone else's request body.
Greptile SummaryThis PR aligns authentication-related emulator behavior with the WorkOS specification.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+27
to
+28
| const record = authorized && value ? ws.apiKeyRecords.findOneBy('key', value) : undefined; | ||
| return c.json({ api_key: record ? formatApiKeyRecord(record) : null }); |
There was a problem hiding this comment.
When an accepted array-form seed contains duplicate API key values, the authentication map retains the last entry while findOneBy returns the first record, causing validation to return the wrong owner, permissions, and expiration metadata.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/workos/routes/api-keys.ts
Line: 27-28
Comment:
**Duplicate keys return wrong metadata**
When an accepted array-form seed contains duplicate API key values, the authentication map retains the last entry while `findOneBy` returns the first record, causing validation to return the wrong owner, permissions, and expiration metadata.
How can I resolve this? If you propose a fix, please make it concise.A key value is the identity of the secret: it is the auth allow-list's map key and the lookup key for the resource behind it. Two seed entries sharing one split that identity — the allow-list keeps the last entry's environment and expiry while the record lookup resolves the first, so validation gated on one entry and reported another's owner and permissions. Deleting either would also stop the other authenticating. Returning the resource from validations is what made the split observable, but the seed accepted the ambiguity to begin with, and production cannot issue one secret twice. Rejecting it alongside the existing duplicate organization name and id checks fixes every reader at once, rather than teaching each one which duplicate to prefer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A service could get an M2M access token out of the emulator but could not
read its scopes, and could not validate an API key at all. Both turned out to be the
emulator inventing a shape the real API never emits.
Summary
scparray with nojti. Production sends aspace-delimited
scopestring and always sendsjti; the SDKs readpayload.scopeandreject an M2M token missing
jti, so a well-signed emulator token verified as invalid.POST /api_keys/validations: readkeyfrom the request and answered{ valid: boolean }. The spec sends the secret asvalueand returns{ api_key: ApiKey | null }. The request field never matched, so the emulator never sawthe key at all; returning the resource also makes a key's
permissionsreachable.GET /sso/jwks/{clientId}: never registered. Only a bare/sso/jwksexisted, whichthe emulation server does not match against the sub-path the spec documents and the SDKs fetch, so every
SDK-based token verification 404'd.
objectdiscriminator, so response envelopes, assembled inline in route handlers, had no
coverage at all. Now extracted per
operation from the spec's own
paths, with divergences held inledgers as exact field sets rather than tolerated silently.
@workos/openapi-specbumpdid not regenerate them and the conformance tests would keep asserting the previous
spec's contract. Added as a step in the existing
testjob, because theruleset pins required check contexts by name.
Wire-format changes
Callers written against the old shapes need updating:
scp: ["a", "b"]scope: "a b"{ "key": "sk_..." }{ "value": "sk_..." }{ "valid": true }{ "api_key": { ... } }A value registered only through the legacy allow-list map form now validates as
api_key: nullrather thantrue. It still authenticates requests, but it has noapi_keyresource behind it, and synthesizing an owner and permission set would reportprivileges the emulator does not hold. Use the array form for keys your code validates.
Also fixed
A pre-existing flake, found while verifying the above and confirmed present on
mainatthe same rate:
event-bus.spec.tsasserted on every call to a spy installed over theglobal
fetch, so another suite's in-flight webhook delivery counted as its own.Closes #26