fix: honor mcp-oauth's enabled/observability filters, disable second unprotected MCP endpoint
- Default oauthEnabled to WPMedia\MCP\OAuth\Context::is_enabled() instead of duplicating its filter logic, so the deprecated rocket_mcp_oauth_server_enabled filter and the typed wpmedia_mcp_oauth_server_enabled filter are honored exactly as mcp-oauth itself defines them. - register() now disables mcp-adapter's default server (mcp_adapter_create_default_server => false) so no second, unauthenticated MCP endpoint (plain HttpTransport, is_user_logged_in()) is exposed alongside our OAuth-protected one; mcp-oauth's own ServerRegistrar registers the shared meta-abilities itself once the default server is disabled. - Pass WPMedia\MCP\OAuth\Context::observability_handler() instead of null so mcp-oauth's own observability filter keeps working for our server too. 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:
@@ -8,12 +8,16 @@ use LIT\MCP\Guides\InstructionsBuilder;
|
|||||||
use LIT\MCP\Integrations\IntegrationRegistry;
|
use LIT\MCP\Integrations\IntegrationRegistry;
|
||||||
use WP\MCP\Core\McpAdapter;
|
use WP\MCP\Core\McpAdapter;
|
||||||
use WP\MCP\Infrastructure\ErrorHandling\ErrorLogMcpErrorHandler;
|
use WP\MCP\Infrastructure\ErrorHandling\ErrorLogMcpErrorHandler;
|
||||||
|
use WPMedia\MCP\OAuth\Context as OAuthContext;
|
||||||
use WPMedia\MCP\OAuth\Transport\OAuthHttpTransport;
|
use WPMedia\MCP\OAuth\Transport\OAuthHttpTransport;
|
||||||
use WPMedia\MCP\OAuth\Transport\ServerRegistrar as OAuthServerRegistrar;
|
use WPMedia\MCP\OAuth\Transport\ServerRegistrar as OAuthServerRegistrar;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Registers the OAuth-protected MCP server with our own tool list.
|
* Registers the OAuth-protected MCP server with our own tool list.
|
||||||
*
|
*
|
||||||
|
* `register()` disables mcp-adapter's default server (`mcp_adapter_create_default_server`)
|
||||||
|
* so no second, unprotected MCP endpoint is exposed alongside ours.
|
||||||
|
*
|
||||||
* wp-media/mcp-oauth hardcodes server id/route (token audience) and only three
|
* wp-media/mcp-oauth hardcodes server id/route (token audience) and only three
|
||||||
* meta-tools, and registers its own server unconditionally on the same
|
* meta-tools, and registers its own server unconditionally on the same
|
||||||
* `mcp_adapter_init` action (priority 10). We register the same server id
|
* `mcp_adapter_init` action (priority 10). We register the same server id
|
||||||
@@ -43,11 +47,18 @@ final class ServerRegistrar
|
|||||||
?callable $oauthEnabled = null
|
?callable $oauthEnabled = null
|
||||||
) {
|
) {
|
||||||
$this->oauthEnabled = $oauthEnabled
|
$this->oauthEnabled = $oauthEnabled
|
||||||
?? static fn (): bool => (bool) apply_filters('wpmedia_mcp_oauth_server_enabled', true);
|
?? static fn (): bool => (new OAuthContext())->is_enabled();
|
||||||
}
|
}
|
||||||
|
|
||||||
public function register(): void
|
public function register(): void
|
||||||
{
|
{
|
||||||
|
// mcp-adapter's default server is a second, unprotected (plain HttpTransport,
|
||||||
|
// is_user_logged_in()) endpoint exposing execute-ability – and thus our own
|
||||||
|
// abilities. We register our own OAuth-protected server instead; mcp-oauth's
|
||||||
|
// ServerRegistrar registers the shared meta-abilities itself once this filter
|
||||||
|
// disables the default server (see its ensure_shared_abilities_registered()).
|
||||||
|
add_filter('mcp_adapter_create_default_server', '__return_false');
|
||||||
|
|
||||||
add_action('mcp_adapter_init', [$this, 'createServer'], 5);
|
add_action('mcp_adapter_init', [$this, 'createServer'], 5);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -95,7 +106,7 @@ final class ServerRegistrar
|
|||||||
$args['version'],
|
$args['version'],
|
||||||
[OAuthHttpTransport::class],
|
[OAuthHttpTransport::class],
|
||||||
ErrorLogMcpErrorHandler::class,
|
ErrorLogMcpErrorHandler::class,
|
||||||
null,
|
(new OAuthContext())->observability_handler(),
|
||||||
$args['tools']
|
$args['tools']
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
|||||||
|
|
||||||
namespace LIT\MCP\Tests\Unit\Server;
|
namespace LIT\MCP\Tests\Unit\Server;
|
||||||
|
|
||||||
|
use Brain\Monkey\Filters;
|
||||||
use Brain\Monkey\Functions;
|
use Brain\Monkey\Functions;
|
||||||
use LIT\MCP\Abilities\Ability;
|
use LIT\MCP\Abilities\Ability;
|
||||||
use LIT\MCP\Guides\InstructionsBuilder;
|
use LIT\MCP\Guides\InstructionsBuilder;
|
||||||
@@ -85,6 +86,34 @@ final class ServerRegistrarTest extends TestCase
|
|||||||
self::assertFalse($oauthChecked, 'createServer must bail out before touching the adapter');
|
self::assertFalse($oauthChecked, 'createServer must bail out before touching the adapter');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testRegisterDisablesAdapterDefaultServerAndHooksCreateServer(): void
|
||||||
|
{
|
||||||
|
$registrar = new ServerRegistrar($this->registry(), $this->instructions());
|
||||||
|
|
||||||
|
Functions\expect('add_filter')
|
||||||
|
->once()
|
||||||
|
->with('mcp_adapter_create_default_server', '__return_false');
|
||||||
|
Functions\expect('add_action')
|
||||||
|
->once()
|
||||||
|
->with('mcp_adapter_init', [$registrar, 'createServer'], 5);
|
||||||
|
|
||||||
|
$registrar->register();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testDefaultOauthEnabledDelegatesToMcpOauthContext(): void
|
||||||
|
{
|
||||||
|
// LIT\MCP\Server\ServerRegistrar::__construct() defaults to
|
||||||
|
// (new \WPMedia\MCP\OAuth\Context())->is_enabled() – exercise that real
|
||||||
|
// Context, only stubbing the filter it reads, to prove we track its notion
|
||||||
|
// of "enabled" (including the deprecated rocket_mcp_oauth_server_enabled
|
||||||
|
// filter Context::is_enabled() also seeds from) instead of duplicating it.
|
||||||
|
Filters\expectApplied('wpmedia_mcp_oauth_server_enabled')->once()->andReturn(false);
|
||||||
|
|
||||||
|
$registrar = new ServerRegistrar($this->registry(), $this->instructions());
|
||||||
|
|
||||||
|
self::assertNull($registrar->serverArguments());
|
||||||
|
}
|
||||||
|
|
||||||
public function testUnhookOAuthDefaultServerRemovesOnlyTheMcpOauthCallback(): void
|
public function testUnhookOAuthDefaultServerRemovesOnlyTheMcpOauthCallback(): void
|
||||||
{
|
{
|
||||||
$oauthRegistrar = Mockery::mock(OAuthServerRegistrar::class);
|
$oauthRegistrar = Mockery::mock(OAuthServerRegistrar::class);
|
||||||
|
|||||||
Reference in New Issue
Block a user