supabase-js icon indicating copy to clipboard operation
supabase-js copied to clipboard

fix(auth): correct getAuthorizationDetails return type for already consented OAuth responses

Open 7ttp opened this issue 1 month ago • 2 comments

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_uriredirect_url).

Related

Closes https://github.com/supabase/supabase-js/issues/2023

7ttp avatar Jan 15 '26 06:01 7ttp

I'd prefer a union type of the 2 possible options.

Kroppeb avatar Jan 15 '26 14:01 Kroppeb

I'd prefer a union type of the 2 possible options.

Lets see what @mandarini suggests, will adjust accordingly if needed! 💚🙌

7ttp avatar Jan 16 '26 02:01 7ttp

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:

  1. First-time authorization (user needs to consent):
  {                                                                                                                                                                                               
    authorization_id: string,                                                                                                                                                                     
    client: OAuthAuthorizationClient,                                                                                                                                                             
    user: { id: string, email: string },                                                                                                                                                          
    scope: string,                                                                                                                                                                                
    redirect_url?: string  // optional                                                                                                                                                            
  }                                                                                                                                                                                               
  1. 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!

mandarini avatar Jan 20 '26 10:01 mandarini

Coverage Status

coverage: 80.997%. remained the same when pulling 1600ea77e6e81d8775950d17a3079e18f3171c7b on 7ttp:fix/oauth-authorization-details-type into 09aa10628b00cbdf65ace0f9e8e79237e7c0c1dc on supabase:master.

coveralls avatar Jan 20 '26 13:01 coveralls

@mandarini can u have another look on this? also please lmk if any more changes are required! 😁💚

7ttp avatar Feb 04 '26 13:02 7ttp

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! 💚

mandarini avatar Feb 04 '26 14:02 mandarini

Fixed here: https://github.com/supabase/supabase-js/pull/2088

mandarini avatar Feb 04 '26 14:02 mandarini