fix: discovery admin notice fires on cached 'recommended' too, guarded against oauth/permalink early returns
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:
@@ -30,8 +30,24 @@ final class AdminNotices
|
|||||||
) . ' ' . __('Claude kann sich nicht verbinden.', 'wp-lit-mcp');
|
) . ' ' . __('Claude kann sich nicht verbinden.', 'wp-lit-mcp');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Fire whenever the cached discovery result is not 'good' (this also covers
|
||||||
|
* 'recommended', which is how HealthCheck::run_self_check() itself reports both
|
||||||
|
* a genuine reachability problem and its acme-challenge-fingerprint "critical"
|
||||||
|
* case) — but only when oauth is enabled and permalinks are pretty. Those two
|
||||||
|
* conditions are exactly when run_self_check() short-circuits to its own
|
||||||
|
* disabled/permalink-missing early returns instead of a real check; guarding on
|
||||||
|
* them here (rather than trying to recognise their shape from the cached array)
|
||||||
|
* keeps this correct even though, in the vendor's current implementation,
|
||||||
|
* neither early return is ever persisted to the transient we read.
|
||||||
|
*/
|
||||||
$discovery = $this->status->discovery(false);
|
$discovery = $this->status->discovery(false);
|
||||||
if (is_array($discovery) && ($discovery['status'] ?? 'good') === 'critical') {
|
if (
|
||||||
|
$this->status->oauthEnabled()
|
||||||
|
&& $this->status->permalinksPretty()
|
||||||
|
&& is_array($discovery)
|
||||||
|
&& ($discovery['status'] ?? 'good') !== 'good'
|
||||||
|
) {
|
||||||
$messages[] = __(
|
$messages[] = __(
|
||||||
'LIT MCP: Die OAuth-Discovery-Dokumente unter /.well-known/ sind nicht erreichbar.',
|
'LIT MCP: Die OAuth-Discovery-Dokumente unter /.well-known/ sind nicht erreichbar.',
|
||||||
'wp-lit-mcp'
|
'wp-lit-mcp'
|
||||||
|
|||||||
@@ -46,6 +46,17 @@ class ConnectionStatus
|
|||||||
return str_starts_with((string) home_url(), 'https://');
|
return str_starts_with((string) home_url(), 'https://');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether a pretty permalink structure is active. HealthCheck::run_self_check()
|
||||||
|
* short-circuits to a 'recommended' result (never persisted to its transient)
|
||||||
|
* when this is false; used to tell that early return apart from a genuine
|
||||||
|
* discovery failure without depending on that vendor implementation detail.
|
||||||
|
*/
|
||||||
|
public function permalinksPretty(): bool
|
||||||
|
{
|
||||||
|
return (string) get_option('permalink_structure') !== '';
|
||||||
|
}
|
||||||
|
|
||||||
/** @return array<string, mixed>|null mcp-oauth's discovery self check (label, status, …). */
|
/** @return array<string, mixed>|null mcp-oauth's discovery self check (label, status, …). */
|
||||||
public function discovery(bool $allowFetch): ?array
|
public function discovery(bool $allowFetch): ?array
|
||||||
{
|
{
|
||||||
@@ -101,28 +112,36 @@ class ConnectionStatus
|
|||||||
$endpoint = $this->endpoint($allowFetch);
|
$endpoint = $this->endpoint($allowFetch);
|
||||||
|
|
||||||
return [
|
return [
|
||||||
['label' => 'MCP-Adapter', 'ok' => $adapter, 'message' => $adapter ? 'geladen' : 'fehlt'],
|
|
||||||
[
|
[
|
||||||
'label' => 'OAuth-Server',
|
'label' => __('MCP-Adapter', 'wp-lit-mcp'),
|
||||||
|
'ok' => $adapter,
|
||||||
|
'message' => $adapter ? __('geladen', 'wp-lit-mcp') : __('fehlt', 'wp-lit-mcp'),
|
||||||
|
],
|
||||||
|
[
|
||||||
|
'label' => __('OAuth-Server', 'wp-lit-mcp'),
|
||||||
'ok' => $oauth,
|
'ok' => $oauth,
|
||||||
'message' => $oauth ? 'aktiv' : 'deaktiviert (Filter wpmedia_mcp_oauth_server_enabled)',
|
'message' => $oauth
|
||||||
|
? __('aktiv', 'wp-lit-mcp')
|
||||||
|
: __('deaktiviert (Filter wpmedia_mcp_oauth_server_enabled)', 'wp-lit-mcp'),
|
||||||
],
|
],
|
||||||
[
|
[
|
||||||
'label' => 'HTTPS',
|
'label' => __('HTTPS', 'wp-lit-mcp'),
|
||||||
'ok' => $https,
|
'ok' => $https,
|
||||||
'message' => $https ? 'aktiv' : 'Website läuft ohne HTTPS – claude.ai verbindet sich nur über HTTPS',
|
'message' => $https
|
||||||
|
? __('aktiv', 'wp-lit-mcp')
|
||||||
|
: __('Website läuft ohne HTTPS – claude.ai verbindet sich nur über HTTPS', 'wp-lit-mcp'),
|
||||||
],
|
],
|
||||||
[
|
[
|
||||||
'label' => 'Discovery (/.well-known/)',
|
'label' => __('Discovery (/.well-known/)', 'wp-lit-mcp'),
|
||||||
'ok' => $discovery === null ? null : ($discovery['status'] ?? '') === 'good',
|
'ok' => $discovery === null ? null : ($discovery['status'] ?? '') === 'good',
|
||||||
'message' => $discovery === null
|
'message' => $discovery === null
|
||||||
? 'noch nicht geprüft'
|
? __('noch nicht geprüft', 'wp-lit-mcp')
|
||||||
: wp_strip_all_tags((string) ($discovery['label'] ?? '')),
|
: wp_strip_all_tags((string) ($discovery['label'] ?? '')),
|
||||||
],
|
],
|
||||||
[
|
[
|
||||||
'label' => 'Endpunkt ohne Token',
|
'label' => __('Endpunkt ohne Token', 'wp-lit-mcp'),
|
||||||
'ok' => $endpoint['ok'] ?? null,
|
'ok' => $endpoint['ok'] ?? null,
|
||||||
'message' => $endpoint['message'] ?? 'noch nicht geprüft',
|
'message' => $endpoint['message'] ?? __('noch nicht geprüft', 'wp-lit-mcp'),
|
||||||
],
|
],
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,23 +18,84 @@ final class AdminNoticesTest extends TestCase
|
|||||||
Functions\stubTranslationFunctions();
|
Functions\stubTranslationFunctions();
|
||||||
}
|
}
|
||||||
|
|
||||||
private function status(bool $oauth, ?array $discovery, bool $https): ConnectionStatus
|
/**
|
||||||
|
* The array shape WPMedia\MCP\OAuth\Auth\Discovery\HealthCheck::build_result()
|
||||||
|
* stores in its transient for a genuine (not early-return) check — see
|
||||||
|
* vendor/wp-media/mcp-oauth/inc/Auth/Discovery/HealthCheck.php. Its 'critical'
|
||||||
|
* internal classification (the acme-challenge fingerprint) is itself reported
|
||||||
|
* here as 'recommended' (never 'critical') by that class's own downgrade.
|
||||||
|
*
|
||||||
|
* @return array<string, mixed>
|
||||||
|
*/
|
||||||
|
private function realDiscoveryResult(string $status): array
|
||||||
|
{
|
||||||
|
return [
|
||||||
|
'label' => 'MCP OAuth discovery documents',
|
||||||
|
'status' => $status,
|
||||||
|
'badge' => ['label' => 'Configuration', 'color' => 'blue'],
|
||||||
|
'description' => '<p>The following .well-known discovery document(s) did not respond as expected: '
|
||||||
|
. 'oauth-protected-resource.</p>',
|
||||||
|
'actions' => '<p>Check your error/access logs for the requests to these paths…</p>',
|
||||||
|
'test' => 'wpmedia_mcp_oauth_wellknown_discovery',
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
private function status(bool $oauth, ?array $discovery, bool $https, bool $permalinksPretty = true): ConnectionStatus
|
||||||
{
|
{
|
||||||
$status = Mockery::mock(ConnectionStatus::class);
|
$status = Mockery::mock(ConnectionStatus::class);
|
||||||
$status->allows('oauthEnabled')->andReturn($oauth);
|
$status->allows('oauthEnabled')->andReturn($oauth);
|
||||||
$status->allows('discovery')->with(false)->andReturn($discovery);
|
$status->allows('discovery')->with(false)->andReturn($discovery);
|
||||||
$status->allows('https')->andReturn($https);
|
$status->allows('https')->andReturn($https);
|
||||||
|
$status->allows('permalinksPretty')->andReturn($permalinksPretty);
|
||||||
|
|
||||||
return $status;
|
return $status;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testWarnsAboutDisabledOAuthAndBrokenDiscoveryEverywhere(): void
|
public function testWarnsAboutDisabledOAuth(): void
|
||||||
{
|
{
|
||||||
$messages = (new AdminNotices($this->status(false, ['status' => 'critical'], true)))->messages(false);
|
$messages = (new AdminNotices($this->status(false, null, true)))->messages(false);
|
||||||
|
|
||||||
self::assertCount(2, $messages);
|
self::assertCount(1, $messages);
|
||||||
self::assertStringContainsString('OAuth-Server ist deaktiviert', $messages[0]);
|
self::assertStringContainsString('OAuth-Server ist deaktiviert', $messages[0]);
|
||||||
self::assertStringContainsString('/.well-known/', $messages[1]);
|
}
|
||||||
|
|
||||||
|
public function testWarnsWhenTheCachedDiscoveryStatusIsCriticalOrRecommended(): void
|
||||||
|
{
|
||||||
|
foreach (['critical', 'recommended'] as $status) {
|
||||||
|
$messages = (new AdminNotices($this->status(true, $this->realDiscoveryResult($status), true)))
|
||||||
|
->messages(false);
|
||||||
|
|
||||||
|
self::assertCount(1, $messages, "status={$status}");
|
||||||
|
self::assertStringContainsString('/.well-known/', $messages[0]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testGoodCachedDiscoveryNeverWarns(): void
|
||||||
|
{
|
||||||
|
$notices = new AdminNotices($this->status(true, $this->realDiscoveryResult('good'), true));
|
||||||
|
|
||||||
|
self::assertSame([], $notices->messages(false));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testDoesNotAddASeparateDiscoveryWarningWhenOAuthIsDisabled(): void
|
||||||
|
{
|
||||||
|
// HealthCheck::run_self_check()'s 'disabled' early return always reports status
|
||||||
|
// 'good' and (like the permalink one below) is never persisted to its transient —
|
||||||
|
// but we guard on the live oauthEnabled() state too rather than relying on that
|
||||||
|
// vendor implementation detail. A stale 'critical' result cached from before OAuth
|
||||||
|
// was disabled must not add a second, redundant notice.
|
||||||
|
$notices = new AdminNotices($this->status(false, $this->realDiscoveryResult('critical'), true));
|
||||||
|
|
||||||
|
$messages = $notices->messages(false);
|
||||||
|
self::assertCount(1, $messages);
|
||||||
|
self::assertStringContainsString('OAuth-Server ist deaktiviert', $messages[0]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testDoesNotWarnAboutDiscoveryWithoutPrettyPermalinks(): void
|
||||||
|
{
|
||||||
|
$notices = new AdminNotices($this->status(true, $this->realDiscoveryResult('recommended'), true, false));
|
||||||
|
|
||||||
|
self::assertSame([], $notices->messages(false));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testHttpsHintOnlyOnTheSettingsPage(): void
|
public function testHttpsHintOnlyOnTheSettingsPage(): void
|
||||||
|
|||||||
@@ -11,6 +11,12 @@ use Mockery;
|
|||||||
|
|
||||||
final class ConnectionStatusTest extends TestCase
|
final class ConnectionStatusTest extends TestCase
|
||||||
{
|
{
|
||||||
|
protected function setUp(): void
|
||||||
|
{
|
||||||
|
parent::setUp();
|
||||||
|
Functions\stubTranslationFunctions();
|
||||||
|
}
|
||||||
|
|
||||||
public function testEndpointCheckExpects401WithBearerChallengeAndCachesIt(): void
|
public function testEndpointCheckExpects401WithBearerChallengeAndCachesIt(): void
|
||||||
{
|
{
|
||||||
Functions\when('get_transient')->justReturn(false);
|
Functions\when('get_transient')->justReturn(false);
|
||||||
@@ -56,4 +62,13 @@ final class ConnectionStatusTest extends TestCase
|
|||||||
self::assertNull($byLabel['Discovery (/.well-known/)']['ok']);
|
self::assertNull($byLabel['Discovery (/.well-known/)']['ok']);
|
||||||
self::assertNull($byLabel['Endpunkt ohne Token']['ok']);
|
self::assertNull($byLabel['Endpunkt ohne Token']['ok']);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testPermalinksPrettyReflectsThePermalinkStructureOption(): void
|
||||||
|
{
|
||||||
|
Functions\when('get_option')->justReturn('/%postname%/');
|
||||||
|
self::assertTrue((new ConnectionStatus())->permalinksPretty());
|
||||||
|
|
||||||
|
Functions\when('get_option')->justReturn('');
|
||||||
|
self::assertFalse((new ConnectionStatus())->permalinksPretty());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user