feat(cloudflare-access): Add audience (aud) validation and harden JWT verification#1805
Open
gradyat wants to merge 44 commits intohonojs:mainfrom
Open
feat(cloudflare-access): Add audience (aud) validation and harden JWT verification#1805gradyat wants to merge 44 commits intohonojs:mainfrom
gradyat wants to merge 44 commits intohonojs:mainfrom
Conversation
…event cross-application token reuse Without validating the aud claim, a valid JWT issued for any application within the same Cloudflare Access team could be used to access any other application. The aud parameter is optional for backwards compatibility but strongly recommended in production. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The condition used `<` instead of `>=`, causing keys to be re-fetched on every request while the cache was valid, and never refreshed after expiration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reject team names with unexpected characters to prevent URL injection in the JWKS endpoint URL. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Log a warning at middleware initialization when no audience tag is provided, since omitting it allows cross-application token reuse. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace `header: object` with a typed interface containing alg, typ, and kid fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ments The header and payload were decoded with plain atob() which fails on base64url characters. Extract a base64urlDecode helper that handles -/_ substitution and use it for all three JWT segments. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reject tokens that specify an unexpected algorithm to prevent algorithm confusion attacks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify that exp is a number and iss is a string before proceeding with further validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reject tokens whose nbf timestamp is in the future. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the verbose error that exposed expected and received issuer URLs with a generic "Invalid team name" message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use consistent lowercase for "bearer token" and "Invalid token" across all error messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix 'Should be throw' -> 'Should throw' in test descriptions and add missing beforeEach to the vitest import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add tests for nbf rejection, alg validation, missing exp, cache refresh behavior, team name validation, aud warning, generic issuer error, and base64url decoding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add rows for alg, nbf, payload validation, and team name validation. Update existing rows to reflect normalized casing and generic issuer error message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…type The nbf (not-before) claim is optional per JWT spec (RFC 7519) and Cloudflare Access tokens may omit it. The runtime code already treats it as optional, so the type should match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rmance Move issuer and audience checks before signature verification (cheap string comparisons gate expensive crypto). Move exp and nbf checks after signature verification (don't trust time-based claims from unverified tokens). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion - Deduplicate generateJWT by delegating to generateJWTWithHeader - Use vi.restoreAllMocks() instead of vi.clearAllMocks() to properly restore original implementations (e.g. console.warn spies) - Fix test description casing to match actual error message Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…imers The existing cache test only verified that a second immediate request used cached keys. Split it into two tests: one for cache hits within TTL, and a new test that uses vi.useFakeTimers() to advance past the 1-hour TTL and verify keys are actually re-fetched. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add strict token structure validation (exactly 3 parts), reject critical extensions per RFC 7515, filter JWKS keys by kty/use, use kid for key selection, and fix base64url padding. Includes comprehensive tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…er RFC 7517 Filter out JWKs where key_ops is present but doesn't include "verify" (§4.3) and skip keys missing a kid field to prevent undefined map entries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y optional Fields like email, type, identity_nonce, sub, and country are not guaranteed to be present in all Cloudflare Access JWT tokens (e.g., service tokens may omit email/sub). The aud field is typed as string | string[] per RFC 7519 §4.1.3 which allows either form. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RFC 7519 §4.1.3 allows the aud claim to be either a string or an array of strings. When aud is a plain string, Array.prototype.includes is not called — instead String.prototype.includes performs substring matching, which could allow a token with aud "expected-aud" to pass validation when the configured audience is "expected-aud-tag". Normalize to array before checking to guarantee exact element matching in all cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RFC 7515 §5.2 requires rejecting base64url-encoded representations that contain line breaks, whitespace, or other extraneous characters. The previous implementation passed input directly to atob() which is lenient about some malformed input. Add a regex guard to reject any characters outside the valid base64url alphabet before decoding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d nbf RFC 7519 §4.1.4-4.1.5 recommends that implementations MAY provide a small leeway (usually no more than a few minutes) to account for clock skew. In distributed systems like Cloudflare Workers running across global edge PoPs, minor clock differences are common. A 30-second tolerance prevents spurious rejections at token boundaries without meaningfully weakening security. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When Cloudflare rotates signing keys mid-cache-period, tokens signed with the new key would fail verification for up to 1 hour until the cache expired. Now, if a token's kid header references a key not in the cache, the middleware re-fetches JWKS from Cloudflare. Re-fetches are rate-limited to at most once per 60 seconds to prevent attackers from busting the cache with forged kid values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The aud parameter now accepts string | string[], allowing a single worker to serve multiple Cloudflare Access applications or to accept tokens during application migrations where both old and new AUD tags are temporarily valid. Validation checks for intersection between the token's aud claim and the configured allowed audiences. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… error to README The error table was missing the 401 response for tokens containing a crit header parameter (RFC 7515 §4.1.11). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cloudflare handles DoS protection at the infrastructure level, making the 60-second rate limiter unnecessary complexity. Re-fetch on kid cache miss is now unconditional, which also eliminates the edge case where legit users could receive 401s during key rotation if the rate limiter had recently fired. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use TextDecoder to properly handle multi-byte UTF-8 sequences in base64url-decoded JWT segments instead of relying on atob() alone. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 5ba880c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The UTF-8 TextDecoder added in 14b0841 was also applied to the signature segment, corrupting raw binary data and causing crypto.subtle.verify() to fail for all valid tokens. Split base64urlDecode into text (header/payload) and binary (signature) variants so signature bytes are preserved. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update the payload example to gracefully handle cases where email is not present (e.g. service tokens), since the field is now optional in CloudflareAccessPayload. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the kid-based JWKS re-fetch to after an initial signature verification failure. Previously, an unverified kid in the JWT header could trigger outbound fetches to Cloudflare before the token was checked. Now the flow is: try verification with cached keys, and only if it fails and the token has an unknown kid, re-fetch and retry once. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Line 172: Remove false "RFC 7517 §4.5: kid MUST be present" claim (§4.5 is about key_ops, and kid is OPTIONAL per §4.4). Replace with accurate rationale for skipping keyless entries. - Line 255: Cite RFC 7515 §5.2 (verification algorithm) instead of §4.1.4 (parameter definition) for kid-based key selection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add tests verifying that tokens just outside the 30s clock skew leeway are correctly rejected: - exp: token expired 31s ago is rejected - nbf: token with nbf 31s in the future is rejected Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… and RFC cite Lowercase accessTeamName to prevent case-sensitive issuer mismatch, use exclusive exp boundary (<) consistent with nbf for clock skew, restrict key fallback to only when kid is absent per RFC 7515 §4.1.4, and fix typo in comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cast cf fetch option as RequestInit, use standard type assertion for result.json(), and pass Uint8Array .buffer as ArrayBuffer to crypto.subtle.verify to fix TS5 ArrayBufferLike incompatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1805 +/- ##
===========================================
+ Coverage 91.73% 100.00% +8.26%
===========================================
Files 113 1 -112
Lines 3785 113 -3672
Branches 957 33 -924
===========================================
- Hits 3472 113 -3359
+ Misses 281 0 -281
+ Partials 32 0 -32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
…lid input Cover the throw branch in base64urlDecodeToBytes when the signature segment contains characters outside the base64url alphabet. Co-Authored-By: Claude Opus 4.6 <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
2 participants
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.
I was looking at using the
@hono/cloudflare-accesspackage, but I noticed that it did not accept theaudparameter to validate the Application Audience Tag (AUD) as described in the Cloudflare Access documentation and code examples. This is a security vulnerability (the same as GHSA-m732-5p4w-x69g forhono) as it allows reusing a token assigned to a different Cloudflare Access application. This PR adds optionalaudvalidation; a warning is provided when this is omitted. In addition, several changes were made for JWT validation security hardening to conform to relevant RFCs (RFC 7515, 7517, 7519), including:nbfclaim (token is not before time), allow 30-second clock skewkidsupportFor transparency, this PR was developed and reviewed with AI agent assistance, and I am not a security engineer. However, I have personally reviewed all changes, and I have tested it with my worker locally with yalc. Please let me know if any concerns.
The author should do the following, if applicable
yarn changesetat the top of this repo and push the changeset