fix: reset request context after the MCP request, resolve client name
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
This commit is contained in:
@@ -4,28 +4,32 @@ declare(strict_types=1);
|
|||||||
|
|
||||||
namespace LIT\MCP\Server;
|
namespace LIT\MCP\Server;
|
||||||
|
|
||||||
|
use Throwable;
|
||||||
use WP_REST_Request;
|
use WP_REST_Request;
|
||||||
|
use WPMedia\MCP\OAuth\Auth\JWT;
|
||||||
|
use WPMedia\MCP\OAuth\Auth\SecretManager;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Tracks whether the current REST dispatch is serving our OAuth-protected MCP
|
* Tracks whether the current REST dispatch is serving our OAuth-protected MCP
|
||||||
* server route (`/mcp/mcp-oauth-server`).
|
* server route (`/mcp/mcp-oauth-server`) and which OAuth client made it.
|
||||||
*
|
*
|
||||||
* Abilities must never run outside that route – not through mcp-adapter's
|
* Abilities must never run outside that route. WordPress fires
|
||||||
* (disabled) default server, not through a differently configured server, and
|
|
||||||
* not through any unrelated REST request. WordPress fires
|
|
||||||
* `rest_request_before_callbacks` for every matched REST route before any
|
* `rest_request_before_callbacks` for every matched REST route before any
|
||||||
* permission_callback runs (see WP_REST_Server::dispatch()), so by the time an
|
* permission_callback runs and `rest_request_after_callbacks` right after the
|
||||||
* ability's permission_callback executes – itself only reachable from inside
|
* route callback returned (WP_REST_Server::respond_to_request()). The flag is
|
||||||
* the MCP route's own callback – the flag below is already accurate for the
|
* set in the first and cleared in the second, so persistent PHP workers or a
|
||||||
* current request.
|
* later `rest_do_request()` in the same process never inherit it.
|
||||||
*/
|
*/
|
||||||
final class RequestContext
|
final class RequestContext
|
||||||
{
|
{
|
||||||
private static bool $isOAuthServerRequest = false;
|
private static bool $isOAuthServerRequest = false;
|
||||||
|
private static ?string $bearerToken = null;
|
||||||
|
private static ?string $clientName = null;
|
||||||
|
|
||||||
public function register(): void
|
public function register(): void
|
||||||
{
|
{
|
||||||
add_filter('rest_request_before_callbacks', [self::class, 'recordRoute'], 10, 3);
|
add_filter('rest_request_before_callbacks', [self::class, 'recordRoute'], 10, 3);
|
||||||
|
add_filter('rest_request_after_callbacks', [self::class, 'clearRoute'], 10, 3);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -37,6 +41,25 @@ final class RequestContext
|
|||||||
{
|
{
|
||||||
if ($request->get_route() === self::route()) {
|
if ($request->get_route() === self::route()) {
|
||||||
self::$isOAuthServerRequest = true;
|
self::$isOAuthServerRequest = true;
|
||||||
|
self::$clientName = null;
|
||||||
|
$header = $request->get_header('authorization');
|
||||||
|
self::$bearerToken = is_string($header) && str_starts_with($header, 'Bearer ')
|
||||||
|
? substr($header, 7)
|
||||||
|
: null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $response;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @param mixed $response
|
||||||
|
* @param array<string, mixed> $handler
|
||||||
|
* @return mixed
|
||||||
|
*/
|
||||||
|
public static function clearRoute(mixed $response, array $handler, WP_REST_Request $request): mixed
|
||||||
|
{
|
||||||
|
if ($request->get_route() === self::route()) {
|
||||||
|
self::reset();
|
||||||
}
|
}
|
||||||
|
|
||||||
return $response;
|
return $response;
|
||||||
@@ -48,11 +71,49 @@ final class RequestContext
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test-only: resets (or forces) the recorded state between unit tests.
|
* Name of the connected OAuth client (the Application Password mcp-oauth created
|
||||||
|
* for it), falling back to the token's client_id; '' when unknown.
|
||||||
*/
|
*/
|
||||||
public static function reset(bool $isOAuthServerRequest = false): void
|
public static function clientName(): string
|
||||||
|
{
|
||||||
|
if (self::$clientName === null) {
|
||||||
|
self::$clientName = self::$bearerToken === null ? '' : self::resolveClientName(self::$bearerToken);
|
||||||
|
}
|
||||||
|
|
||||||
|
return self::$clientName;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resets (or forces) the recorded state – after each request and between unit tests.
|
||||||
|
*/
|
||||||
|
public static function reset(bool $isOAuthServerRequest = false, ?string $clientName = null): void
|
||||||
{
|
{
|
||||||
self::$isOAuthServerRequest = $isOAuthServerRequest;
|
self::$isOAuthServerRequest = $isOAuthServerRequest;
|
||||||
|
self::$bearerToken = null;
|
||||||
|
self::$clientName = $clientName;
|
||||||
|
}
|
||||||
|
|
||||||
|
private static function resolveClientName(string $token): string
|
||||||
|
{
|
||||||
|
try {
|
||||||
|
$claims = JWT::decode($token, SecretManager::get_secret());
|
||||||
|
if (! is_array($claims)) {
|
||||||
|
return '';
|
||||||
|
}
|
||||||
|
|
||||||
|
$userId = (int) ($claims['sub'] ?? 0);
|
||||||
|
$uuid = (string) ($claims['app_pass_id'] ?? '');
|
||||||
|
if ($userId > 0 && $uuid !== '' && class_exists('WP_Application_Passwords')) {
|
||||||
|
$item = \WP_Application_Passwords::get_user_application_password($userId, $uuid);
|
||||||
|
if (is_array($item) && is_string($item['name'] ?? null) && $item['name'] !== '') {
|
||||||
|
return $item['name'];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return (string) ($claims['client_id'] ?? '');
|
||||||
|
} catch (Throwable) {
|
||||||
|
return '';
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static function route(): string
|
private static function route(): string
|
||||||
|
|||||||
@@ -5,7 +5,8 @@ declare(strict_types=1);
|
|||||||
if (! class_exists('WP_REST_Request')) {
|
if (! class_exists('WP_REST_Request')) {
|
||||||
class WP_REST_Request
|
class WP_REST_Request
|
||||||
{
|
{
|
||||||
public function __construct(private string $route = '')
|
/** @param array<string, string> $headers Lower-case header name => value. */
|
||||||
|
public function __construct(private string $route = '', private array $headers = [])
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -13,5 +14,10 @@ if (! class_exists('WP_REST_Request')) {
|
|||||||
{
|
{
|
||||||
return $this->route;
|
return $this->route;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function get_header(string $key): ?string
|
||||||
|
{
|
||||||
|
return $this->headers[strtolower($key)] ?? null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,10 +7,14 @@ namespace LIT\MCP\Tests\Unit\Server;
|
|||||||
use Brain\Monkey\Functions;
|
use Brain\Monkey\Functions;
|
||||||
use LIT\MCP\Server\RequestContext;
|
use LIT\MCP\Server\RequestContext;
|
||||||
use LIT\MCP\Tests\Unit\TestCase;
|
use LIT\MCP\Tests\Unit\TestCase;
|
||||||
|
use stdClass;
|
||||||
use WP_REST_Request;
|
use WP_REST_Request;
|
||||||
|
use WPMedia\MCP\OAuth\Auth\JWT;
|
||||||
|
|
||||||
final class RequestContextTest extends TestCase
|
final class RequestContextTest extends TestCase
|
||||||
{
|
{
|
||||||
|
private const ROUTE = '/mcp/mcp-oauth-server';
|
||||||
|
|
||||||
protected function tearDown(): void
|
protected function tearDown(): void
|
||||||
{
|
{
|
||||||
RequestContext::reset();
|
RequestContext::reset();
|
||||||
@@ -24,37 +28,90 @@ final class RequestContextTest extends TestCase
|
|||||||
|
|
||||||
public function testRecordRouteMarksTheOAuthServerRoute(): void
|
public function testRecordRouteMarksTheOAuthServerRoute(): void
|
||||||
{
|
{
|
||||||
$request = new WP_REST_Request('/mcp/mcp-oauth-server');
|
RequestContext::recordRoute(null, [], new WP_REST_Request(self::ROUTE));
|
||||||
|
|
||||||
RequestContext::recordRoute(null, [], $request);
|
|
||||||
|
|
||||||
self::assertTrue(RequestContext::isOAuthServerRequest());
|
self::assertTrue(RequestContext::isOAuthServerRequest());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testRecordRouteIgnoresOtherRoutes(): void
|
public function testRecordRouteIgnoresOtherRoutes(): void
|
||||||
{
|
{
|
||||||
$request = new WP_REST_Request('/wp/v2/users/me');
|
RequestContext::recordRoute(null, [], new WP_REST_Request('/wp/v2/users/me'));
|
||||||
|
|
||||||
RequestContext::recordRoute(null, [], $request);
|
|
||||||
|
|
||||||
self::assertFalse(RequestContext::isOAuthServerRequest());
|
self::assertFalse(RequestContext::isOAuthServerRequest());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testRecordRouteReturnsResponseUnchanged(): void
|
public function testRecordRouteReturnsResponseUnchanged(): void
|
||||||
{
|
{
|
||||||
$response = new \stdClass();
|
$response = new stdClass();
|
||||||
$request = new WP_REST_Request('/mcp/mcp-oauth-server');
|
|
||||||
|
|
||||||
$result = RequestContext::recordRoute($response, [], $request);
|
self::assertSame($response, RequestContext::recordRoute($response, [], new WP_REST_Request(self::ROUTE)));
|
||||||
|
|
||||||
self::assertSame($response, $result);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testRegisterHooksRecordRouteOnRestRequestBeforeCallbacks(): void
|
public function testClearRouteResetsTheFlagAfterOurRequest(): void
|
||||||
|
{
|
||||||
|
$request = new WP_REST_Request(self::ROUTE);
|
||||||
|
RequestContext::recordRoute(null, [], $request);
|
||||||
|
$response = new stdClass();
|
||||||
|
|
||||||
|
$result = RequestContext::clearRoute($response, [], $request);
|
||||||
|
|
||||||
|
self::assertSame($response, $result);
|
||||||
|
self::assertFalse(RequestContext::isOAuthServerRequest());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testClearRouteIgnoresNestedRequestsToOtherRoutes(): void
|
||||||
|
{
|
||||||
|
RequestContext::recordRoute(null, [], new WP_REST_Request(self::ROUTE));
|
||||||
|
|
||||||
|
RequestContext::clearRoute(null, [], new WP_REST_Request('/wp/v2/posts'));
|
||||||
|
|
||||||
|
self::assertTrue(RequestContext::isOAuthServerRequest());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testClientNameFallsBackToTheClientIdClaim(): void
|
||||||
|
{
|
||||||
|
Functions\when('get_option')->justReturn('test-secret');
|
||||||
|
$token = JWT::encode([
|
||||||
|
'sub' => '3',
|
||||||
|
'app_pass_id' => 'uuid-unknown',
|
||||||
|
'client_id' => 'https://claude.ai/oauth/mcp-oauth-client-metadata',
|
||||||
|
'exp' => time() + 60,
|
||||||
|
], 'test-secret');
|
||||||
|
|
||||||
|
RequestContext::recordRoute(
|
||||||
|
null,
|
||||||
|
[],
|
||||||
|
new WP_REST_Request(self::ROUTE, ['authorization' => 'Bearer ' . $token])
|
||||||
|
);
|
||||||
|
|
||||||
|
self::assertSame('https://claude.ai/oauth/mcp-oauth-client-metadata', RequestContext::clientName());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testClientNameIsEmptyForAnInvalidToken(): void
|
||||||
|
{
|
||||||
|
Functions\when('get_option')->justReturn('test-secret');
|
||||||
|
|
||||||
|
RequestContext::recordRoute(null, [], new WP_REST_Request(self::ROUTE, ['authorization' => 'Bearer garbage']));
|
||||||
|
|
||||||
|
self::assertSame('', RequestContext::clientName());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testResetCanForceAClientName(): void
|
||||||
|
{
|
||||||
|
RequestContext::reset(true, 'Claude');
|
||||||
|
|
||||||
|
self::assertTrue(RequestContext::isOAuthServerRequest());
|
||||||
|
self::assertSame('Claude', RequestContext::clientName());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRegisterHooksBeforeAndAfterCallbacks(): void
|
||||||
{
|
{
|
||||||
Functions\expect('add_filter')
|
Functions\expect('add_filter')
|
||||||
->once()
|
->once()
|
||||||
->with('rest_request_before_callbacks', [RequestContext::class, 'recordRoute'], 10, 3);
|
->with('rest_request_before_callbacks', [RequestContext::class, 'recordRoute'], 10, 3);
|
||||||
|
Functions\expect('add_filter')
|
||||||
|
->once()
|
||||||
|
->with('rest_request_after_callbacks', [RequestContext::class, 'clearRoute'], 10, 3);
|
||||||
|
|
||||||
(new RequestContext())->register();
|
(new RequestContext())->register();
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user