Skip to content

Commit 5652305

Browse files
committed
fix: address MR review comments for ClientRegistration
- Use Mcp\Exception\InvalidArgumentException instead of global \InvalidArgumentException - Add interface documentation for ClientRegistrarInterface with RFC 7591 references
1 parent 7974a2f commit 5652305

3 files changed

Lines changed: 20 additions & 5 deletions

File tree

src/Server/Transport/Http/Middleware/ClientRegistrationMiddleware.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Http\Discovery\Psr17FactoryDiscovery;
1515
use Mcp\Exception\ClientRegistrationException;
16+
use Mcp\Exception\InvalidArgumentException;
1617
use Mcp\Server\Transport\Http\OAuth\ClientRegistrarInterface;
1718
use Psr\Http\Message\ResponseFactoryInterface;
1819
use Psr\Http\Message\ResponseInterface;
@@ -42,7 +43,7 @@ public function __construct(
4243
?StreamFactoryInterface $streamFactory = null,
4344
) {
4445
if ('' === trim($localBaseUrl)) {
45-
throw new \InvalidArgumentException('The $localBaseUrl must not be empty.');
46+
throw new InvalidArgumentException('The $localBaseUrl must not be empty.');
4647
}
4748

4849
$this->responseFactory = $responseFactory ?? Psr17FactoryDiscovery::findResponseFactory();

src/Server/Transport/Http/OAuth/ClientRegistrarInterface.php

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,28 @@
1515

1616
/**
1717
* Interface for OAuth 2.0 Dynamic Client Registration (RFC 7591).
18+
*
19+
* Implementations are responsible for persisting client credentials and
20+
* returning a registration response as defined in RFC 7591 Section 3.2.
21+
*
22+
* @see https://datatracker.ietf.org/doc/html/rfc7591
1823
*/
1924
interface ClientRegistrarInterface
2025
{
2126
/**
22-
* @param array<string, mixed> $registrationRequest
27+
* Registers a new OAuth 2.0 client.
28+
*
29+
* The registration request contains metadata fields as defined in RFC 7591
30+
* Section 2 (e.g. redirect_uris, client_name, token_endpoint_auth_method).
31+
*
32+
* The returned array MUST include at least "client_id" and should include
33+
* "client_secret" when the token endpoint auth method requires one.
34+
*
35+
* @param array<string, mixed> $registrationRequest Client metadata from the registration request body
2336
*
24-
* @return array<string, mixed>
37+
* @return array<string, mixed> Registration response including client_id and optional client_secret
2538
*
26-
* @throws ClientRegistrationException
39+
* @throws ClientRegistrationException If registration fails (e.g. invalid metadata, storage error)
2740
*/
2841
public function register(array $registrationRequest): array;
2942
}

tests/Unit/Server/Transport/Http/Middleware/ClientRegistrationMiddlewareTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Mcp\Tests\Unit\Server\Transport\Http\Middleware;
1313

1414
use Mcp\Exception\ClientRegistrationException;
15+
use Mcp\Exception\InvalidArgumentException;
1516
use Mcp\Server\Transport\Http\Middleware\ClientRegistrationMiddleware;
1617
use Mcp\Server\Transport\Http\OAuth\ClientRegistrarInterface;
1718
use Nyholm\Psr7\Factory\Psr17Factory;
@@ -165,7 +166,7 @@ public function testNonMatchingRoutePassesThrough(): void
165166
#[TestDox('constructor rejects empty localBaseUrl')]
166167
public function testConstructorRejectsEmptyBaseUrl(): void
167168
{
168-
$this->expectException(\InvalidArgumentException::class);
169+
$this->expectException(InvalidArgumentException::class);
169170

170171
new ClientRegistrationMiddleware(
171172
$this->createStub(ClientRegistrarInterface::class),

0 commit comments

Comments
 (0)