Merge pull request #894 from cbcoutinho/feat/advertise-offline-access-scope
feat(auth): advertise offline_access in discovered OAuth scopes
This commit is contained in:
@@ -523,11 +523,12 @@ def discover_all_scopes(mcp) -> list[str]:
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
scopes = discover_all_scopes(mcp)
|
scopes = discover_all_scopes(mcp)
|
||||||
# Returns: ["notes.read", "notes.write", "openid", "profile", "email"]
|
# Returns: ["notes.read", "notes.write", "offline_access", "openid", ...]
|
||||||
```
|
```
|
||||||
|
|
||||||
Note:
|
Note:
|
||||||
- Base OIDC scopes (openid, profile, email) are always included
|
- Base OIDC scopes (openid, profile, email) are always included
|
||||||
|
- offline_access is always included so clients can request a refresh token
|
||||||
- Scopes are deduplicated and sorted alphabetically
|
- Scopes are deduplicated and sorted alphabetically
|
||||||
- Only scopes from decorated tools are included
|
- Only scopes from decorated tools are included
|
||||||
- Must be called after tools are registered
|
- Must be called after tools are registered
|
||||||
@@ -535,6 +536,18 @@ def discover_all_scopes(mcp) -> list[str]:
|
|||||||
# Start with base OIDC scopes that are always required
|
# Start with base OIDC scopes that are always required
|
||||||
all_scopes = {"openid", "profile", "email"}
|
all_scopes = {"openid", "profile", "email"}
|
||||||
|
|
||||||
|
# Advertise offline_access so discovery-driven MCP clients can request a
|
||||||
|
# refresh token. The AS proxy forwards it upstream to Nextcloud, which
|
||||||
|
# issues a refresh token when the MCP server's OIDC client is permitted the
|
||||||
|
# scope. Optional for clients (unlike the base OIDC scopes) and never tied
|
||||||
|
# to a tool, so it is added here rather than discovered from @require_scopes.
|
||||||
|
#
|
||||||
|
# Advertised unconditionally — independent of settings.enable_offline_access
|
||||||
|
# (which gates the server's own Flow 2 background access). Per RFC 8414,
|
||||||
|
# scopes_supported lists what the AS *can* support, not what it will always
|
||||||
|
# grant; the actual refresh token is still gated upstream by Nextcloud.
|
||||||
|
all_scopes.add("offline_access")
|
||||||
|
|
||||||
# Get all registered tools
|
# Get all registered tools
|
||||||
try:
|
try:
|
||||||
tools = mcp._tool_manager.list_tools()
|
tools = mcp._tool_manager.list_tools()
|
||||||
|
|||||||
@@ -1,9 +1,11 @@
|
|||||||
"""Unit tests for scope decorator metadata and classification logic."""
|
"""Unit tests for scope decorator metadata and classification logic."""
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from mcp.server.fastmcp import FastMCP
|
||||||
|
|
||||||
from nextcloud_mcp_server.auth.scope_authorization import (
|
from nextcloud_mcp_server.auth.scope_authorization import (
|
||||||
InsufficientScopeError,
|
InsufficientScopeError,
|
||||||
|
discover_all_scopes,
|
||||||
require_scopes,
|
require_scopes,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -63,3 +65,36 @@ def test_insufficient_scope_error_with_custom_message():
|
|||||||
|
|
||||||
assert error.missing_scopes == missing
|
assert error.missing_scopes == missing
|
||||||
assert str(error) == custom_msg
|
assert str(error) == custom_msg
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
def test_discover_all_scopes_always_includes_offline_access():
|
||||||
|
"""offline_access is advertised so discovery-driven clients can request a refresh token.
|
||||||
|
|
||||||
|
It is not tied to any tool's @require_scopes, so it must be present even on
|
||||||
|
an MCP instance with a single unrelated tool. Guards the metadata exposed at
|
||||||
|
/.well-known/oauth-protected-resource and /.well-known/oauth-authorization-server.
|
||||||
|
"""
|
||||||
|
mcp = FastMCP(name="test-scope-discovery")
|
||||||
|
|
||||||
|
@mcp.tool()
|
||||||
|
@require_scopes("notes.read")
|
||||||
|
async def example_tool():
|
||||||
|
pass
|
||||||
|
|
||||||
|
scopes = discover_all_scopes(mcp)
|
||||||
|
|
||||||
|
assert "offline_access" in scopes
|
||||||
|
# Base OIDC scopes and tool-derived scopes still come through.
|
||||||
|
assert {"openid", "profile", "email", "notes.read"}.issubset(scopes)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
def test_discover_all_scopes_offline_access_without_any_tools():
|
||||||
|
"""The offline_access invariant must not depend on any tool being registered."""
|
||||||
|
mcp = FastMCP(name="empty")
|
||||||
|
|
||||||
|
scopes = discover_all_scopes(mcp)
|
||||||
|
|
||||||
|
assert "offline_access" in scopes
|
||||||
|
assert {"openid", "profile", "email"}.issubset(scopes)
|
||||||
|
|||||||
Reference in New Issue
Block a user