feat: add validateAccessJwt to cloudflare:workers#5837
feat: add validateAccessJwt to cloudflare:workers#5837
Conversation
|
The generated output of Full Type Diffdiff -r types/generated-snapshot/latest/index.d.ts bazel-bin/types/definitions/latest/index.d.ts
11660a11661,11701
> // Access JWT validation
> export type AccessJwtErrorCode =
> | "ERR_JWT_MISSING"
> | "ERR_JWT_MALFORMED"
> | "ERR_JWT_INVALID_SIGNATURE"
> | "ERR_JWT_EXPIRED"
> | "ERR_JWT_NOT_YET_VALID"
> | "ERR_JWT_AUDIENCE_MISMATCH"
> | "ERR_JWT_ISSUER_MISMATCH"
> | "ERR_JWKS_FETCH_FAILED"
> | "ERR_JWKS_NO_MATCHING_KEY"
> | "ERR_INVALID_TEAM_DOMAIN"
> | "ERR_INVALID_AUDIENCE";
> export class AccessJwtError extends Error {
> readonly code: AccessJwtErrorCode;
> constructor(code: AccessJwtErrorCode, message: string);
> }
> export interface AccessJwtPayload {
> aud: string | string[];
> email?: string;
> exp: number;
> iat: number;
> nbf?: number;
> iss: string;
> sub: string;
> [key: string]: unknown;
> }
> /**
> * Validates a Cloudflare Access JWT from an incoming request.
> *
> * @param req - The incoming Request containing the cf-access-jwt-assertion header
> * @param teamDomain - The Cloudflare One team domain (e.g., "mycompany" or "mycompany.cloudflareaccess.com")
> * @param audience - The Application Audience (AUD) tag
> * @throws {AccessJwtError} If validation fails for any reason
> * @returns The decoded JWT payload on success
> */
> export function validateAccessJwt(
> req: Request,
> teamDomain: string,
> audience: string,
> ): Promise<AccessJwtPayload>;
diff -r types/generated-snapshot/latest/index.ts bazel-bin/types/definitions/latest/index.ts
11628a11629,11669
> // Access JWT validation
> export type AccessJwtErrorCode =
> | "ERR_JWT_MISSING"
> | "ERR_JWT_MALFORMED"
> | "ERR_JWT_INVALID_SIGNATURE"
> | "ERR_JWT_EXPIRED"
> | "ERR_JWT_NOT_YET_VALID"
> | "ERR_JWT_AUDIENCE_MISMATCH"
> | "ERR_JWT_ISSUER_MISMATCH"
> | "ERR_JWKS_FETCH_FAILED"
> | "ERR_JWKS_NO_MATCHING_KEY"
> | "ERR_INVALID_TEAM_DOMAIN"
> | "ERR_INVALID_AUDIENCE";
> export class AccessJwtError extends Error {
> readonly code: AccessJwtErrorCode;
> constructor(code: AccessJwtErrorCode, message: string);
> }
> export interface AccessJwtPayload {
> aud: string | string[];
> email?: string;
> exp: number;
> iat: number;
> nbf?: number;
> iss: string;
> sub: string;
> [key: string]: unknown;
> }
> /**
> * Validates a Cloudflare Access JWT from an incoming request.
> *
> * @param req - The incoming Request containing the cf-access-jwt-assertion header
> * @param teamDomain - The Cloudflare One team domain (e.g., "mycompany" or "mycompany.cloudflareaccess.com")
> * @param audience - The Application Audience (AUD) tag
> * @throws {AccessJwtError} If validation fails for any reason
> * @returns The decoded JWT payload on success
> */
> export function validateAccessJwt(
> req: Request,
> teamDomain: string,
> audience: string,
> ): Promise<AccessJwtPayload>; |
55d2823 to
5ee0f15
Compare
|
The tests (after building...) fail locally — unclear (to me) why: |
5ee0f15 to
2dda59e
Compare
b678274 to
962eade
Compare
mhart
left a comment
There was a problem hiding this comment.
Some minor suggestions, just to use some slightly more modern/idiomatic methods, but looks great overall
962eade to
c343bd7
Compare
|
@mhart @anonrig I think I got this right — addressed all the comments:
|
a8cc773 to
5d87ec9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
Except the failing tests, the implementation looks really good. |
| async function fetchJwks(normalizedDomain: string): Promise<JwkSet> { | ||
| const url = `https://${normalizedDomain}/cdn-cgi/access/certs`; |
There was a problem hiding this comment.
Nit: best way to do this is by doing:
const url = new URL(`https://cloudflare.com/cdn-cgi/access/certs`)
url.hostname = normalizedDomain;
There was a problem hiding this comment.
shouldn't that be const url = new URL('https://cloudflare.com/cdn-cgi/access/certs);` ?
There was a problem hiding this comment.
The domain has to include the team name: e.g. https://opencode-devtools.cloudflareaccess.com/cdn-cgi/access/certs - as the certs are scoped to the Access team/domain
There was a problem hiding this comment.
You can just do url.hostname = "opencode-devtools.cloudflareaccess.com" on the new URL() and it would be a lot safer
There was a problem hiding this comment.
I don't think I understand the comments here :)
- The certs are always at
https://${teamName}.cloudflareaccess.com/cdn-cgi/access/certs-teamNameis dynamic - It's not static (to be extra clear)
We can make this https://${teamName}.cloudflareaccess.com/cdn-cgi/access/certs so that the string template doesn't need to cover the 'static' part of the domain but I don't see that as a net win?
|
@anonrig unfortunately getting the tests to run locally in any reliable way is beyond me, and I can't grok why the tests can't see the |
61d8b61 to
e799974
Compare
|
Changes to fix the test module import issue:
|
|
For internal reviewers: https://share.opencode.cloudflare.dev/share/fqzhroMQ |
|
@anonrig might need your help to get this over the line from a toolchain PoV. |
Adds a new validateAccessJwt() function that validates Cloudflare Access JWTs against team-specific JWKs. The function throws AccessJwtError with specific error codes on validation failure, making error handling explicit. Key features: - No external dependencies (uses WebCrypto APIs) - Retry logic for JWKS fetch (3 attempts, 5s backoff on 5xx) - Isolate-level JWKS caching (1 hour TTL) - Team domain normalization (accepts both short and full forms) - 60s clock skew tolerance for expiration
Tests can't import from cloudflare-internal:* modules directly. Each test now uses its own team domain to avoid cache conflicts.
- Add nbf (not-before) test case - Add iat (issued-at) validation for defense-in-depth - Wrap importKey in try/catch for meaningful error messages - Add JWK kty validation before import - Fix aud type to string | string[] per JWT spec - Remove misleading testJwks4xxError test
- Add iat !== undefined check to match nbf pattern - Fix testMultipleAudiences assertion for string | string[] type - Add testClockSkewToleranceIssuedAt for iat within tolerance - Add testAlgorithmNoneRejected (CVE-2015-9235 mitigation) - Add testAlgorithmCaseSensitive to ensure exact RS256 match
Security review found no critical vulnerabilities. Implementation properly mitigates algorithm confusion and key injection attacks. Changes: - Add type validation for nbf/iat claims (defense-in-depth against type confusion) - Add type validation for aud claim (must be string or array) - Add ERR_JWKS_INVALID_KEY error code for invalid JWKS keys vs malformed JWTs - Fix AccessJwtPayload.iat to be optional per RFC 7519 - Remove unused FixedLengthArray type helper - Remove redundant await in return statement - Add tests for non-numeric nbf/iat claims
e6f6733 to
7e410c0
Compare
| AccessJwtError, | ||
| type AccessJwtErrorCode, | ||
| type AccessJwtPayload, | ||
| } from 'cloudflare-internal:access-jwt'; |
There was a problem hiding this comment.
Adding this to the cloudflare:workers module means that this JavaScript code will need to be parsed and compiled by every isolate that imports cloudflare:workers, whether or not they actually use the JWT functions. The cloudflare:workers module is relevant to almost everyone these days, but the JWT functions are relevant to a much smaller set of people. Also, there's a proposal to add JWT validation to the native APIs which would make these functions obsolete -- but we'll still have to support them forever, of course.
Given all this I'd really prefer these functions be placed in a separate module, so that people who don't import the module aren't impacted.
(That said, I'd even more prefer that we just go ahead and implement the native API... it'd be pretty easy.)
There was a problem hiding this comment.
What’s the path forward? Do we keep telling customers to copy paste JWT validation code from the docs?
Are JWT native APIs landing this week? Next year?
There was a problem hiding this comment.
To be clear I was only asking that you move the function to a different module name, like cloudflare:access or something. (Though admittedly I don't quite understand why a built-in is better than an npm module here, but I won't fight on that point.)
The native API would be where ctx.accessCredentials is just automatically filled in with the verified credentials. I could (prompt Claude to) implement this in an hour probably except that the runtime doesn't know the worker's access team name (to fetch the keys) no audience. Seems like these should be part of the workers config and delivered via the control plane. If someone could spec that out and get the control plane to deliver that info we could ship it next week. But PMs and EMs control priorities here, not me.
But again, assuming we can't prioritize that now, I'm fine with the library, it just has to be a different module name.
There was a problem hiding this comment.
fwiw I have an engineer about to start working on this from the access side and will probably reach out to you @kentonv soon.
|
Closing this as it sounds like we're taking a different approach. |
Adds a
validateAccessJwt()function to thecloudflare:workersmodule that validates Cloudflare Access JWTs against team-specific JWKs, providing a built-in alternative for users to validate Access JWTs directly vs. glueing it all together themselves.AccessJwtErrorwith specific error codes (ERR_JWT_MISSING,ERR_JWT_EXPIRED, etc.) on validation failure - defaults to throwing exceptions that you must catch vs. someisValidJWT()type function that can be mishandled"myteam"and"myteam.cloudflareaccess.com")Usage
Error Codes
These will be added to the public docs under https://developers.cloudflare.com/cloudflare-one/access-controls/applications/http-apps/authorization-cookie/validating-json/ + new Workers
/examplesas wellERR_JWT_MISSINGcf-access-jwt-assertionheaderERR_JWT_MALFORMEDERR_JWT_INVALID_SIGNATUREERR_JWT_EXPIREDexpclaim is in the pastERR_JWT_NOT_YET_VALIDnbfclaim is in the futureERR_JWT_AUDIENCE_MISMATCHauddoesn't match expected audienceERR_JWT_ISSUER_MISMATCHissdoesn't match team domainERR_JWKS_FETCH_FAILEDERR_JWKS_NO_MATCHING_KEYkidfoundERR_INVALID_TEAM_DOMAINERR_INVALID_AUDIENCETest Plan
just test '//src/cloudflare/internal/test/access-jwt:access-jwt-test@'