Merge pull request #9 from magebitcom/bugfix/prevent-store-redirect-well-known

KristofersOzolinsMagebit · web-flow · commit 96cb93d18cdb · 2026-05-27T15:25:22.000+03:00
fix(oauth): prevent store-code redirect for .well-known paths
diff --git a/Controller/Index/Index.php b/Controller/Index/Index.php
@@ -86,7 +86,7 @@ public function execute(): ResponseInterface
             try {
                 $context = $this->authenticator->authenticate($this->header('Authorization'));
             } catch (UnauthorizedException $e) {
-                $metadataUrl = $this->urlBuilder->buildUrl('/.well-known/oauth-protected-resource');
+                $metadataUrl = $this->urlBuilder->getProtectedResourceWellKnownUrl();
                 $this->response->setHeader(
                     'WWW-Authenticate',
                     sprintf('Bearer realm="Magento MCP", resource_metadata="%s"', $metadataUrl),
diff --git a/Controller/OAuth/AuthorizationServerMetadata.php b/Controller/OAuth/AuthorizationServerMetadata.php
@@ -58,8 +58,8 @@ public function execute(): ResponseInterface
 
         $payload = [
             'issuer' => $issuer,
-            'authorization_endpoint' => $issuer . '/mcp/oauth/authorize',
-            'token_endpoint' => $issuer . '/mcp/oauth/token',
+            'authorization_endpoint' => $this->urlBuilder->buildUrl('/mcp/oauth/authorize'),
+            'token_endpoint' => $this->urlBuilder->buildUrl('/mcp/oauth/token'),
             'response_types_supported' => ['code'],
             'grant_types_supported' => ['authorization_code', 'refresh_token'],
             'code_challenge_methods_supported' => ['S256'],
@@ -69,7 +69,7 @@ public function execute(): ResponseInterface
             // S256 is the only advertised challenge method.
             'pkce_required' => true,
             'require_pushed_authorization_requests' => false,
-            'service_documentation' => $issuer . '/mcp',
+            'service_documentation' => $this->urlBuilder->getResourceUrl(),
         ];
 
         $body = json_encode($payload, JSON_UNESCAPED_SLASHES);
diff --git a/Controller/OAuth/ProtectedResourceMetadata.php b/Controller/OAuth/ProtectedResourceMetadata.php
@@ -54,12 +54,11 @@ public function execute(): ResponseInterface
             return $this->corsResponder->emitPreflight($this->response, self::ALLOWED_METHODS);
         }
 
-        $baseUrl = $this->urlBuilder->getBaseUrl();
-        $resourceUrl = $baseUrl . '/mcp';
+        $resourceUrl = $this->urlBuilder->getResourceUrl();
 
         $payload = [
             'resource' => $resourceUrl,
-            'authorization_servers' => [$baseUrl],
+            'authorization_servers' => [$this->urlBuilder->getBaseUrl()],
             'bearer_methods_supported' => ['header'],
             'scopes_supported' => Scope::allValues(),
             'resource_documentation' => $resourceUrl,
diff --git a/Controller/Router/WellKnownRouter.php b/Controller/Router/WellKnownRouter.php
@@ -27,6 +27,7 @@
 class WellKnownRouter implements RouterInterface
 {
     private const PATH_AUTHORIZATION_SERVER = '.well-known/oauth-authorization-server';
+    private const PATH_AUTHORIZATION_SERVER_PREFIX = '.well-known/oauth-authorization-server/';
     private const PATH_PROTECTED_RESOURCE = '.well-known/oauth-protected-resource';
     private const PATH_PROTECTED_RESOURCE_PREFIX = '.well-known/oauth-protected-resource/';
 
@@ -85,7 +86,9 @@ public function match(RequestInterface $request): ?ActionInterface
      */
     private function resolveAction(string $identifier): ?string
     {
-        if ($identifier === self::PATH_AUTHORIZATION_SERVER) {
+        if ($identifier === self::PATH_AUTHORIZATION_SERVER
+            || str_starts_with($identifier, self::PATH_AUTHORIZATION_SERVER_PREFIX)
+        ) {
             return self::ACTION_AUTHORIZATION_SERVER;
         }
         if ($identifier === self::PATH_PROTECTED_RESOURCE
diff --git a/Model/Url/PublicUrlBuilder.php b/Model/Url/PublicUrlBuilder.php
@@ -44,4 +44,87 @@ public function buildUrl(string $path): string
         $path = '/' . ltrim($path, '/');
         return $this->getBaseUrl() . $path;
     }
+
+    /**
+     * Scheme + host + optional port of the base URL, with no path component.
+     * RFC 8414 §3 / RFC 9728 §3.1 insert `.well-known` directly after this.
+     *
+     * @return string
+     */
+    public function getAuthority(): string
+    {
+        $base = $this->getBaseUrl();
+        $parts = parse_url($base);
+        if (!is_array($parts) || !isset($parts['scheme'], $parts['host'])) {
+            return $base;
+        }
+
+        $authority = $parts['scheme'] . '://' . $parts['host'];
+        if (isset($parts['port'])) {
+            $authority .= ':' . $parts['port'];
+        }
+
+        return $authority;
+    }
+
+    /**
+     * Path component of the base URL, trimmed of slashes. Empty for a root
+     * install, e.g. `lv` for https://example.com/lv/.
+     *
+     * @return string
+     */
+    public function getBasePath(): string
+    {
+        $parts = parse_url($this->getBaseUrl());
+        $path = is_array($parts) && isset($parts['path']) ? $parts['path'] : '';
+
+        return trim($path, '/');
+    }
+
+    /**
+     * The RFC 9728 resource identifier — what the MCP client targets.
+     *
+     * @return string
+     */
+    public function getResourceUrl(): string
+    {
+        return $this->buildUrl('/mcp');
+    }
+
+    /**
+     * Build a spec-compliant well-known URL by inserting `.well-known/<name>`
+     * between the authority and the resource/issuer path.
+     *
+     * @param string $name
+     * @param string $resourcePath
+     * @return string
+     */
+    public function getWellKnownUrl(string $name, string $resourcePath = ''): string
+    {
+        $url = $this->getAuthority() . '/.well-known/' . trim($name, '/');
+        $suffix = trim($resourcePath, '/');
+        if ($suffix !== '') {
+            $url .= '/' . $suffix;
+        }
+
+        return $url;
+    }
+
+    /**
+     * @return string
+     */
+    public function getProtectedResourceWellKnownUrl(): string
+    {
+        $path = trim($this->getBasePath() . '/mcp', '/');
+
+        return $this->getWellKnownUrl('oauth-protected-resource', $path);
+    }
+
+    /**
+     * @return string
+     */
+    public function getAuthServerWellKnownUrl(): string
+    {
+        return $this->getWellKnownUrl('oauth-authorization-server', $this->getBasePath());
+    }
 }
diff --git a/Plugin/Store/WellKnownRedirectExemption.php b/Plugin/Store/WellKnownRedirectExemption.php
@@ -0,0 +1,58 @@
+<?php
+/**
+ * @author    Magebit <info@magebit.com>
+ * @copyright Copyright (c) Magebit, Ltd. (https://magebit.com)
+ * @license   MIT
+ */
+declare(strict_types=1);
+
+namespace Magebit\Mcp\Plugin\Store;
+
+use Magento\Framework\App\Request\Http as HttpRequest;
+use Magento\Store\Model\BaseUrlChecker;
+
+/**
+ * Exempts the OAuth/MCP `.well-known` discovery paths from Magento's
+ * redirect-to-base check. RFC 8414 §3 / RFC 9728 §3.1 require these documents
+ * at the authority root, but on stores whose base URL carries a path (store
+ * code) the base-URL check would otherwise 302 them to the store-coded path
+ * before the WellKnownRouter runs. This treats the paths as already canonical;
+ * it never adds or rewrites the store code.
+ */
+class WellKnownRedirectExemption
+{
+    /**
+     * @var string[]
+     */
+    private const EXEMPT_PREFIXES = [
+        '.well-known/oauth-protected-resource',
+        '.well-known/oauth-authorization-server',
+    ];
+
+    /**
+     * @param BaseUrlChecker $subject
+     * @param bool $result
+     * @param array<string, mixed> $uri
+     * @param HttpRequest $request
+     * @return bool
+     */
+    public function afterExecute(
+        BaseUrlChecker $subject,
+        bool $result,
+        array $uri,
+        HttpRequest $request
+    ): bool {
+        if ($result) {
+            return true;
+        }
+
+        $path = trim((string) $request->getPathInfo(), '/');
+        foreach (self::EXEMPT_PREFIXES as $prefix) {
+            if ($path === $prefix || str_starts_with($path, $prefix . '/')) {
+                return true;
+            }
+        }
+
+        return $result;
+    }
+}
diff --git a/Test/Api/AuthTest.php b/Test/Api/AuthTest.php
@@ -32,8 +32,10 @@ public function testMissingAuthorizationHeaderReturns401(): void
             'resource_metadata="',
             $response['headers']['www-authenticate']
         );
+        // RFC 9728 §3.1: .well-known is inserted directly after the authority,
+        // with the resource path appended after it — never behind a store-code path.
         self::assertStringContainsString(
-            '/mcp/oauth/protected-resource-metadata',
+            '/.well-known/oauth-protected-resource/mcp',
             $response['headers']['www-authenticate']
         );
     }
diff --git a/Test/Unit/Controller/Router/WellKnownRouterTest.php b/Test/Unit/Controller/Router/WellKnownRouterTest.php
@@ -52,6 +52,21 @@ public function testMatchesAuthorizationServerMetadata(): void
         self::assertSame($action, $this->router->match($this->request('/.well-known/oauth-authorization-server')));
     }
 
+    public function testMatchesAuthorizationServerMetadataWithIssuerSuffix(): void
+    {
+        // RFC 8414 §3 — the issuer path is appended to the .well-known prefix.
+        $action = $this->createMock(ActionInterface::class);
+        $this->routeConfig->method('getModulesByFrontName')->with('mcp')->willReturn(['Magebit_Mcp']);
+        $this->actionList->method('get')
+            ->with('Magebit_Mcp', '', 'oauth', 'authorizationservermetadata')
+            ->willReturn(AuthorizationServerMetadata::class);
+        $this->actionFactory->method('create')
+            ->with(AuthorizationServerMetadata::class)
+            ->willReturn($action);
+
+        self::assertSame($action, $this->router->match($this->request('/.well-known/oauth-authorization-server/lv')));
+    }
+
     public function testMatchesProtectedResourceMetadata(): void
     {
         $action = $this->createMock(ActionInterface::class);
diff --git a/Test/Unit/Model/Url/PublicUrlBuilderTest.php b/Test/Unit/Model/Url/PublicUrlBuilderTest.php
@@ -39,10 +39,7 @@ public function testReturnsStoreBaseUrl(): void
 
     public function testBuildUrlConcatenatesPathOntoBaseUrl(): void
     {
-        $store = $this->createMock(Store::class);
-        $store->method('getBaseUrl')->with(UrlInterface::URL_TYPE_WEB)
-            ->willReturn('https://example.com/');
-        $this->storeManager->method('getStore')->willReturn($store);
+        $this->withBaseUrl('https://example.com/');
 
         self::assertSame(
             'https://example.com/.well-known/oauth-protected-resource',
@@ -53,4 +50,100 @@ public function testBuildUrlConcatenatesPathOntoBaseUrl(): void
             $this->builder->buildUrl('foo')
         );
     }
+
+    public function testGetAuthorityStripsPathComponent(): void
+    {
+        $this->withBaseUrl('https://example.com/lv/');
+        self::assertSame('https://example.com', $this->builder->getAuthority());
+    }
+
+    public function testGetAuthorityPreservesPort(): void
+    {
+        $this->withBaseUrl('https://example.com:8443/lv/');
+        self::assertSame('https://example.com:8443', $this->builder->getAuthority());
+    }
+
+    public function testGetAuthorityFallsBackOnMalformedBaseUrl(): void
+    {
+        // parse_url can't yield scheme+host — never emit an empty authority.
+        $this->withBaseUrl('not a url');
+        self::assertSame('not a url', $this->builder->getAuthority());
+    }
+
+    /**
+     * @dataProvider basePathProvider
+     * @param string $baseUrl
+     * @param string $expected
+     * @return void
+     */
+    public function testGetBasePath(string $baseUrl, string $expected): void
+    {
+        $this->withBaseUrl($baseUrl);
+        self::assertSame($expected, $this->builder->getBasePath());
+    }
+
+    /**
+     * @return array<string, array{string, string}>
+     */
+    public static function basePathProvider(): array
+    {
+        return [
+            'root' => ['https://example.com/', ''],
+            'store code' => ['https://example.com/lv/', 'lv'],
+            'subfolder' => ['https://example.com/shop/', 'shop'],
+        ];
+    }
+
+    public function testGetResourceUrlKeepsBasePath(): void
+    {
+        $this->withBaseUrl('https://example.com/lv/');
+        self::assertSame('https://example.com/lv/mcp', $this->builder->getResourceUrl());
+    }
+
+    public function testProtectedResourceWellKnownUrlInsertsAfterAuthority(): void
+    {
+        $this->withBaseUrl('https://example.com/lv/');
+        self::assertSame(
+            'https://example.com/.well-known/oauth-protected-resource/lv/mcp',
+            $this->builder->getProtectedResourceWellKnownUrl()
+        );
+    }
+
+    public function testProtectedResourceWellKnownUrlOnCleanBaseUrl(): void
+    {
+        $this->withBaseUrl('https://example.com/');
+        self::assertSame(
+            'https://example.com/.well-known/oauth-protected-resource/mcp',
+            $this->builder->getProtectedResourceWellKnownUrl()
+        );
+    }
+
+    public function testAuthServerWellKnownUrlAppendsBasePath(): void
+    {
+        $this->withBaseUrl('https://example.com/lv/');
+        self::assertSame(
+            'https://example.com/.well-known/oauth-authorization-server/lv',
+            $this->builder->getAuthServerWellKnownUrl()
+        );
+    }
+
+    public function testAuthServerWellKnownUrlOnCleanBaseUrl(): void
+    {
+        $this->withBaseUrl('https://example.com/');
+        self::assertSame(
+            'https://example.com/.well-known/oauth-authorization-server',
+            $this->builder->getAuthServerWellKnownUrl()
+        );
+    }
+
+    /**
+     * @param string $baseUrl
+     * @return void
+     */
+    private function withBaseUrl(string $baseUrl): void
+    {
+        $store = $this->createMock(Store::class);
+        $store->method('getBaseUrl')->with(UrlInterface::URL_TYPE_WEB)->willReturn($baseUrl);
+        $this->storeManager->method('getStore')->willReturn($store);
+    }
 }
diff --git a/Test/Unit/Plugin/Store/WellKnownRedirectExemptionTest.php b/Test/Unit/Plugin/Store/WellKnownRedirectExemptionTest.php
diff --git a/etc/frontend/di.xml b/etc/frontend/di.xml

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,7 @@`
`27`	`27`	`class WellKnownRouter implements RouterInterface`
`28`	`28`	`{`
`29`	`29`	`private const PATH_AUTHORIZATION_SERVER = '.well-known/oauth-authorization-server';`
	`30`	`+ private const PATH_AUTHORIZATION_SERVER_PREFIX = '.well-known/oauth-authorization-server/';`
`30`	`31`	`private const PATH_PROTECTED_RESOURCE = '.well-known/oauth-protected-resource';`
`31`	`32`	`private const PATH_PROTECTED_RESOURCE_PREFIX = '.well-known/oauth-protected-resource/';`
`32`	`33`
`@@ -85,7 +86,9 @@ public function match(RequestInterface $request): ?ActionInterface`
`85`	`86`	`*/`
`86`	`87`	`private function resolveAction(string $identifier): ?string`
`87`	`88`	`{`
`88`		`- if ($identifier === self::PATH_AUTHORIZATION_SERVER) {`
	`89`	`+ if ($identifier === self::PATH_AUTHORIZATION_SERVER`
	`90`	`+ \|\| str_starts_with($identifier, self::PATH_AUTHORIZATION_SERVER_PREFIX)`
	`91`	`+ ) {`
`89`	`92`	`return self::ACTION_AUTHORIZATION_SERVER;`
`90`	`93`	`}`
`91`	`94`	`if ($identifier === self::PATH_PROTECTED_RESOURCE`