fix: add JWT signature verification using JWKS#1700
Closed
WcaleNieWolny wants to merge 4 commits into
Closed
Conversation
Strengthen JWT validation by verifying signatures against the Supabase JWKS endpoint before trusting claims. Previously getClaimsFromJWT decoded payloads locally without signature verification. A new verifyJWT function uses jose's createRemoteJWKSet + jwtVerify to cryptographically verify tokens. All auth middleware and direct callers are updated.
Contributor
📝 WalkthroughWalkthroughReplaces synchronous JWT extraction with an asynchronous JWKS/HS256 verifier across auth handlers, introduces a cached JWKS loader and async Changes
Sequence DiagramsequenceDiagram
participant Handler as HTTP Handler
participant Middleware as middlewareAuth
participant Verify as verifyJWT(context, jwt)
participant Cache as JWKS Cache
participant Remote as Supabase JWKS Endpoint
participant JoseLib as jose.jwtVerify()
Handler->>Middleware: receives Authorization header
Middleware->>Verify: verifyJWT(c, jwt)
Verify->>Cache: getJWKS(c)
alt JWKS cached
Cache-->>Verify: cached JWKS
else JWKS not cached
Cache->>Remote: fetch JWKS
Remote-->>Cache: JWKS payload
Cache-->>Verify: JWKS
end
Verify->>JoseLib: jwtVerify(jwt, jwks)
alt JWKS verification succeeds
JoseLib-->>Verify: decoded claims
Verify-->>Middleware: JWTClaims
Middleware-->>Handler: auth context set
else JWKS verification fails
JoseLib-->>Verify: error
Verify->>Verify: try HS256 fallback (shared secret)
alt HS256 succeeds
Verify-->>Middleware: JWTClaims
Middleware-->>Handler: auth context set
else HS256 fails
Verify-->>Middleware: null
Middleware-->>Handler: 401 invalid_jwt
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
All auth paths now use verifyJWT with JWKS-based signature verification, so the decode-only helper is no longer needed.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
supabase/functions/_backend/utils/hono.ts (1)
59-60: Log verification failures instead of silently collapsing all errors tonull.The current
catch { return null }makes invalid-token errors indistinguishable from JWKS fetch/config failures during incidents.🛠️ Proposed observability improvement
- catch { + catch (error) { + cloudlog({ + requestId: c.get('requestId'), + message: 'jwt_verification_failed', + error: error instanceof Error ? error.message : String(error), + }) return null }As per coding guidelines "Use structured logging with
cloudlog({ requestId: c.get('requestId'), message: '...' })for all backend logging".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/hono.ts` around lines 59 - 60, Replace the silent catch in the token verification path (the catch { return null } block) with a structured log: change it to catch (err) { cloudlog({ requestId: c.get('requestId'), message: 'Token verification failure', error: err, context: 'verifyJwt / JWKS fetch' }); return null; } so verification errors (invalid token vs JWKS/config failures) are recorded using cloudlog and include the error and requestId for observability; keep the return null behavior unchanged.supabase/functions/_backend/private/verify_email_otp.ts (1)
50-51: Avoid double-verifying the same JWT in this request.
middlewareAuthalready verifies this token. Re-verifying here adds redundant JWKS work and a second failure point; consider storing verified claims in context and reusing them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/verify_email_otp.ts` around lines 50 - 51, middlewareAuth already verifies the JWT, so remove the redundant verifyJWT call in verify_email_otp.ts and read the verified claims populated by middlewareAuth (e.g., c.authClaims or the field middlewareAuth sets on context) instead of calling verifyJWT(c, authorization); replace "const claims = await verifyJWT(c, authorization); const email = claims?.email" with code that reads the claims/email from the context (and throw a clear error if that field is missing), and remove any now-unused imports or variables related to verifyJWT/authorization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/utils/hono.ts`:
- Around line 55-57: The verification currently only checks signature; update
the jwtVerify call in the block that creates jwks via getJWKS(c) to pass
explicit claim constraints (issuer and audience) so tokens are also validated
for intended issuer/audience; retrieve the expected values from your
configuration/context (e.g., from c.env or your app config) and call
jwtVerify(token, jwks, { issuer: expectedIssuer, audience: expectedAudience }),
then return the payload as JWTClaims as before.
---
Nitpick comments:
In `@supabase/functions/_backend/private/verify_email_otp.ts`:
- Around line 50-51: middlewareAuth already verifies the JWT, so remove the
redundant verifyJWT call in verify_email_otp.ts and read the verified claims
populated by middlewareAuth (e.g., c.authClaims or the field middlewareAuth sets
on context) instead of calling verifyJWT(c, authorization); replace "const
claims = await verifyJWT(c, authorization); const email = claims?.email" with
code that reads the claims/email from the context (and throw a clear error if
that field is missing), and remove any now-unused imports or variables related
to verifyJWT/authorization.
In `@supabase/functions/_backend/utils/hono.ts`:
- Around line 59-60: Replace the silent catch in the token verification path
(the catch { return null } block) with a structured log: change it to catch
(err) { cloudlog({ requestId: c.get('requestId'), message: 'Token verification
failure', error: err, context: 'verifyJwt / JWKS fetch' }); return null; } so
verification errors (invalid token vs JWKS/config failures) are recorded using
cloudlog and include the error and requestId for observability; keep the return
null behavior unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
supabase/functions/_backend/private/validate_password_compliance.tssupabase/functions/_backend/private/verify_email_otp.tssupabase/functions/_backend/public/replication.tssupabase/functions/_backend/utils/hono.tssupabase/functions/_backend/utils/hono_middleware.ts
Constrain jwtVerify to only accept tokens issued by our Supabase auth endpoint with the 'authenticated' audience claim.
|
Contributor
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 `@supabase/functions/_backend/utils/hono.ts`:
- Around line 70-80: The function getSharedJwtSecret currently returns the
public LOCAL_SUPABASE_DEFAULT_JWT_SECRET solely based on hostname checks
(localhost/127.0.0.1/kong); change it to only return that default when an
explicit local-mode env flag is set (e.g., require getEnv(c,
'<LOCAL_MODE_FLAG>') === 'true' such as SUPABASE_LOCAL or
SUPABASE_DISABLE_JWT_PROTECTION), otherwise do not fall back to
LOCAL_SUPABASE_DEFAULT_JWT_SECRET. Update both places where this fallback occurs
in getSharedJwtSecret to first check the explicit local-mode config (using
getEnv with the chosen flag) before returning LOCAL_SUPABASE_DEFAULT_JWT_SECRET,
preserving existing behavior when SUPABASE_JWT_SECRET or JWT_SECRET are
provided.
Comment on lines
+70
to
+80
| function getSharedJwtSecret(c: Context<MiddlewareKeyVariables>, rawSupabaseUrl: string): string { | ||
| const configuredSecret = getEnv(c, 'SUPABASE_JWT_SECRET') || getEnv(c, 'JWT_SECRET') | ||
| if (configuredSecret) | ||
| return configuredSecret | ||
|
|
||
| try { | ||
| const normalized = normalizeSupabaseBaseUrl(rawSupabaseUrl) | ||
| const host = new URL(normalized).hostname | ||
| if (host === '127.0.0.1' || host === 'localhost' || host === 'kong') | ||
| return LOCAL_SUPABASE_DEFAULT_JWT_SECRET | ||
| } |
Contributor
There was a problem hiding this comment.
Gate local default JWT secret behind explicit local-mode config.
The known default secret is enabled from hostname checks alone (localhost/127.0.0.1/kong). If SUPABASE_URL is accidentally set to one of these in a non-local deployment, forged HS tokens signed with the public default secret can pass verification.
Proposed fix
function getSharedJwtSecret(c: Context<MiddlewareKeyVariables>, rawSupabaseUrl: string): string {
const configuredSecret = getEnv(c, 'SUPABASE_JWT_SECRET') || getEnv(c, 'JWT_SECRET')
if (configuredSecret)
return configuredSecret
+ const envName = (getEnv(c, 'ENV_NAME') || '').toLowerCase()
+ const allowLocalFallback
+ = getEnv(c, 'ALLOW_LOCAL_SUPABASE_JWT_FALLBACK') === 'true'
+ || envName === 'local'
+ || envName === 'development'
+ || envName === 'dev'
+
try {
const normalized = normalizeSupabaseBaseUrl(rawSupabaseUrl)
const host = new URL(normalized).hostname
- if (host === '127.0.0.1' || host === 'localhost' || host === 'kong')
+ if (allowLocalFallback && (host === '127.0.0.1' || host === 'localhost' || host === 'kong'))
return LOCAL_SUPABASE_DEFAULT_JWT_SECRET
}
catch {
}
return ''
}Also applies to: 124-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/_backend/utils/hono.ts` around lines 70 - 80, The function
getSharedJwtSecret currently returns the public
LOCAL_SUPABASE_DEFAULT_JWT_SECRET solely based on hostname checks
(localhost/127.0.0.1/kong); change it to only return that default when an
explicit local-mode env flag is set (e.g., require getEnv(c,
'<LOCAL_MODE_FLAG>') === 'true' such as SUPABASE_LOCAL or
SUPABASE_DISABLE_JWT_PROTECTION), otherwise do not fall back to
LOCAL_SUPABASE_DEFAULT_JWT_SECRET. Update both places where this fallback occurs
in getSharedJwtSecret to first check the explicit local-mode config (using
getEnv with the chosen flag) before returning LOCAL_SUPABASE_DEFAULT_JWT_SECRET,
preserving existing behavior when SUPABASE_JWT_SECRET or JWT_SECRET are
provided.
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.
Summary
verifyJWTfunction usingjoselibrary'screateRemoteJWKSet+jwtVerifyto cryptographically verify JWT signatures against the Supabase JWKS endpointmiddlewareAuthandmiddlewareV2(viafoundJWT) to use signature-verified claims instead of raw decodegetClaimsFromJWTin auth paths to useverifyJWTgetClaimsFromJWTfor non-auth reads with updated docstring warningTest plan
middlewareV2endpoints (devices, stats) reject forged JWTsSummary by CodeRabbit