fix: validate JWT audience and issuer claims#41
Conversation
|
Hey mate thx for it 💚 Meantime could you fix/rebase the merging conflicts? |
jwtVerify was called without audience or issuer options, so any JWT signed by a key in the JWKS was accepted regardless of who issued it or who it was intended for. In setups where multiple services share signing keys, a token from one service would pass verification on another. Add optional SUPABASE_JWT_AUDIENCE and SUPABASE_JWT_ISSUER env vars. When set, they are forwarded to jose's jwtVerify options. Existing behavior is unchanged when they are not set.
31fbfbd to
3c0193a
Compare
|
Rebased on latest main and resolved the conflicts. |
| | `SUPABASE_JWT_AUDIENCE` | `https://<ref>.supabase.co` | Expected JWT `aud` claim (optional) | All | | ||
| | `SUPABASE_JWT_ISSUER` | `https://<ref>.supabase.co/auth/v1` | Expected JWT `iss` claim (optional) | All | |
There was a problem hiding this comment.
Better keep it as non-default Supabase available.
Users need to manually supply it from env property when working in a Supabase Environment.
cc @tomaspozo, This envs are blockers if you consider merging it, like we discussed before I don't plan to expose it at Platform / any other Supabase environment.
There was a problem hiding this comment.
It should be fine to merge without coupling this envs, so the feature JWT audience will be optional and requires manual setup.
@alanzabihi, are u okay with manual setup using env property? like I described in the comment above, the requirement of theses envs are blocking this PR.
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
jwtVerifyinverify-credentials.tsis called withoutaudienceorissueroptions:Any JWT signed by a key in the project's JWKS is accepted, regardless of who issued it or who it was intended for. In setups where multiple services share signing keys (common in self-hosted Supabase), a token minted by Service A is accepted by Service B as valid user auth.
What is the new behavior?
Two new optional fields on
SupabaseEnv:audienceandissuer. When set, they're forwarded tojwtVerify's options. Tokens that don't match are rejected withInvalidCredentialsError.New environment variables:
SUPABASE_JWT_AUDIENCEandSUPABASE_JWT_ISSUER. Both are optional. Existing behavior is unchanged when they aren't set.Files changed:
src/types.ts-- addaudience?andissuer?toSupabaseEnvsrc/core/resolve-env.ts-- read the new env varssrc/core/verify-credentials.ts-- pass them tojwtVerifydocs/security.md-- document the new options and why they matterdocs/environment-variables.md-- list the new varsTests:
The actual logic this PR adds is: "if the env field is set, pass it to
jose." The claim validation itself happens insidejose. Every existingusermode test already callsmakeEnv()without audience or issuer, so the backward-compatible path is implicitly covered.The one test worth adding is a
resolveEnvwiring test: does it correctly readSUPABASE_JWT_AUDIENCEandSUPABASE_JWT_ISSUERfrom the environment and surface them onSupabaseEnv? Happy to add that if you'd like.