feat: add hybrid jwt verification#4721
Conversation
Pull Request Test Coverage Report for Build 22905960749Details
💛 - Coveralls |
|
@kallebysantos const JWT_SECRET = Deno.env.get('SUPABASE_INTERNAL_JWT_SECRET')!;
const HOST_PORT = Deno.env.get('SUPABASE_INTERNAL_HOST_PORT')!;
const DEBUG = Deno.env.get('SUPABASE_INTERNAL_DEBUG') === 'true';
const FUNCTIONS_CONFIG_STRING = Deno.env.get(
'SUPABASE_INTERNAL_FUNCTIONS_CONFIG'
)!;
async function verifyLegacyJWT(
jwt: string
): Promise<JWTVerifyResult<JWTPayload> | undefined> {
// const JWT_SECRET = Deno.env.get('SUPABASE_INTERNAL_JWT_SECRET')!;
logger.debug('JWT secret: ' + JWT_SECRET);
logger.debug('Host port: ' + HOST_PORT);
logger.debug('Debug: ' + DEBUG);
logger.debug('Functions config string: ' + FUNCTIONS_CONFIG_STRING);
const encoder = new TextEncoder();
const secretKey = encoder.encode(JWT_SECRET);
try {
return await jwtVerify(jwt, secretKey);
} catch (e) {
logger.error('Symmetric Legacy JWT verification error', e);
return undefined;
}
}
|
|
Hi @joelpramos 💚 |
|
hi @kallebysantos:
|
|
Hey @joelpramos 💚 Since you're using self-hosted, the I'm not really sure, but new JWK are not available for self-hosting yet, so you "can't" replicate this hybrid replication — At least without extra manual steps. cc @aantti can give you more details about Self-Host situation of new Asymmetric Keys. |
It's WIP :) Hoping to add it soon. |
|
@aantti @kallebysantos at least locally the new solution works totally fine for users but not for service_user (e.g., functions triggered from Cron). I am also having some issues in the deployed instance with functions triggered from cron jobs but haven't triaged yet the root cause and missing some logs. fwiw other than the fact the variable wasn't available, this hybrid solution you posted @kallebysantos works for me i.e. seems like the service_role from the cron user is using the old auth method. Is that something on both your radars? |
|
Hey @joelpramos
Yes,
I don't think so, user's tokens is now being issued as new JWT (Asymmetric JWK) and the purpose of this PR is to handle both kinds of tokens, using legacy as fallback. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces the jose import with jsr: Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant RequestHandler as Request Handler
participant VerifyHybrid as verifyHybridJWT()
participant AlgoCheck as Algorithm Checker
participant LegacyPath as isValidLegacyJWT()
participant JWKSLoader as JWKS Loader
participant JWKSRemote as JWKS Endpoint
Client->>RequestHandler: HTTP request with Authorization: Bearer <token>
RequestHandler->>VerifyHybrid: verifyHybridJWT(JWT_SECRET, JWKS_ENDPOINT, token)
VerifyHybrid->>AlgoCheck: read "alg" from JWT header
alt alg == "HS256"
AlgoCheck->>LegacyPath: verify with jwtSecret
LegacyPath-->>VerifyHybrid: success/failure
else alg == "ES256" or "RS256"
AlgoCheck->>JWKSLoader: ensure jwks loaded (SUPABASE_INTERNAL_JWKS or fetch)
JWKSLoader->>JWKSRemote: fetch JWKS (lazy, if needed)
JWKSRemote-->>JWKSLoader: return keys
JWKSLoader->>VerifyHybrid: provide JWK
VerifyHybrid->>VerifyHybrid: verify using public key
VerifyHybrid-->>RequestHandler: success/failure
end
VerifyHybrid-->>RequestHandler: verification outcome
Assessment against linked issues
Out-of-scope changes
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Looking forward to see this happen. Currently, we're lowering security on our apps to work around this issue. No action here in two weeks isn't impressive for a bug impacting security/auth. |
So I guess what I am not yet 100% clear is what is now the flow for my Postgres Functions ( Not sure if there's a different network route that it isn't sending it through Kong to 'transform' the Bearer token from secret API key into a valid JWT? At the end of the day it's not clear yet to me what is the appropriate configuration for this set up... so that I can wrap these function calls with cron. Clearly behaviours diverge. |
|
Hey guys!
We do understand your disappointment 🥲
Unfortunately there's a different behaviour on how Local CLI is handling the
Its a good catch 👍 |
|
Hey @joelpramos 💚 While we're still working on a better official solution Using new API Keys to create webhooks:
Details
Details// Setup type definitions for built-in Supabase Runtime APIs
import "jsr:@supabase/functions-js/edge-runtime.d.ts";
import { createClient } from 'npm:@supabase/supabase-js@2'
console.info('server started');
Deno.serve(async (req: Request) => {
const apiKey = req.headers.get('apiKey')
if (apiKey !== Deno.env.get('SB_CRON_WEBHOOK')!) {
return new Response(null, { status: 401 });
}
// Use the request 'apiKey' to init supabase client
const supabase = createClient(Deno.env.get('SUPABASE_URL')!, apiKey);
// Do somenthing
return new Response();
});
|
Allows verify new JWTs as well legacy
It helps to reduce latency for Legacy token verifications, since it avoid unnecessary requests.
kallebysantos
force-pushed
the
fix-update-functions-verify-jwt
branch
from
4c30f0a to
00c30af
Compare
- It reduces functions bootime, since there's no need to fetch JWK on fly
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/functions/serve/templates/main.ts`:
- Around line 122-137: The current initialization of jwks using
createLocalJWKSet when SUPABASE_INTERNAL_JWKS is present makes isValidJWT always
use the static set and never fall back to remote keys; update isValidJWT (or the
jwks initialization) so that when jwt verification fails due to unknown key ID
(kid) it falls back to a createRemoteJWKSet(new URL(jwksUrl)) resolver and
retries verification, or prefer the remote resolver when SUPABASE_INTERNAL_JWKS
exists but may be stale; specifically modify the logic in isValidJWT to catch
key-not-found verification errors from jose.jwtVerify and then assign jwks =
jose.createRemoteJWKSet(new URL(jwksUrl)) before re-invoking jwtVerify so
rotated keys are accepted while keeping the local set as an initial
optimization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1d8a5b6b-102d-4cda-8222-657cbc10d71b
📒 Files selected for processing (1)
internal/functions/serve/templates/main.ts
| let jwks = (() => { | ||
| try { | ||
| // using injected JWKS from cli | ||
| return jose.createLocalJWKSet(JSON.parse(Deno.env.get('SUPABASE_INTERNAL_JWKS'))); | ||
| } catch (error) { | ||
| return null | ||
| } | ||
| })(); | ||
|
|
||
| async function isValidJWT(jwksUrl: string, jwt: string): Promise<boolean> { | ||
| try { | ||
| if (!jwks) { | ||
| // Loading from remote-url on fly | ||
| jwks = jose.createRemoteJWKSet(new URL(jwksUrl)); | ||
| } | ||
| await jose.jwtVerify(jwt, jwks); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l internal/functions/serve/templates/main.tsRepository: supabase/cli
Length of output: 102
🏁 Script executed:
cat -n internal/functions/serve/templates/main.ts | head -150Repository: supabase/cli
Length of output: 5779
🌐 Web query:
@panva/jose createLocalJWKSet vs createRemoteJWKSet behavior key rotation
💡 Result:
Key rotation behavior differences
createLocalJWKSet(jwks)
- Uses only the JWKS object you pass in (“locally stored, or otherwise available”). It will never re-fetch or “learn” rotated keys by itself. If the issuer rotates keys (new
kid), you must replace/update the JWKS in your app (e.g., refetch + rebuild your verifier) for verification to start working again. [1]
createRemoteJWKSet(url, options?)
- Fetches the JWKS from the remote
jwks_urilazily and refreshes when needed:- If the incoming token’s header (
alg/kid) doesn’t match any cached key, it will fetch the JWKS again (but rate-limited bycooldownDurationto prevent abuse). This is the main mechanism that makes key rotation “just work” in steady state. [2] - Independently, it won’t go longer than
cacheMaxAgebetween successful HTTP fetches (default 10 minutes), so it will periodically refresh even without rotation-triggered misses. [3]
- If the incoming token’s header (
- Recreating the RemoteJWKSet instance on failures is discouraged; it can allow attackers to force constant reloads—keep one instance and let its cooldown/caching behavior do the right thing. [4]
Practical summary for rotation
- Local JWKS: rotation requires your refresh logic.
- Remote JWKS: rotation is typically automatic: first request with a new
kidtriggers a refetch (subject tocooldownDuration), then verification succeeds once the new key appears in JWKS. [2]
Sources:
[1] createLocalJWKSet docs (panva/jose) (github.com)
[2] createRemoteJWKSet docs (panva/jose) (github.com)
[3] RemoteJWKSetOptions (cacheMaxAge, cooldownDuration, defaults) (github.com)
[4] panva guidance on not recreating RemoteJWKSet instance (github.com)
Citations:
- 1: https://github.com/panva/jose/blob/main/docs/jwks/local/functions/createLocalJWKSet.md
- 2: https://github.com/panva/jose/blob/main/docs/jwks/remote/functions/createRemoteJWKSet.md
- 3: https://github.com/panva/jose/blob/main/docs/jwks/remote/interfaces/RemoteJWKSetOptions.md
- 4: Pattern for invalidating cached remote JWKS panva/jose#448
🏁 Script executed:
cat -n internal/functions/serve/templates/main.ts | sed -n '131,165p'Repository: supabase/cli
Length of output: 1384
🏁 Script executed:
# Check if jwks variable is reassigned anywhere else in the file
rg "jwks\s*=" internal/functions/serve/templates/main.ts -nRepository: supabase/cli
Length of output: 139
Don't freeze asymmetric verification to the startup JWKS snapshot.
If SUPABASE_INTERNAL_JWKS is present, jwks is set to a createLocalJWKSet at startup and never reaches the remote resolver. createLocalJWKSet only resolves against the supplied local keys and does not refresh, while createRemoteJWKSet refetches when encountering an unknown kid (key ID). After a signing-key rotation, valid ES256/RS256 tokens will fail verification until the process restarts. This breaks the stated migration scenario. Either fall back to remote resolution when the local set misses a key, or prefer the remote resolver when available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/functions/serve/templates/main.ts` around lines 122 - 137, The
current initialization of jwks using createLocalJWKSet when
SUPABASE_INTERNAL_JWKS is present makes isValidJWT always use the static set and
never fall back to remote keys; update isValidJWT (or the jwks initialization)
so that when jwt verification fails due to unknown key ID (kid) it falls back to
a createRemoteJWKSet(new URL(jwksUrl)) resolver and retries verification, or
prefer the remote resolver when SUPABASE_INTERNAL_JWKS exists but may be stale;
specifically modify the logic in isValidJWT to catch key-not-found verification
errors from jose.jwtVerify and then assign jwks = jose.createRemoteJWKSet(new
URL(jwksUrl)) before re-invoking jwtVerify so rotated keys are accepted while
keeping the local set as an initial optimization.
There was a problem hiding this comment.
while createRemoteJWKSet refetches when encountering an unknown
Not really true, the previous one also freeze the jwks and was only fetching it at 1º startup.
Also since its local dev, its already expected to users full restart after config.toml changes or similar.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/api/types.gen.go`:
- Around line 3400-3401: You added the generated field PresenceEnabled
(json:"presence_enabled") directly into pkg/api/types.gen.go which is
overwritten by codegen; instead add the presence_enabled property to the OpenAPI
overlay used by generation (update api/overlay.yaml or the source OpenAPI spec
referenced by pkg/api/types.cfg.yaml) so the field is part of the schema, then
run the oapi-codegen regeneration to produce an updated pkg/api/types.gen.go
(verify the PresenceEnabled field appears and remove your manual edit). Ensure
any corresponding comments/metadata match the added schema entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d2b46ad7-4478-4fa6-bacf-d0fcf9c8678b
📒 Files selected for processing (1)
pkg/api/types.gen.go
7 participants
What kind of change does this PR introduce?
Bug fix, feature
What is the current behavior?
Since API keys did change, users can't call functions without manually disabling
verify_jwt.What is the new behavior?
JWKs are now default exposed #4688, so It allows to verify both new asymmetric tokens as well legacy ones.
This way it applies a temporary fix while migrating API keys.