Skip to content

Commit 5301147

Browse files
simonchrzsimonchrz
committed
refactor: keep StrictOidcDiscoveryMetadataPolicy strict, add LenientOidcDiscoveryMetadataPolicy
- StrictOidcDiscoveryMetadataPolicy remains RFC-aligned: requires code_challenge_methods_supported - LenientOidcDiscoveryMetadataPolicy accepts missing code_challenge_methods_supported for providers like FusionAuth and Microsoft Entra ID that omit it despite supporting PKCE with S256 - If code_challenge_methods_supported is present, both policies validate it strictly - Update OidcDiscoveryTest with tests for both policies - Update Microsoft example README to reference the built-in lenient policy
1 parent 7b351c2 commit 5301147

File tree

5 files changed

+207
-5
lines changed

5 files changed

+207
-5
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
99
* Add client component for building MCP clients
1010
* Add `Builder::setReferenceHandler()` to allow custom `ReferenceHandlerInterface` implementations (e.g. authorization decorators)
1111
* Add elicitation enum schema types per SEP-1330: `TitledEnumSchemaDefinition`, `MultiSelectEnumSchemaDefinition`, `TitledMultiSelectEnumSchemaDefinition`
12-
* Add `LenientOidcDiscoveryMetadataPolicy` for identity providers that omit `code_challenge_methods_supported` in OIDC discovery
12+
* Add `LenientOidcDiscoveryMetadataPolicy` for identity providers that omit `code_challenge_methods_supported` (e.g. FusionAuth, Microsoft Entra ID)
1313
* Add OAuth 2.0 Dynamic Client Registration middleware (RFC 7591)
1414

1515
0.4.0

‎examples/server/oauth-microsoft/README.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,11 @@ Microsoft's JWKS endpoint is public. Ensure your container can reach:
198198

199199
### `code_challenge_methods_supported` missing in discovery metadata
200200

201-
This example configures `OidcDiscovery` with `MicrosoftOidcMetadataPolicy`, so this
202-
field can be missing or malformed and will not fail discovery.
201+
The default `StrictOidcDiscoveryMetadataPolicy` requires `code_challenge_methods_supported`.
202+
Microsoft Entra ID omits this field despite supporting PKCE with S256.
203+
Use the built-in `LenientOidcDiscoveryMetadataPolicy` which accepts missing `code_challenge_methods_supported`
204+
(defaults to S256 downstream). The `MicrosoftOidcMetadataPolicy` in this example demonstrates
205+
how to implement a custom policy via `OidcDiscoveryMetadataPolicyInterface`.
203206

204207
### Graph API errors
205208

‎src/Server/Transport/Http/OAuth/LenientOidcDiscoveryMetadataPolicy.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the official PHP MCP SDK.
5+
*
6+
* A collaboration between Symfony and the PHP Foundation.
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Mcp\Server\Transport\Http\OAuth;
13+
14+
/**
15+
* Lenient metadata policy for identity providers that omit
16+
* code_challenge_methods_supported from their OIDC discovery response
17+
* despite supporting PKCE (e.g. FusionAuth, Microsoft Entra ID).
18+
*
19+
* If code_challenge_methods_supported is present, it is still validated.
20+
* If absent, the downstream OAuthProxyMiddleware defaults to ['S256'].
21+
*
22+
* @author Simon Chrzanowski <simon.chrzanowski@quentic.com>
23+
*/
24+
final class LenientOidcDiscoveryMetadataPolicy implements OidcDiscoveryMetadataPolicyInterface
25+
{
26+
public function isValid(mixed $metadata): bool
27+
{
28+
if (!\is_array($metadata)
29+
|| !isset($metadata['authorization_endpoint'], $metadata['token_endpoint'], $metadata['jwks_uri'])
30+
|| !\is_string($metadata['authorization_endpoint'])
31+
|| '' === trim($metadata['authorization_endpoint'])
32+
|| !\is_string($metadata['token_endpoint'])
33+
|| '' === trim($metadata['token_endpoint'])
34+
|| !\is_string($metadata['jwks_uri'])
35+
|| '' === trim($metadata['jwks_uri'])
36+
) {
37+
return false;
38+
}
39+
40+
if (isset($metadata['code_challenge_methods_supported'])) {
41+
if (!\is_array($metadata['code_challenge_methods_supported']) || [] === $metadata['code_challenge_methods_supported']) {
42+
return false;
43+
}
44+
45+
foreach ($metadata['code_challenge_methods_supported'] as $method) {
46+
if (!\is_string($method) || '' === trim($method)) {
47+
return false;
48+
}
49+
}
50+
}
51+
52+
return true;
53+
}
54+
}

