fix(auth): correct getAuthorizationDetails return type for already consented OAuth responses
Summary
Make OAuthAuthorizationDetails fields optional to support already-consented OAuth responses.
Problem
getAuthorizationDetails is typed to return AuthOAuthAuthorizationDetailsResponse with required fields (authorization_id, client, user, scope).
However, when a user has already approved access to an OAuth client, the API returns only { redirect_url: string } without the other fields, causing TypeScript type mismatches.
Solution
Made authorization_id, client, user, and scope optional in OAuthAuthorizationDetails type.
Also fixed a typo in the JSDoc (redirect_uri → redirect_url).
Related
Closes https://github.com/supabase/supabase-js/issues/2023
I'd prefer a union type of the 2 possible options.
I'd prefer a union type of the 2 possible options.
Lets see what @mandarini suggests, will adjust accordingly if needed! 💚🙌
Thank you for identifying and fixing this issue! The type mismatch you've found is indeed a real bug, the current types don't match the actual API behavior. However, I'd like to suggest an alternative approach that preserves type safety while fixing the issue.
While making all fields optional does technically solve the TypeScript error, it loses important type information:
// With optional fields, TypeScript can't help us:
const { data } = await getAuthorizationDetails(id)
console.log(data.client.name) // TypeScript says this is fine, but might crash at runtime
The API actually returns two distinct response shapes depending on the authorization state:
- First-time authorization (user needs to consent):
{
authorization_id: string,
client: OAuthAuthorizationClient,
user: { id: string, email: string },
scope: string,
redirect_url?: string // optional
}
- Already consented (immediate redirect):
{
redirect_url: string
}
I think we should use a union type like @Kroppeb suggested to represent both cases accurately. The codebase already has AuthOAuthConsentResponse for the already-consented case (line 1794).
Here's the proposed change:
// Keep OAuthAuthorizationDetails as-is (required fields for first-time auth)
export type OAuthAuthorizationDetails = {
/** The authorization ID */
authorization_id: string
/** Redirect URL - present if user already consented (can be used to trigger immediate redirect) */
redirect_url?: string
/** OAuth client requesting authorization */
client: OAuthAuthorizationClient
/** User object associated with the authorization */
user: {
/** User ID (UUID) */
id: string
/** User email */
email: string
}
/** Space-separated list of requested scopes */
scope: string
}
// Add a new type for the already-consented response
export type OAuthAlreadyConsentedDetails = {
/** URL to redirect the user back to the OAuth client */
redirect_url: string
}
// Update the response type to be a union
export type AuthOAuthAuthorizationDetailsResponse = RequestResult<
| OAuthAuthorizationDetails
| OAuthAlreadyConsentedDetails
>
With this approach, TypeScript forces developers to handle both scenarios:
const { data, error } = await supabase.auth.oauth.getAuthorizationDetails(authorizationId)
if (error || !data) {
// Handle error
return
}
// Type narrowing using discriminator
if ('authorization_id' in data) {
// TypeScript knows all fields are present here
console.log('Client:', data.client.name)
console.log('Scopes:', data.scope)
console.log('User:', data.user.email)
// Show consent UI...
} else {
// TypeScript knows only redirect_url is present
console.log('Already consented, redirecting to:', data.redirect_url)
// Handle redirect...
}
The current types claim all fields are required, but the API sometimes returns only redirect_url. So the existing types are incorrect and misleading. Fixing them to match reality is a bug fix, not a breaking change. Existing code that incorrectly assumes all fields are always present already has runtime bugs that just haven't surfaced yet. The union type approach will surface these bugs at compile time, which is exactly what TypeScript is for.
P.S. The JSDoc typo fix (redirect_uri → redirect_url) is perfect - that should definitely stay regardless of which approach we take!
coverage: 80.997%. remained the same when pulling 1600ea77e6e81d8775950d17a3079e18f3171c7b on 7ttp:fix/oauth-authorization-details-type into 09aa10628b00cbdf65ace0f9e8e79237e7c0c1dc on supabase:master.
@mandarini can u have another look on this? also please lmk if any more changes are required! 😁💚
Hey @7ttp! Unfortunately, I think I'm going to close this PR in favor of a more comprehensive fix. The problem is on us.
The issue with this PR: While you added the union type for the already-consented case, you missed the underlying bug in OAuthAuthorizationDetails itself. The API actually returns redirect_uri (not redirect_url) when consent is needed. This was incorrectly changed in PR #1891.
What we're doing instead:
- Fix the core bug: redirect_url → redirect_uri in OAuthAuthorizationDetails (matches actual API)
- Add union type (as you did), but with a more generic OAuthRedirect type
- Consolidate duplicate types (reuse OAuthRedirect in consent response)
- Fix all JSDoc to clarify which field names appear in which cases
Your PR would have fixed the already-consented case but left the consent-needed case still broken with the wrong field name. Thanks again for reporting this! 💚
Fixed here: https://github.com/supabase/supabase-js/pull/2088