Repository navigation
feat(auth0-api-js): Principal + buildPrincipal - #284
Closed
tusharpandey13 wants to merge 5 commits into
Closed
tusharpandey13 wants to merge 5 commits into
tusharpandey13 wants to merge 5 commits into
Conversation
Introduce a typed, verified caller identity for downstream tool code instead of a raw claims bag. `buildPrincipal(claims)` is a pure transform over already-verified access-token claims (no re-verification). - `Principal`: sub, clientId, orgId, expiresAt, scopes, permissions - clientId = client_id ?? azp ?? null (no rejection) - permissions tri-state: null when absent, [] when present-empty, list otherwise - scopes = space-split of the scope claim ([] when absent) - missing/blank sub is the only rejection -> VerifyAccessTokenError Mirrors the Python `build_principal` field-for-field. The token fingerprint is intentionally omitted (contested design, pending resolution); adding it later is non-breaking. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reachedThis review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Next included review available in 50 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled. Title check ✅ Passed The title clearly identifies the addition of the Principal type and buildPrincipal function to auth0-api-js. Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
Comment @coderabbitai help to get the list of available commands.
Non-blocking review findings from forge code-review gate: - scope split now uses /\s+/ (any whitespace) to match Python's str.split() parity claim; previously split on single space only. - document Principal.expiresAt as epoch SECONDS with the exp-present guarantee. - harden sub-missing test with an error-message assertion, not just instanceof. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…SDK-11359) Forge test-review MINOR: blank-sub and empty-sub throw tests asserted instanceof only. Add the message regex to both so each rejection path independently guards the error identity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tusharpandey13
changed the title
nandan-bhat
reviewed
Oct 8, 2026
Guard non-string sub/scope and malformed permissions instead of throwing TypeError, copy permissions, expand Principal and buildPrincipal JSDoc, document buildPrincipal in README and EXAMPLES, and pin scope/clientId semantics with extra tests. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
Closing this PR. The fixes from the review carried over to the wrapper implementation: |
tusharpandey13
added a commit
that referenced
this pull request
Oct 21, 2026
* feat(auth0-api-js): add OBO TokenStore contracts and cache-aside to getTokenOnBehalfOf
Introduces optional token caching for the on-behalf-of flow in the API client.
When a caller passes a TokenStore, getTokenOnBehalfOf performs cache-aside:
verify the subject token, derive a namespaced cache key from the internally
verified claims, look up the cache, and on a miss exchange then store. When no
store is passed, behavior is byte-identical to before.
Core contracts only (the MCP layer owns the running store instance, the
covering-lookup index, and eviction):
- TokenStore interface (get/set/delete by opaque key)
- CachedToken record (extends TokenSet; records grantedScopes)
- DownscopedTokenError (thrown when the exchange grants fewer scopes than
requested; the downscoped token is never cached)
expiresAt values are epoch seconds. Cache key is
{iss}|{clientId}|{sub}|{orgId}|{audience}|{normalizedRequestedScopes}, derived
solely from internally verified claims. Adds 27 tests (302 total passing).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(auth0-api-js): document OBO TokenStore caching and DownscopedTokenError
Adds an EXAMPLES.md section showing an InMemoryTokenStore implementation passed
to getTokenOnBehalfOf, the CachedToken shape (expiresAt in epoch seconds,
grantedScopes), DownscopedTokenError semantics, and a responsibility table for
the core vs. higher-layer (store instance, covering-lookup, index, eviction).
Adds a short OBO caching note to README.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(auth0-api-js): injective OBO cache key + preserve token type on cache hit
S1: buildOboCacheKey joined components with a raw `|`, but Auth0 `sub`
values always contain `|` and audience/scope are caller-controlled, so
distinct requests could collapse to one key and a hit could return a
token minted for the wrong audience. Encode the key with JSON.stringify
over the fixed-length component array so each segment is unambiguously
quoted/escaped. Key composition (iss|clientId|sub|orgId|audience|scopes)
is unchanged — only the encoding is now injective.
S2: CachedToken dropped tokenType/issuedTokenType, so a cache hit
returned a Bearer-looking shape even for DPoP-bound tokens. Persist both
fields on set and return them on hit, matching the miss/no-store shape.
Adds tests: raw-delimiter collision inputs now map to distinct keys (and
a `|`-bearing sub stays intact), and a cache hit returns tokenType/
issuedTokenType identical to the miss path. No-store path untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* security(obo-cache): fix SR-2 key isolation and SR-3 unconditional verification
SR-2: buildOboCacheKey now includes exchangingClientId and exchangingAudience
as the first two slots, preventing cache collisions when two ApiClient
instances with different clientId or configured audience share a TokenStore.
SR-3: verifyAccessToken, #enforceOrganizationPolicy, and the downscope guard
are now unconditional — they run on both the store path and the no-store path.
Added OrganizationPolicy type, MissingOrganizationError, and
OrganizationNotAllowedError to enforce org_id claim policy before exchange.
* fix(auth0-api-js): add clock-skew leeway to OBO cache-hit; harden isolation/scope-key tests
Back-port of the verified hardening from the integration branch.
- SR-8: apply a 5-second clock-skew leeway to the OBO cache-hit expiry
check, so a near-expiry cached token is treated as a miss and re-exchanged
rather than served and then rejected downstream.
- SR-9: tests pinning scope-key shape parity (scope order and duplicates
normalize to the same key; genuinely distinct scope sets get distinct
keys).
- SR-7: tests proving no cross-entry bleed on a shared store (distinct scope
keys and distinct exchangingClientId prefixes are isolated), satisfied by
the 8-slot key binding.
- Subject-isolation e2e: a cached OBO token is keyed injectively across all
four identity axes (sub, org_id, iss, subjectClientId); varying any one
produces a distinct cache entry.
Also removes a pre-existing unused OrganizationPolicy import (eslint).
buildPrincipal wiring is deferred until PR #284 (Principal) lands on main;
the cache key derives identity from verified claims in the meantime.
vitest 319, tsup, eslint, tsc all green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(auth0-api-js): add MSW end-to-end tests for OBO cache-aside
Add api-client.obo-cache-e2e.spec.ts covering the complete getTokenOnBehalfOf
pipeline without internal spies: JWKS fetched from an MSW-mocked discovery
endpoint, token exchange executed against an MSW-mocked token endpoint, and
a real functional Map-based TokenStore. Tests cover cache miss (exchange runs
and result is stored), cache hit (second call with identical params skips the
endpoint), near-expiry re-exchange (cached token within the 5 s clock-skew
leeway is treated as a miss), DownscopedTokenError (partial grant is rejected
and nothing is stored), and cache-key injectivity across all eight key
dimensions (exchangingClientId, exchangingAudience, iss, subjectClientId, sub,
orgId, downstreamAudience, normalizedScopes).
Test count: 319 before, 325 after (+6).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(auth0-api-js): cache on-behalf-of tokens behind a pluggable token store
Pass a token store to `getTokenOnBehalfOf` and an exchanged token is reused
instead of being requested again. The token is cached under the scopes the
exchange actually granted, and those scopes are recorded alongside it, so a
later call asking for a narrower set is served from cache. A new
`scopeMatching` option selects exact matching, the default, or covering
matching, which reuses a token whose granted scopes cover the request and
prefers the smallest one. Covering matching searches several entries per
subject, so it requires an `IndexedTokenStore`.
Verification is conditional on the store. The claims are needed to derive a
cache key, so the subject token is verified when a store is supplied and
forwarded untouched otherwise, which is how the released package behaves. That
internal verification runs as a bearer token with no request context, so it
cannot satisfy a required DPoP mode, accept a DPoP-bound token, or give a
domains resolver a URL or headers. For those cases, pass the result of an
earlier `verifyAccessToken` call as `context.verified`: the claims object is
checked by identity against the ones this client verified, the token strings
must match, and expired claims are rejected. The key is derived from a copy
taken at verification time, so editing a claims object afterwards cannot move
the cache key.
Cache keys are versioned, namespaced by matching mode, and fixed length. They
hash the exchanging tenant, client and audience, the subject token's issuer,
client, subject, organization and session, the downstream audience, and the
scope set. Each field is length prefixed and an absent field encodes
distinctly from an empty one, so no two identities share a key, and no claim
value is held in the clear. A token whose key claims are not strings is not
cached at all, rather than keyed on a value the encoding cannot distinguish.
The cache never becomes an availability dependency. A read that throws, a
malformed entry, an entry close to expiry and an unreadable index are all
treated as a miss, and a write that throws still returns the token that was
just issued. Entry fields are read once into a fresh object, so a value cannot
change between validation and use. An index member is a hint: a member whose
key does not match the scopes it claims is skipped, and a candidate's coverage
is re-checked against the stored entry.
A result granted fewer scopes than requested is no longer rejected here. It is
cached under what was granted and returned, with `scope` exactly as the
authorization server sent it and `grantedScopes` carrying the normalized set.
The caller decides whether a narrowed grant is acceptable, because it has the
request context to turn that into the right response. `DownscopedTokenError` is
removed.
Organization policy also moves out of this method. The option and its two error
classes are removed here and the organization-policy change owns them, which
makes it a prerequisite. One guard remains at the pre-verified path as a
marked integration point: it refuses the combination rather than silently
skipping a policy it cannot yet enforce.
Also in this change: an empty granted scope is no longer dropped as falsy, so
it stays distinct from an absent one instead of falling back to the requested
scopes; missing client authentication is reported before any verification or
cache access; and `verifyAccessToken` uses the audience captured at
construction.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(auth0-api-js): document on-behalf-of token caching
Cover the token store, the two scope-matching modes, the indexed store the
covering mode needs, and the cache-entry shape.
Correct three claims that no longer hold. The subject token is forwarded
untouched only when no store is supplied; with a store it is verified, and that
verification runs as a bearer token, so a required DPoP mode, a DPoP-bound
token or a custom domains resolver needs the token verified at ingress and
handed in. A narrowed grant is cached and returned rather than rejected, and
the caller checks `grantedScopes` to decide what to do about it. An empty
`scope` means nothing was granted and is distinct from the field being absent,
so a truthiness test on it reads an empty grant as a full grant.
State the operational caveats plainly: this package applies no encryption, so a
store on shared or persistent storage is responsible for protecting the tokens
it holds; a store failure costs extra exchanges and never fails a call; entries
are validated for shape but not integrity, and must expose their fields as own
properties; nothing is ever pruned, so eviction belongs to the implementation;
and because the cache key's session component outlives any single access token,
a revoked session does not invalidate an already cached delegated token before
that token expires.
Co-Authored-By: Claude <noreply@anthropic.com>
* docs(auth0-api-js): state the absent and empty scope rule on OBO results
An absent scope in the token response means the scopes were granted as requested (RFC 6749 section 5.1), and an empty scope means nothing was granted. Say so on OnBehalfOfTokenResult.scope and grantedScopes, TokenSet.scope, the getTokenOnBehalfOf description and the README.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(auth0-api-js): enforce organization policy on pre-verified claims in the OBO path
getTokenOnBehalfOf skips access token verification when the caller supplies
context.verified, which also skipped the organization policy that
verifyAccessToken enforces. The policy is now applied to the trusted copy of
the claims before the cache key is derived, with the same checks, errors and
challenges as verifyAccessToken, so a rejected token never reaches the store.
This replaces the earlier refusal to combine the two options.
Drops the interim notes that referred to the organization policy being
carried by a separate change, and the removed-exports expectation for the
organization errors, which this package now exports.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(auth0-api-js): hash OBO cache keys without node crypto types
obo-cache-key.ts imported createHash from 'crypto', but the package does
not declare @types/node, so the tsup DTS build failed wherever those types
were not hoisted into the workspace. The key builder now hashes with
WebCrypto (crypto.subtle.digest), which needs no Node typings and runs on
every runtime the package targets.
buildOboTokenKey, buildOboIndexKey and deriveSessionKey now return
promises. They are new on this branch and unreleased. The produced keys
are byte-identical to the previous output.
Co-Authored-By: Claude <noreply@anthropic.com>
* test(auth0-api-js): pin OBO cache key digests and guard missing Web Crypto
Adds a spec asserting exact key strings (strict token key with a sid, non-strict
index key, access token session key fallback). The expected values were captured
by running the previous createHash implementation, so a change to the field
separator, ordering or digest encoding now fails the suite instead of silently
orphaning cached entries.
sha256Base64Url now throws an InvalidConfigurationError naming Web Crypto when
crypto.subtle is unavailable, instead of a raw TypeError. The cache write path
computes its key inside the existing fail-open block, so a key failure after a
successful exchange skips the cache write and still returns the token.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(auth0-api-js): report stale pre-verified claims as invalid tokens and tighten OBO cache edges
Pre-verified claims that expired, or that belong to a different token than the one
being exchanged, now throw VerifyAccessTokenError with the same challenge headers as
verifyAccessToken, so adapters answer 401 instead of 500. Claims this instance never
produced stay an InvalidConfigurationError because that is an integration bug.
A failure to derive a cache key on the read path is now a cache miss, matching the
write path. getTokenByExchangeProfile again drops falsy idToken, refreshToken,
tokenType and issuedTokenType. The challenge scheme inference is shared between the
verify and pre-verified paths, the expiry leeway constant has a neutral name, and the
key helper exports carry a beta note.
Docs now state how strict matching keys entries by granted scopes: a response without
scope hits on repeat, a narrowed or widened grant re-exchanges each time.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(auth0-api-js): align pre-verified token challenges and exchange profile scope with main
getTokenByExchangeProfile goes back to main's form for scope (an empty scope is left out); the on-behalf-of path uses a private helper that still tells empty from absent.
The early verified/token mismatch check now derives the challenge scheme from the recorded claims, so a DPoP-bound token gets a DPoP challenge. Pre-verified rejections use a generic "Invalid token" description so internal wording does not reach WWW-Authenticate.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
What
UNIT-1 of MCP JS SDK Phase 1 (
@auth0/auth0-api-js): a typed, verified caller identity so downstream tool code stops passing around a raw claims bag.buildPrincipal(claims: VerifiedAccessTokenClaims): Principal— pure transform over already-verified access-token claims. No re-verification.Contract
submissing/blank → throwVerifyAccessTokenError. Only rejection.clientId = client_id ?? azp ?? null(no reject when both absent).orgId = org_id ?? null.scopes= space-split ofscopeclaim;[]when absent.permissionstri-state:nullwhen absent,[]when present-empty, list otherwise.expiresAtfromexp(epoch seconds).Acceptance
Principal+buildPrincipalKnown invariant (reviewer note)
expiresAtreadsexpdirectly (no guard) — mirrors Python's direct access and relies onverifyAccessTokenhaving already validatedexpupstream. Adding an exp-guard would be a second throw case, diverging from the frozen "sub is the only rejection" contract. Flagged for sign-off if desired.Delivery
Stacked onto
feat/mcp-phase1integration branch. Do not merge — pending Snehil (parity) + Frederik (arch) sign-off on the DESIGN.🤖 Generated with Claude Code
Summary by CodeRabbit