Skip to content

Conversation

@mathiasmoser
Copy link

This Bugfix adds consideration of the newly added redirect_uris field in Oauth Clients Table. This new field was introduced with Passport 13. If you decided to switch to new oauth_clients table fields you will stumble over the described problem.

Behaviour without fix:

If you are using the Oauth Register Route in Laravel MCP to register a new Client you send the request:

The Register Request:

POST oauth/register

Body:
{"client_name":"Test","redirect_uris":["https://test1.com", "https://test2.com"]}

Response:
{
"client_id": "51",
"grant_types": [
"authorization_code",
"implicit",
"refresh_token",
"urn:ietf:params:oauth:grant-type:device_code"
],
"response_types": [
"code"
],
"redirect_uris": [],
"scope": "mcp:use",
"token_endpoint_auth_method": "none"
}

Problem: redirect_uris are empty because of the missing consideration of a potential redirect_uris field (instead of the old redirect field).

Behaviour with fix:

The Register Request:

POST oauth/register

Body:
{"client_name":"Test","redirect_uris":["https://test1.com", "https://test2.com"]}

Response:
{
"client_id": "51",
"grant_types": [
"authorization_code",
"implicit",
"refresh_token",
"urn:ietf:params:oauth:grant-type:device_code"
],
"response_types": [
"code"
],
"redirect_uris": [
"https://test1.com",
"https://test2.com"
],
"scope": "mcp:use",
"token_endpoint_auth_method": "none"
}

The Redirect URIs are now correctly returned!

Side Effects of the Bug

This Bug currently does lead to non working MCP Oauth Workflow with Anthropic Claude. OpenAI instead seems to have no problems with the missing redirect_uris in the response.

@hafezdivandari
Copy link
Contributor

@mathiasmoser
Copy link
Author

@hafezdivandari thanks for your input.

I still think that it is a bug in Passport.

Why?

The MCP Package is a Consumer of the Package and uses the offered Accessor Method "redirectUris". This method relies on a field name "redirect" which was the old way (before Passport 13) to store the redirectUris in the Client Table.

With Passport 13 the Structure of Client Table was refactored and the new field name is redirect_uris. But the switch to the new Client Table structure is not mandatory - so both possible field names (redirect and redirect_uris) should be supported.

This is exactly what this PR will provide.

After that it's safe to access the available Accessor Method "redirectUris" on the Client Model and for example also the MCP Package Problem is solved.

@hafezdivandari
Copy link
Contributor

@mathiasmoser I understand what you mean, but this accessor returns redirect_uris attribute if present and fallbacks to redirects as expected. As I said in the issue's comment above Model attributes are snake case. Please close this one and send a PR to MCP package if you want:

https://github.com/laravel/mcp/blob/01d44b89b20480409990a5051d522686756ddf2d/src/Server/Http/Controllers/OAuthRegisterController.php#L47-L54

return response()->json([
    'client_id' => (string) $client->id,
-   'grant_types' => $client->grantTypes,
+   'grant_types' => $client->grant_types,
    'response_types' => ['code'],
-   'redirect_uris' => $client->redirectUris,
+   'redirect_uris' => $client->redirect_uris,
    'scope' => 'mcp:use',
    'token_endpoint_auth_method' => 'none',
]);

cc @pushpak1300

@mathiasmoser
Copy link
Author

So this means $client->redirectUris should never be called because it would behave wrongly in combination with redirect_uris field. in my eyes my fix would have solved this. but fine, i will close the pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants