OAuth: Fix for multiple `redirect_uris`

[sugyan]

In the client metadata, multiple redirect_uri can be specified,

const clientMetadata: OAuthClientMetadata = {
  client_id: "https://oauth.example.com/client.json",
  response_types: ["code"],
  grant_types: ["authorization_code"],
  redirect_uris: [
    "https://oauth.example.com/callback1",
    "https://oauth.example.com/callback2",
  ],
};

Any value contained in the redirect_uris can be used in the options of the OAuthClient.authorize() method.

const client = new OAuthClient({
  clientMetadata,
  responseMode: "query",

  ...

});
const authorization_url = await client.authorize(input, {
  scope: "atproto",
  redirect_uri: "https://oauth.example.com/callback2",
});

However, even in the above case, the current implementation automatically selects and sends this.clientMetadata.redirect_uris[0] in the token request, so even if the redirected params are correctly obtained, exchangeCode() returns the following error.

{"error": "invalid_grant", "error_description": "This code was issued for another redirect_uri"}

Actually, the callback() method does not receive the redirect_uri, so I think the only way to get the value of the redirect_uri used in the PAR is through stateStore.

[matthieusieben]

I would be fine with incorporating something like this only it should be optional, especially for clients that only have one redirect uri.

[sugyan]

Thank you for responding to my pull request! I changed the redirectUri stored in the stateStore to an optional one. The same behavior as before is achieved by implementing the following: save only when the value is not equal to this.clientMetadata.redirect_uris[0], and use this.clientMetadata.redirect_uris[0] as the default value if there is no value when retrieving. This will avoid wasting space in the stateStore since the value is saved only in special cases.

[erlend-sh]

We’re running into this now as @avdb13 is using atrium to add support for atproto-oauth in rauthy.

avdb13 said:

Is it normal to start off with the client metadata containing only a single redirect uri when creating a new OAuthClient in Atrium?

let config = OAuthClientConfig {
        client_metadata: AtprotoLocalhostClientMetadata {
            redirect_uris: vec!["http://127.0.0.1".to_string()],
        },
        keys: None,
        resolver: oauth_resolver_config,
        state_store: MemoryStateStore::default(),
    };
    let client = OAuthClient::new(config)?;

    let authorization_url = client.authorize(&pds, atrium_oauth_client::AuthorizeOptions::default()).await.unwrap();

The Authorization Server Metadata endpoint (.well-known/oauth-authorization-server) will allow the client to obtain all the information it needs to initiate an OAuth2 authorization flow (see the server metadata section below). When a client ID uses "http://localhost" as origin, the AS will not be able to resolve the client metadata using the method described above. Instead, the Authorization Server will derive the client metadata document from the client ID.

avdb13 said:

I had a look at the authorize function, and it seems to do the correct thing as discussed in the implementation guide

sugyan said:

It should be possible to specify multiple redirect_uris with Vec<String>, but my understanding is that there is a bug in the current implementation (even in the TypeScript SDK) and it will not work correctly if anything other than the first uri is specified as an authorize's argument.