mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 15:19:10 +02:00
fix(auth): disambiguate auth-denied vs missing component messages (#4165)
This commit is contained in:
parent
11951585c0
commit
344a4f81b5
2 changed files with 92 additions and 6 deletions
|
|
@ -136,11 +136,15 @@ class AuthMiddleware(Middleware):
|
|||
f"Authorization failed for tool '{tool_name}': missing context"
|
||||
)
|
||||
|
||||
# Get tool (component auth is checked in get_tool, raises if unauthorized)
|
||||
# get_tool returns None both when the tool does not exist and when
|
||||
# component-level auth denied access, so the two cases are
|
||||
# indistinguishable here. Keep the message ambiguous to avoid
|
||||
# disclosing existence of tools the caller is not authorized to see.
|
||||
tool = await fastmcp.fastmcp.get_tool(tool_name)
|
||||
if tool is None:
|
||||
raise AuthorizationError(
|
||||
f"Authorization failed for tool '{tool_name}': tool not found"
|
||||
f"Authorization failed for tool '{tool_name}': "
|
||||
"not found or not authorized"
|
||||
)
|
||||
|
||||
# Global auth check
|
||||
|
|
@ -204,13 +208,17 @@ class AuthMiddleware(Middleware):
|
|||
f"Authorization failed for resource '{uri}': missing context"
|
||||
)
|
||||
|
||||
# Get resource/template (component auth is checked in get_*, raises if unauthorized)
|
||||
# get_resource/get_resource_template return None both when the resource
|
||||
# does not exist and when component-level auth denied access, so the two
|
||||
# cases are indistinguishable here. Keep the message ambiguous to avoid
|
||||
# disclosing existence of resources the caller is not authorized to see.
|
||||
component = await fastmcp.fastmcp.get_resource(str(uri))
|
||||
if component is None:
|
||||
component = await fastmcp.fastmcp.get_resource_template(str(uri))
|
||||
if component is None:
|
||||
raise AuthorizationError(
|
||||
f"Authorization failed for resource '{uri}': resource not found"
|
||||
f"Authorization failed for resource '{uri}': "
|
||||
"not found or not authorized"
|
||||
)
|
||||
|
||||
# Global auth check
|
||||
|
|
@ -303,11 +311,15 @@ class AuthMiddleware(Middleware):
|
|||
f"Authorization failed for prompt '{prompt_name}': missing context"
|
||||
)
|
||||
|
||||
# Get prompt (component auth is checked in get_prompt, raises if unauthorized)
|
||||
# get_prompt returns None both when the prompt does not exist and when
|
||||
# component-level auth denied access, so the two cases are
|
||||
# indistinguishable here. Keep the message ambiguous to avoid
|
||||
# disclosing existence of prompts the caller is not authorized to see.
|
||||
prompt = await fastmcp.fastmcp.get_prompt(prompt_name)
|
||||
if prompt is None:
|
||||
raise AuthorizationError(
|
||||
f"Authorization failed for prompt '{prompt_name}': prompt not found"
|
||||
f"Authorization failed for prompt '{prompt_name}': "
|
||||
"not found or not authorized"
|
||||
)
|
||||
|
||||
# Global auth check
|
||||
|
|
|
|||
|
|
@ -810,3 +810,77 @@ class TestAuthMiddlewareCallTool:
|
|||
)
|
||||
finally:
|
||||
auth_context_var.reset(tok)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Tests for component-level auth denial messaging (issue #4054 bug 1)
|
||||
# =============================================================================
|
||||
|
||||
|
||||
def _allow_all(ctx: AuthContext) -> bool:
|
||||
"""Global auth check that always passes (component-level auth still applies)."""
|
||||
return True
|
||||
|
||||
|
||||
class TestComponentAuthDenialMessage:
|
||||
"""When component-level auth denies access, get_tool/get_resource/get_prompt
|
||||
return None, so the middleware cannot distinguish "missing" from "denied".
|
||||
|
||||
The message must stay ambiguous ("not found or not authorized") rather than
|
||||
asserting the component does not exist (misleading) or that it exists but is
|
||||
forbidden (leaks existence to unauthorized callers).
|
||||
"""
|
||||
|
||||
async def test_call_tool_denied_by_component_auth(self):
|
||||
mcp = FastMCP(middleware=[AuthMiddleware(auth=_allow_all)])
|
||||
|
||||
@mcp.tool(auth=require_scopes("admin"))
|
||||
def secret_tool() -> str:
|
||||
return "secret"
|
||||
|
||||
token = make_token(scopes=["read"])
|
||||
tok = set_token(token)
|
||||
try:
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(Exception) as exc_info:
|
||||
await client.call_tool("secret_tool", {})
|
||||
message = str(exc_info.value)
|
||||
assert "not found or not authorized" in message
|
||||
finally:
|
||||
auth_context_var.reset(tok)
|
||||
|
||||
async def test_read_resource_denied_by_component_auth(self):
|
||||
mcp = FastMCP(middleware=[AuthMiddleware(auth=_allow_all)])
|
||||
|
||||
@mcp.resource("data://secret", auth=require_scopes("admin"))
|
||||
def secret_resource() -> str:
|
||||
return "secret"
|
||||
|
||||
token = make_token(scopes=["read"])
|
||||
tok = set_token(token)
|
||||
try:
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(Exception) as exc_info:
|
||||
await client.read_resource("data://secret")
|
||||
message = str(exc_info.value)
|
||||
assert "not found or not authorized" in message
|
||||
finally:
|
||||
auth_context_var.reset(tok)
|
||||
|
||||
async def test_get_prompt_denied_by_component_auth(self):
|
||||
mcp = FastMCP(middleware=[AuthMiddleware(auth=_allow_all)])
|
||||
|
||||
@mcp.prompt(auth=require_scopes("admin"))
|
||||
def secret_prompt() -> str:
|
||||
return "secret"
|
||||
|
||||
token = make_token(scopes=["read"])
|
||||
tok = set_token(token)
|
||||
try:
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(Exception) as exc_info:
|
||||
await client.get_prompt("secret_prompt")
|
||||
message = str(exc_info.value)
|
||||
assert "not found or not authorized" in message
|
||||
finally:
|
||||
auth_context_var.reset(tok)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue