fix: OAuth token exchange fails with 500 for public PKCE clients (#576) (#577)

* fix: OAuth token exchange fails with 500 for public PKCE clients (#576)

Three fixes for OAuth flow failing when claude.ai connects via MCP:

1. Allow public clients (no client_secret) using PKCE code_verifier for
   identity proof per OAuth 2.1 §2.1 — previously authenticate_client()
   was called with empty string, always failing authentication.

2. Add /.well-known/oauth-protected-resource endpoint (RFC 9728) which
   was returning 404 — required by MCP spec for OAuth discovery.

3. Add exc_info=True to error logging in token and authorization
   endpoints so actual exceptions are visible in logs instead of
   generic "Token endpoint error" messages.

Closes #576

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Update src/mcp_memory_service/web/oauth/authorization.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update src/mcp_memory_service/web/oauth/discovery.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* fix: address code review security findings

- Enforce PKCE (code_verifier) for public clients — prevents zero-auth
  token exchange when no client_secret and no PKCE challenge (OAuth 2.1 §7.5.2)
- Fix type annotation: final_client_secret is Optional[str], not str
- Fix resource_documentation: remove broken _docs_endpoint_exists() call
  from committed suggestion, use FastAPI's default /docs endpoint URL

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Heinrich Krupp <hkr@Mac-mini-von-Heinrich.local>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Author: 5 people

src/mcp_memory_service/web/oauth/authorization.py

@@ -295,7 +295,7 @@ async def authorize_post(
     except HTTPException:
         raise
     except Exception:
-        logger.error("Authorization error occurred")
+        logger.error("Authorization error occurred", exc_info=True)
         error_params = {"error": "server_error", "error_description": "Internal server error"}
         if state:
             error_params["state"] = _sanitize_state(state)
@@ -306,7 +306,7 @@ async def authorize_post(
 
 async def _handle_authorization_code_grant(
     final_client_id: str,
-    final_client_secret: str,
+    final_client_secret: Optional[str],
     code: Optional[str],
     redirect_uri: Optional[str],
     code_verifier: Optional[str] = None
@@ -330,15 +330,38 @@ async def _handle_authorization_code_grant(
             }
         )
 
-    # Authenticate client
-    if not await get_oauth_storage().authenticate_client(final_client_id, final_client_secret or ""):
-        raise HTTPException(
-            status_code=status.HTTP_401_UNAUTHORIZED,
-            detail={
-                "error": "invalid_client",
-                "error_description": "Client authentication failed"
-            }
-        )
+    # Authenticate client — but allow public clients using PKCE (OAuth 2.1 §2.1)
+    # Public clients (e.g. claude.ai) may not send a client_secret; they prove
+    # identity via PKCE code_verifier instead.
+    if final_client_secret:
+        if not await get_oauth_storage().authenticate_client(final_client_id, final_client_secret):
+            raise HTTPException(
+                status_code=status.HTTP_401_UNAUTHORIZED,
+                detail={
+                    "error": "invalid_client",
+                    "error_description": "Client authentication failed"
+                }
+            )
+    else:
+        # Public client — verify it exists and is actually a public client
+        client = await get_oauth_storage().get_client(final_client_id)
+        if not client or client.token_endpoint_auth_method != "none":
+            raise HTTPException(
+                status_code=status.HTTP_401_UNAUTHORIZED,
+                detail={
+                    "error": "invalid_client",
+                    "error_description": "Client authentication failed"
+                }
+            )
+        # PKCE is mandatory for public clients (OAuth 2.1 §7.5.2)
+        if not code_verifier:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail={
+                    "error": "invalid_request",
+                    "error_description": "code_verifier required for public clients"
+                }
+            )
 
     # Get and consume authorization code
     code_data = await get_oauth_storage().get_authorization_code(code)
@@ -507,7 +530,7 @@ async def token(
         # Re-raise HTTP exceptions
         raise
     except Exception:
-        logger.error("Token endpoint error")
+        logger.error("Token endpoint error", exc_info=True)
         raise HTTPException(
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
             detail={

src/mcp_memory_service/web/oauth/discovery.py

@@ -21,14 +21,33 @@
 import logging
 from fastapi import APIRouter
 from ...config import OAUTH_ISSUER, get_jwt_algorithm
-from .models import OAuthServerMetadata
+from .models import OAuthServerMetadata, OAuthProtectedResourceMetadata
 
 logger = logging.getLogger(__name__)
 
 router = APIRouter()
 
 
 
+@router.get("/.well-known/oauth-protected-resource")
+async def oauth_protected_resource_metadata() -> OAuthProtectedResourceMetadata:
+    """
+    OAuth 2.0 Protected Resource Metadata endpoint (RFC 9728).
+
+    Returns metadata about the protected resource, including which authorization
+    server(s) can issue tokens for it. Required by MCP spec for OAuth integration.
+    """
+    logger.info("OAuth protected resource metadata requested")
+
+    return OAuthProtectedResourceMetadata(
+        resource=OAUTH_ISSUER,
+        authorization_servers=[OAUTH_ISSUER],
+        scopes_supported=["read", "write", "admin"],
+        bearer_methods_supported=["header"],
+        resource_documentation=f"{OAUTH_ISSUER}/docs",
+    )
+
+
 @router.get("/.well-known/oauth-authorization-server/mcp")
 async def oauth_authorization_server_metadata() -> OAuthServerMetadata:
     """
@@ -49,7 +68,7 @@ async def oauth_authorization_server_metadata() -> OAuthServerMetadata:
         registration_endpoint=f"{OAUTH_ISSUER}/oauth/register",
         grant_types_supported=["authorization_code", "client_credentials"],
         response_types_supported=["code"],
-        token_endpoint_auth_methods_supported=["client_secret_basic", "client_secret_post"],
+        token_endpoint_auth_methods_supported=["client_secret_basic", "client_secret_post", "none"],
         scopes_supported=["read", "write", "admin"],
         id_token_signing_alg_values_supported=[algorithm],
         code_challenge_methods_supported=["S256"]

src/mcp_memory_service/web/oauth/models.py

@@ -54,6 +54,24 @@ class OAuthServerMetadata(BaseModel):
     )
 
 
+class OAuthProtectedResourceMetadata(BaseModel):
+    """OAuth 2.0 Protected Resource Metadata (RFC 9728)."""
+
+    resource: str = Field(..., description="Protected resource identifier (its URL)")
+    authorization_servers: List[str] = Field(
+        ..., description="Authorization server(s) that can authorize access"
+    )
+    scopes_supported: Optional[List[str]] = Field(
+        default=None, description="Scopes supported by this resource"
+    )
+    bearer_methods_supported: Optional[List[str]] = Field(
+        default=None, description="Methods for sending bearer tokens"
+    )
+    resource_documentation: Optional[str] = Field(
+        default=None, description="URL of resource documentation"
+    )
+
+
 class ClientRegistrationRequest(BaseModel):
     """OAuth 2.1 Dynamic Client Registration Request (RFC 7591).