‎tests/Unit/Server/Transport/Http/OAuth/LenientOidcDiscoveryMetadataPolicyTest.php‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the official PHP MCP SDK.
5+
*
6+
* A collaboration between Symfony and the PHP Foundation.
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Mcp\Tests\Unit\Server\Transport\Http\OAuth;
13+
14+
use Mcp\Server\Transport\Http\OAuth\LenientOidcDiscoveryMetadataPolicy;
15+
use PHPUnit\Framework\Attributes\TestDox;
16+
use PHPUnit\Framework\TestCase;
17+
18+
/**
19+
* Tests LenientOidcDiscoveryMetadataPolicy validation behavior.
20+
*
21+
* @author Simon Chrzanowski <simon.chrzanowski@quentic.com>
22+
*/
23+
class LenientOidcDiscoveryMetadataPolicyTest extends TestCase
24+
{
25+
#[TestDox('metadata without code challenge methods is valid (defaults to S256 downstream)')]
26+
public function testMissingCodeChallengeMethodsIsValid(): void
27+
{
28+
$policy = new LenientOidcDiscoveryMetadataPolicy();
29+
$metadata = [
30+
'authorization_endpoint' => 'https://auth.example.com/authorize',
31+
'token_endpoint' => 'https://auth.example.com/token',
32+
'jwks_uri' => 'https://auth.example.com/jwks',
33+
];
34+
35+
$this->assertTrue($policy->isValid($metadata));
36+
}
37+
38+
#[TestDox('valid code challenge methods list is accepted')]
39+
public function testValidCodeChallengeMethodsIsAccepted(): void
40+
{
41+
$policy = new LenientOidcDiscoveryMetadataPolicy();
42+
$metadata = [
43+
'authorization_endpoint' => 'https://auth.example.com/authorize',
44+
'token_endpoint' => 'https://auth.example.com/token',
45+
'jwks_uri' => 'https://auth.example.com/jwks',
46+
'code_challenge_methods_supported' => ['S256'],
47+
];
48+
49+
$this->assertTrue($policy->isValid($metadata));
50+
}
51+
52+
#[TestDox('empty code challenge methods list is invalid')]
53+
public function testEmptyCodeChallengeMethodsIsInvalid(): void
54+
{
55+
$policy = new LenientOidcDiscoveryMetadataPolicy();
56+
$metadata = [
57+
'authorization_endpoint' => 'https://auth.example.com/authorize',
58+
'token_endpoint' => 'https://auth.example.com/token',
59+
'jwks_uri' => 'https://auth.example.com/jwks',
60+
'code_challenge_methods_supported' => [],
61+
];
62+
63+
$this->assertFalse($policy->isValid($metadata));
64+
}
65+
66+
#[TestDox('non string code challenge method is invalid')]
67+
public function testNonStringCodeChallengeMethodIsInvalid(): void
68+
{
69+
$policy = new LenientOidcDiscoveryMetadataPolicy();
70+
$metadata = [
71+
'authorization_endpoint' => 'https://auth.example.com/authorize',
72+
'token_endpoint' => 'https://auth.example.com/token',
73+
'jwks_uri' => 'https://auth.example.com/jwks',
74+
'code_challenge_methods_supported' => ['S256', 123],
75+
];
76+
77+
$this->assertFalse($policy->isValid($metadata));
78+
}
79+
80+
#[TestDox('missing required fields is invalid')]
81+
public function testMissingRequiredFieldsIsInvalid(): void
82+
{
83+
$policy = new LenientOidcDiscoveryMetadataPolicy();
84+
85+
$this->assertFalse($policy->isValid([
86+
'authorization_endpoint' => 'https://auth.example.com/authorize',
87+
'token_endpoint' => 'https://auth.example.com/token',
88+
// missing jwks_uri
89+
]));
90+
}
91+
92+
#[TestDox('empty string endpoint is invalid')]
93+
public function testEmptyStringEndpointIsInvalid(): void
94+
{
95+
$policy = new LenientOidcDiscoveryMetadataPolicy();
96+
97+
$this->assertFalse($policy->isValid([
98+
'authorization_endpoint' => '',
99+
'token_endpoint' => 'https://auth.example.com/token',
100+
'jwks_uri' => 'https://auth.example.com/jwks',
101+
]));
102+
}
103+
104+
#[TestDox('non-array input is invalid')]
105+
public function testNonArrayInputIsInvalid(): void
106+
{
107+
$policy = new LenientOidcDiscoveryMetadataPolicy();
108+
109+
$this->assertFalse($policy->isValid('not an array'));
110+
}
111+
}

