Skip to content

Commit 3b3e699

Browse files
committed
test: add differential coverage for public visibility in team-scoped /ids endpoints
Add direct unit tests for _merge_select_all_ids helper covering all branches (flag not set, valid merge, int normalization, JSON decode error, missing key, empty checked list). Add regression tests for prompts and resources /ids endpoints to match the existing tool test, ensuring standalone visibility='public' condition is present in team-scoped queries. Closes #3446. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 810e53e commit 3b3e699

File tree

1 file changed

+131
-2
lines changed

1 file changed

+131
-2
lines changed

tests/unit/mcpgateway/test_admin.py

Lines changed: 131 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
_get_timeseries_metrics_python,
4848
_get_user_team_ids,
4949
_get_user_team_roles,
50+
_merge_select_all_ids,
5051
_normalize_search_query,
5152
_normalize_team_id,
5253
_normalize_ui_hide_values,
@@ -10331,7 +10332,10 @@ async def test_admin_get_all_tool_ids_team_scoped_includes_public(monkeypatch, m
1033110332
condition (not gated by team_id) so that platform-public tools from public MCP
1033210333
servers appear in team-scoped Select All fetches and can be associated with
1033310334
team-owned virtual servers. Regression test for issue #3446."""
10335+
# Standard
1033410336
import re
10337+
10338+
# Third-Party
1033510339
from sqlalchemy.dialects import sqlite as sqlite_dialect
1033610340

1033710341
setup_team_service(monkeypatch, ["team-1"])
@@ -10352,11 +10356,136 @@ async def test_admin_get_all_tool_ids_team_scoped_includes_public(monkeypatch, m
1035210356
# OR alternative — not wrapped inside `team_id = '...' AND visibility IN (...)`.
1035310357
# This is what makes platform-public tools visible to team-scoped queries.
1035410358
assert re.search(r"tools\.visibility\s*=\s*'public'", sql), (
10355-
"Expected a standalone visibility='public' condition in team-scoped tool IDs query. "
10356-
"Platform-public tools must be accessible when associating with team-owned virtual servers."
10359+
"Expected a standalone visibility='public' condition in team-scoped tool IDs query. " "Platform-public tools must be accessible when associating with team-owned virtual servers."
1035710360
)
1035810361

1035910362

10363+
@pytest.mark.asyncio
10364+
async def test_admin_get_all_prompt_ids_team_scoped_includes_public(monkeypatch, mock_db):
10365+
"""When team_id is set, the SQL query must include a standalone visibility='public'
10366+
condition so platform-public prompts appear in team-scoped Select All fetches.
10367+
Regression test for issue #3446."""
10368+
# Standard
10369+
import re
10370+
10371+
# Third-Party
10372+
from sqlalchemy.dialects import sqlite as sqlite_dialect
10373+
10374+
setup_team_service(monkeypatch, ["team-1"])
10375+
mock_db.execute.return_value.all.return_value = []
10376+
10377+
await admin_get_all_prompt_ids(
10378+
include_inactive=False,
10379+
gateway_id=None,
10380+
team_id="team-1",
10381+
db=mock_db,
10382+
user={"email": "user@example.com", "db": mock_db},
10383+
)
10384+
10385+
executed_query = mock_db.execute.call_args[0][0]
10386+
sql = str(executed_query.compile(dialect=sqlite_dialect.dialect(), compile_kwargs={"literal_binds": True}))
10387+
10388+
assert re.search(r"prompts\.visibility\s*=\s*'public'", sql), (
10389+
"Expected a standalone visibility='public' condition in team-scoped prompt IDs query. " "Platform-public prompts must be accessible when associating with team-owned virtual servers."
10390+
)
10391+
10392+
10393+
@pytest.mark.asyncio
10394+
async def test_admin_get_all_resource_ids_team_scoped_includes_public(monkeypatch, mock_db):
10395+
"""When team_id is set, the SQL query must include a standalone visibility='public'
10396+
condition so platform-public resources appear in team-scoped Select All fetches.
10397+
Regression test for issue #3446."""
10398+
# Standard
10399+
import re
10400+
10401+
# Third-Party
10402+
from sqlalchemy.dialects import sqlite as sqlite_dialect
10403+
10404+
setup_team_service(monkeypatch, ["team-1"])
10405+
mock_db.execute.return_value.all.return_value = []
10406+
10407+
await admin_get_all_resource_ids(
10408+
include_inactive=False,
10409+
gateway_id=None,
10410+
team_id="team-1",
10411+
db=mock_db,
10412+
user={"email": "user@example.com", "db": mock_db},
10413+
)
10414+
10415+
executed_query = mock_db.execute.call_args[0][0]
10416+
sql = str(executed_query.compile(dialect=sqlite_dialect.dialect(), compile_kwargs={"literal_binds": True}))
10417+
10418+
assert re.search(r"resources\.visibility\s*=\s*'public'", sql), (
10419+
"Expected a standalone visibility='public' condition in team-scoped resource IDs query. " "Platform-public resources must be accessible when associating with team-owned virtual servers."
10420+
)
10421+
10422+
10423+
class TestMergeSelectAllIds:
10424+
"""Direct unit tests for the _merge_select_all_ids helper."""
10425+
10426+
def test_flag_not_set_returns_checked_list_unchanged(self):
10427+
"""When the select-all flag is absent, return the checked list as-is."""
10428+
form = FakeForm({"otherField": "value"})
10429+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", ["t1", "t2"])
10430+
assert result == ["t1", "t2"]
10431+
10432+
def test_flag_false_returns_checked_list_unchanged(self):
10433+
"""When the select-all flag is 'false', return the checked list as-is."""
10434+
form = FakeForm({"selectAllTools": "false"})
10435+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", ["t1"])
10436+
assert result == ["t1"]
10437+
10438+
def test_merge_union_of_server_and_checked_ids(self):
10439+
"""When select-all is active, return the union of server IDs and checked IDs."""
10440+
form = FakeForm(
10441+
{
10442+
"selectAllTools": "true",
10443+
"allToolIds": json.dumps(["s1", "s2"]),
10444+
}
10445+
)
10446+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", ["c1", "s2"])
10447+
assert set(result) == {"s1", "s2", "c1"}
10448+
10449+
def test_int_ids_normalised_to_str(self):
10450+
"""Integer IDs from JSON should be stringified to avoid int/str duplicates."""
10451+
form = FakeForm(
10452+
{
10453+
"selectAllTools": "true",
10454+
"allToolIds": json.dumps([1, 2, 3]),
10455+
}
10456+
)
10457+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", ["2", "4"])
10458+
assert set(result) == {"1", "2", "3", "4"}
10459+
10460+
def test_invalid_json_falls_back_to_checked_list(self):
10461+
"""If the JSON payload is malformed, fall back to checked list."""
10462+
form = FakeForm(
10463+
{
10464+
"selectAllTools": "true",
10465+
"allToolIds": "not-json",
10466+
}
10467+
)
10468+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", ["t1"])
10469+
assert result == ["t1"]
10470+
10471+
def test_missing_all_ids_key_uses_empty_default(self):
10472+
"""If the all-IDs field is missing, treat as empty server list."""
10473+
form = FakeForm({"selectAllTools": "true"})
10474+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", ["t1"])
10475+
assert set(result) == {"t1"}
10476+
10477+
def test_empty_checked_list_returns_server_ids_only(self):
10478+
"""When no checkboxes were checked, return only server-fetched IDs."""
10479+
form = FakeForm(
10480+
{
10481+
"selectAllTools": "true",
10482+
"allToolIds": json.dumps(["s1", "s2"]),
10483+
}
10484+
)
10485+
result = _merge_select_all_ids(form, "selectAllTools", "allToolIds", [])
10486+
assert set(result) == {"s1", "s2"}
10487+
10488+
1036010489
@pytest.mark.asyncio
1036110490
async def test_admin_get_all_prompt_ids_gateway_and_team_filters(monkeypatch, mock_db):
1036210491
"""Cover non-null gateway filters and team membership checks for prompt IDs helper."""

0 commit comments

Comments
 (0)