Skip to content

Commit a89b5ea

Browse files
committed
fix: address oauth-status review feedback on catalog OAuth registration
Widen _build_oauth_config_from_credentials to carry redirect_uri, username, password, audience, and resource, matching the field set admin._assemble_oauth_config_from_fields already accepts, so a catalog-registered OAuth gateway can get RFC 8707 resource/audience and password-grant fields set at registration time. Restore coverage for the requires_oauth_config boundary case removed alongside the oauth_config listing column: a disabled OAuth gateway must still requires_oauth_config even once oauth_config is persisted. Addresses PR #6588 review feedback. Signed-off-by: Marek Dano <mk.dano@gmail.com>
1 parent 0a295ec commit a89b5ea

2 files changed

Lines changed: 87 additions & 2 deletions

File tree

mcpgateway/services/catalog_service.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,8 @@ def _build_oauth_config_from_credentials(oauth_credentials: Optional[Dict[str, A
417417
418418
Args:
419419
oauth_credentials: Caller-supplied OAuth credential overrides (issuer, scopes, and
420-
optionally client_id/client_secret/token_url/authorization_url), or None.
420+
optionally client_id/client_secret/token_url/authorization_url/redirect_uri/
421+
username/password/audience/resource), or None.
421422
422423
Returns:
423424
A raw oauth_config dict with authorization_code/store_tokens/auto_refresh defaults
@@ -429,13 +430,19 @@ def _build_oauth_config_from_credentials(oauth_credentials: Optional[Dict[str, A
429430
"store_tokens": True,
430431
"auto_refresh": True,
431432
}
432-
for key in ("issuer", "client_id", "client_secret", "token_url", "authorization_url"):
433+
# Mirrors the field set admin._assemble_oauth_config_from_fields() accepts, so a
434+
# catalog-registered gateway can carry the same RFC 8707 resource/audience and
435+
# password-grant fields a manually-created gateway can (#5967 follow-up).
436+
for key in ("issuer", "client_id", "client_secret", "token_url", "authorization_url", "redirect_uri", "username", "password", "audience"):
433437
value = oauth_credentials.get(key)
434438
if value:
435439
raw_oauth_config[key] = value
436440
scopes = oauth_credentials.get("scopes")
437441
if scopes:
438442
raw_oauth_config["scopes"] = scopes if isinstance(scopes, list) else [str(scopes)]
443+
resource = oauth_credentials.get("resource")
444+
if resource:
445+
raw_oauth_config["resource"] = resource if isinstance(resource, list) else str(resource)
439446
return raw_oauth_config
440447

441448
async def register_catalog_server(

tests/unit/mcpgateway/services/test_catalog_service.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,56 @@ async def test_get_catalog_servers_requires_oauth_config_enabled(service):
218218
assert server.requires_oauth_config is False
219219

220220

221+
@pytest.mark.asyncio
222+
async def test_get_catalog_servers_requires_oauth_config_true_even_when_oauth_config_set(service, test_db):
223+
"""A disabled OAuth gateway still requires_oauth_config even once oauth_config is persisted.
224+
225+
Catalog registration now persists oauth_config up front (#5967), so an unauthorized gateway
226+
can have a populated oauth_config and still need the caller to complete the OAuth flow.
227+
requires_oauth_config must key off enabled/auth_type alone, not oauth_config presence -
228+
otherwise a genuinely unauthorized gateway would look fully configured to the caller.
229+
"""
230+
# First-Party
231+
from mcpgateway.db import Gateway as DbGateway
232+
233+
gateway = DbGateway(
234+
id="gw-configured-disabled",
235+
name="oauth-configured",
236+
slug="oauth-configured",
237+
url="http://oauth-configured.example.com",
238+
description="OAuth server with oauth_config already set, still disabled",
239+
capabilities={},
240+
auth_type="oauth",
241+
enabled=False,
242+
oauth_config={"grant_type": "authorization_code", "issuer": "https://idp.example.com"},
243+
)
244+
test_db.add(gateway)
245+
test_db.commit()
246+
247+
fake_catalog = {
248+
"catalog_servers": [
249+
{
250+
"id": "1",
251+
"name": "oauth-configured",
252+
"url": "http://oauth-configured.example.com",
253+
"category": "cat",
254+
"auth_type": "OAuth2.1",
255+
"provider": "prov",
256+
"tags": [],
257+
"description": "OAuth server",
258+
},
259+
]
260+
}
261+
with patch.object(service, "load_catalog", AsyncMock(return_value=fake_catalog)), patch.object(service, "_get_registry_cache", return_value=None):
262+
req = CatalogListRequest(offset=0, limit=10)
263+
result = await service.get_catalog_servers(req, test_db)
264+
assert result.total == 1
265+
server = result.servers[0]
266+
assert server.is_registered is True
267+
assert server.gateway_id == "gw-configured-disabled"
268+
assert server.requires_oauth_config is True
269+
270+
221271
@pytest.mark.asyncio
222272
async def test_register_catalog_server_not_found(service):
223273
with patch.object(service, "load_catalog", AsyncMock(return_value={"catalog_servers": []})):
@@ -546,6 +596,34 @@ def mock_refresh(obj):
546596
assert db_gateway.visibility == "private"
547597

548598

599+
def test_build_oauth_config_from_credentials_carries_resource_and_password_grant_fields(service):
600+
"""The catalog registration path must not drop RFC 8707 resource/audience or
601+
password-grant fields that the equivalent admin.py OAuth form assembly accepts,
602+
otherwise a catalog-registered gateway can never get them set at registration time."""
603+
raw = service._build_oauth_config_from_credentials(
604+
{
605+
"issuer": "https://issuer.example.com",
606+
"redirect_uri": "https://gateway.example.com/oauth/callback",
607+
"username": "svc-account",
608+
"password": "svc-secret", # pragma: allowlist secret
609+
"audience": "https://api.example.com",
610+
"resource": "https://api.example.com/mcp",
611+
}
612+
)
613+
assert raw["redirect_uri"] == "https://gateway.example.com/oauth/callback"
614+
assert raw["username"] == "svc-account"
615+
assert raw["password"] == "svc-secret" # pragma: allowlist secret
616+
assert raw["audience"] == "https://api.example.com"
617+
assert raw["resource"] == "https://api.example.com/mcp"
618+
619+
620+
def test_build_oauth_config_from_credentials_resource_list_preserved(service):
621+
"""A caller-supplied multi-value ``resource`` (RFC 7519 aud-claim shape) round-trips as a
622+
list rather than being coerced to str or dropped for not being a plain string."""
623+
raw = service._build_oauth_config_from_credentials({"resource": ["https://a.example.com", "https://b.example.com"]})
624+
assert raw["resource"] == ["https://a.example.com", "https://b.example.com"]
625+
626+
549627
@pytest.mark.asyncio
550628
async def test_register_oauth_skip_init_persists_oauth_credentials(service):
551629
"""oauth_credentials submitted with the register request land on the gateway's

0 commit comments

Comments
 (0)