fix: key name resolution for client creation#9
Conversation
…lting verifyCredentials now returns the matched keyName in AuthResult. createContextClient and createAdminClient accept an optional keyName parameter, falling back to 'default' when not provided.
…e key Client creation now falls back to first available key when no 'default' exists and keyName is null. Tests cover named keys, wildcard matching non-first keys, and the full fallback chain.
There was a problem hiding this comment.
Pull request overview
This PR improves multi-key support by carrying the matched key name through authentication and using it during Supabase client creation, rather than always assuming "default".
Changes:
- Extend
AuthResultto includekeyName, and haveverifyCredentialspopulate it (including wildcard matches). - Update
createSupabaseContextto pass the matchedkeyNameintocreateContextClient/createAdminClient. - Enhance client creation helpers to support named keys and add fallback behavior when
"default"is absent; update README examples and document bare-array JWKS parsing.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds keyName to AuthResult to propagate matched key identity. |
| src/create-supabase-context.ts | Uses auth.keyName to select named keys when creating clients. |
| src/core/verify-credentials.ts | Returns the matched key name for public/secret modes (incl. wildcard). |
| src/core/verify-credentials.test.ts | Adds assertions ensuring keyName is returned correctly across modes. |
| src/core/create-context-client.ts | Adds keyName parameter and named-key resolution + fallback. |
| src/core/create-context-client.test.ts | Adds coverage for named key selection and fallback/empty-key behavior. |
| src/core/create-admin-client.ts | Adds keyName parameter and named-key resolution + fallback. |
| src/core/create-admin-client.test.ts | Adds coverage for named key selection and fallback/empty-key behavior. |
| README.md | Updates Deno examples to export default { fetch } and documents JWKS bare array format. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const secretKey = resolved.secretKeys['default'] | ||
| const name = keyName ?? 'default' | ||
| const keys = resolved.secretKeys | ||
| const secretKey = keys[name] ?? (keyName ? undefined : Object.values(keys)[0]) |
There was a problem hiding this comment.
Same issue as createContextClient: fallback logic uses a truthiness check on keyName (keyName ? undefined : Object.values(keys)[0]). Passing keyName='' will incorrectly allow fallback-to-first-key. Prefer checking keyName == null (or explicitly rejecting empty strings) so explicit-but-empty values don’t change behavior unexpectedly.
| const secretKey = keys[name] ?? (keyName ? undefined : Object.values(keys)[0]) | |
| const secretKey = keys[name] ?? (keyName == null ? Object.values(keys)[0] : undefined) |
Use keyName == null instead of truthiness check so empty strings don't incorrectly trigger fallback. Error messages now only suggest SUPABASE_*_KEY for the default key, not for named keys.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const supabase = createContextClient(auth.token, options?.env) | ||
| const supabaseAdmin = createAdminClient(options?.env) | ||
| const supabase = createContextClient(auth.token, options?.env, auth.keyName) | ||
| const supabaseAdmin = createAdminClient(options?.env, auth.keyName) |
There was a problem hiding this comment.
createSupabaseContext passes auth.keyName to createAdminClient for all auth types. For public:* / public:<name>, auth.keyName is derived from publishableKeys, but createAdminClient looks up secretKeys[name] and will throw if secret keys aren’t named identically. This can turn a successful public auth into a 500. Consider only passing keyName to createAdminClient when auth.authType === 'secret' (or implement a fallback to default/first secret key when the named secret key is missing).
| const supabaseAdmin = createAdminClient(options?.env, auth.keyName) | |
| const adminKeyName = auth.authType === 'secret' ? auth.keyName : undefined | |
| const supabaseAdmin = createAdminClient(options?.env, adminKeyName) |
| token: string | null | ||
| userClaims: UserClaims | null | ||
| claims: JWTClaims | null | ||
| keyName: string | null |
There was a problem hiding this comment.
AuthResult is an exported type, and adding a new required field (keyName) is a TypeScript breaking change for consumers who construct/annotate AuthResult values. If you want this to be non-breaking, consider making it optional (keyName?: string | null) while still returning it from verifyCredentials, or ensure this change is released as a breaking/major bump.
| keyName: string | null | |
| keyName?: string | null |
…optional Admin client now only receives keyName when authType is 'secret', preventing mismatched key lookups when auth is public. AuthResult.keyName is now optional to avoid breaking consumers who construct the type.
kallebysantos
left a comment
kallebysantos
left a comment
There was a problem hiding this comment.
Looks good to me!!
Just noticing that some docs was changed outside main PR context...
Summary
verifyCredentialsnow returns the matchedkeyNameinAuthResult, so downstream code knows which named key was used for authcreateContextClientandcreateAdminClientuse the matched key name instead of always defaulting to'default'— with fallback to first available key when no'default'existsexport default { fetch }format, removed repetitive imports, and documented bare array JWKS format