Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions mcp_server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,15 @@ def _build_oauth(mongo: Any, *, mcp_base: str, webapp_base: str) -> tuple[Any, A
issuer_url=AnyHttpUrl(mcp_base),
resource_server_url=AnyHttpUrl(mcp_base),
client_registration_options=ClientRegistrationOptions(
# Offer "write" as an available scope; keep the default read-only so a
# client that registers without asking (and every existing connection)
# stays read-only. Write is granted only when a client explicitly
# registers for and requests it, and the user approves it on consent.
# Default new registrations to BOTH read+write. A client that registers
# without naming a scope (Claude.ai does exactly this) then gets the
# write ceiling at DCR, so it *can* be granted write. This is not a
# silent grant: the user still approves the shown scopes on the consent
# screen, which is the real gate. A read-only default capped every
# Claude.ai connection at read and made write unreachable.
enabled=True,
valid_scopes=["read", "write"],
default_scopes=["read"],
default_scopes=["read", "write"],
),
revocation_options=RevocationOptions(enabled=True),
required_scopes=[],
Expand Down
13 changes: 12 additions & 1 deletion mcp_server/oauth_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,23 @@ async def register_client(self, client_info: OAuthClientInformationFull) -> None
async def authorize(
self, client: OAuthClientInformationFull, params: AuthorizationParams
) -> str:
# Scope resolution. If the client narrowed scope at /authorize, honor it
# (the SDK already validated it ⊆ the client's registered scope). If it
# omitted scope — as Claude.ai does — grant the client's full *registered*
# scope, so the DCR default (ClientRegistrationOptions.default_scopes) is
# the single source of truth instead of a second default hardcoded here
# that silently capped every scope-less authorize at read. Consent still
# shows and gates whatever scopes land here.
scopes = list(params.scopes) if params.scopes else None
if scopes is None:
registered = (getattr(client, "scope", None) or "").split()
scopes = registered or list(self._default_scopes)
txn = {
"client_id": client.client_id,
"redirect_uri": str(params.redirect_uri),
"redirect_uri_provided_explicitly": bool(params.redirect_uri_provided_explicitly),
"code_challenge": params.code_challenge,
"scopes": list(params.scopes or self._default_scopes),
"scopes": scopes,
"state": params.state,
"resource": params.resource,
}
Expand Down
62 changes: 60 additions & 2 deletions tests/test_mcp_oauth_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def save_file(self, *a, **k):
return {"ok": True, "created": True, "file": {}}


def _build():
def _build(default_scopes=("read",)):
store = OAuthStore(FakeDB())
provider = CodeKeeperOAuthProvider(
store=store,
Expand All @@ -75,7 +75,7 @@ def _build():
issuer_url=AnyHttpUrl(BASE),
resource_server_url=AnyHttpUrl(BASE),
client_registration_options=ClientRegistrationOptions(
enabled=True, valid_scopes=["read", "write"], default_scopes=["read"]
enabled=True, valid_scopes=["read", "write"], default_scopes=list(default_scopes)
),
revocation_options=RevocationOptions(enabled=True),
required_scopes=[],
Expand Down Expand Up @@ -223,6 +223,64 @@ async def test_write_scope_survives_from_authorize_to_token():
assert "write" in (tok.json().get("scope") or "").split()


async def test_scopeless_client_gets_write_from_registered_default():
# Reproduces the real Claude.ai failure: the client registers AND authorizes
# without ever naming a scope. With default_scopes=["read","write"], DCR
# registers it for read+write and the scope-less /authorize grants that full
# registered scope — so write reaches the token (consent still gates it).
# Before the fix this yielded read-only because /authorize fell back to a
# hardcoded ("read",) instead of the client's registered scope.
app = _build(default_scopes=("read", "write"))
verifier, challenge = _pkce()
async with _client(app) as c:
reg = await c.post(
"/register",
json={
"redirect_uris": [f"{BASE}/callback"],
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
"token_endpoint_auth_method": "none",
# NOTE: no "scope" key — mirrors Claude.ai's registration.
},
)
assert reg.status_code == 201, reg.text
client_id = reg.json()["client_id"]
# DCR assigned the read+write default as the client's registered ceiling.
assert set((reg.json().get("scope") or "").split()) == {"read", "write"}
az = await c.get(
"/authorize",
params={
"client_id": client_id,
"redirect_uri": f"{BASE}/callback",
"response_type": "code",
"code_challenge": challenge,
"code_challenge_method": "S256",
"state": "xyz",
# NOTE: no "scope" param — mirrors Claude.ai's /authorize.
},
)
assert az.status_code == 302, az.text
txn = parse_qs(urlparse(az.headers["location"]).query)["txn"][0]
exp, sig = sign_identity(SECRET, 555, txn)
cp = await c.post(
"/oauth/consent",
data={"txn": txn, "user_id": "555", "exp": str(exp), "sig": sig, "action": "approve"},
)
code = parse_qs(urlparse(cp.headers["location"]).query)["code"][0]
tok = await c.post(
"/token",
data={
"grant_type": "authorization_code",
"code": code,
"redirect_uri": f"{BASE}/callback",
"client_id": client_id,
"code_verifier": verifier,
},
)
assert tok.status_code == 200, tok.text
assert "write" in (tok.json().get("scope") or "").split()


async def test_token_rejects_wrong_pkce_verifier():
app = _build()
_, challenge = _pkce()
Expand Down
36 changes: 36 additions & 0 deletions tests/test_mcp_oauth_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,42 @@ async def test_authorize_creates_txn_and_redirect():
assert txn["client_id"] == "c1"


def _params_no_scope():
return AuthorizationParams(
state="st",
scopes=None, # client omitted scope at /authorize (the Claude.ai case)
code_challenge="chal",
redirect_uri=AnyUrl("https://claude.ai/cb"),
redirect_uri_provided_explicitly=True,
resource=None,
)


async def test_authorize_without_scope_falls_back_to_registered_scope():
# No scope at /authorize ⇒ grant the client's full *registered* scope, not a
# hardcoded read-only default. This is what lets Claude.ai (which never names
# a scope) reach write once it is registered for it.
store, prov = _provider()
client = OAuthClientInformationFull(
client_id="c1", redirect_uris=[AnyUrl("https://claude.ai/cb")], scope="read write"
)
url = await prov.authorize(client, _params_no_scope())
txn_id = up.parse_qs(up.urlparse(url).query)["txn"][0]
assert store.get_txn(txn_id)["scopes"] == ["read", "write"]


async def test_authorize_without_scope_and_no_registration_uses_read_default():
# Defensive fallback: a client with no registered scope at all (shouldn't
# happen via DCR) stays read-only rather than silently widening.
store, prov = _provider()
client = OAuthClientInformationFull(
client_id="c1", redirect_uris=[AnyUrl("https://claude.ai/cb")]
)
url = await prov.authorize(client, _params_no_scope())
txn_id = up.parse_qs(up.urlparse(url).query)["txn"][0]
assert store.get_txn(txn_id)["scopes"] == ["read"]


async def test_code_exchange_issues_tokens_and_consumes():
store, prov = _provider()
code = new_secret("ckoc_")
Expand Down
Loading