‎tests/Unit/Server/Transport/Http/OAuth/OidcDiscoveryTest.php‎

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

1414
use Mcp\Exception\RuntimeException;
15+
use Mcp\Server\Transport\Http\OAuth\LenientOidcDiscoveryMetadataPolicy;
1516
use Mcp\Server\Transport\Http\OAuth\OidcDiscovery;
1617
use Nyholm\Psr7\Factory\Psr17Factory;
1718
use PHPUnit\Framework\Attributes\TestDox;
@@ -51,7 +52,7 @@ public function testDiscoverRejectsMetadataWithoutCodeChallengeMethodsSupported(
5152

5253
$factory = new Psr17Factory();
5354
$issuer = 'https://auth.example.com';
54-
$metadataWithoutCodeChallengeMethods = [
55+
$metadata = [
5556
'issuer' => $issuer,
5657
'authorization_endpoint' => 'https://auth.example.com/oauth2/v2.0/authorize',
5758
'token_endpoint' => 'https://auth.example.com/oauth2/v2.0/token',
@@ -62,7 +63,7 @@ public function testDiscoverRejectsMetadataWithoutCodeChallengeMethodsSupported(
6263
$httpClient->expects($this->exactly(2))
6364
->method('sendRequest')
6465
->willReturn($factory->createResponse(200)->withBody(
65-
$factory->createStream(json_encode($metadataWithoutCodeChallengeMethods, \JSON_THROW_ON_ERROR)),
66+
$factory->createStream(json_encode($metadata, \JSON_THROW_ON_ERROR)),
6667
));
6768

6869
$discovery = new OidcDiscovery(
@@ -75,6 +76,39 @@ public function testDiscoverRejectsMetadataWithoutCodeChallengeMethodsSupported(
7576
$discovery->discover($issuer);
7677
}
7778

79+
#[TestDox('lenient discovery accepts metadata without code challenge methods')]
80+
public function testDiscoverAcceptsMetadataWithoutCodeChallengeMethodsUsingLenientPolicy(): void
81+
{
82+
$this->skipIfPsrHttpClientIsMissing();
83+
84+
$factory = new Psr17Factory();
85+
$issuer = 'https://auth.example.com';
86+
$metadata = [
87+
'issuer' => $issuer,
88+
'authorization_endpoint' => 'https://auth.example.com/oauth2/v2.0/authorize',
89+
'token_endpoint' => 'https://auth.example.com/oauth2/v2.0/token',
90+
'jwks_uri' => 'https://auth.example.com/discovery/v2.0/keys',
91+
];
92+
93+
$httpClient = $this->createMock(ClientInterface::class);
94+
$httpClient->expects($this->once())
95+
->method('sendRequest')
96+
->willReturn($factory->createResponse(200)->withBody(
97+
$factory->createStream(json_encode($metadata, \JSON_THROW_ON_ERROR)),
98+
));
99+
100+
$discovery = new OidcDiscovery(
101+
httpClient: $httpClient,
102+
requestFactory: $factory,
103+
metadataPolicy: new LenientOidcDiscoveryMetadataPolicy(),
104+
);
105+
106+
$result = $discovery->discover($issuer);
107+
108+
$this->assertSame($metadata['authorization_endpoint'], $result['authorization_endpoint']);
109+
$this->assertArrayNotHasKey('code_challenge_methods_supported', $result);
110+
}
111+
78112
#[TestDox('discover falls back to the next metadata URL when first response is invalid')]
79113
public function testDiscoverFallsBackOnInvalidMetadataResponse(): void
80114
{

0 commit comments

Comments
 (0)