Skip to content

Commit cd241b2

Browse files
fix(ui): set MAX_MEMBERS_PER_TEAM in team forms (#3650)
* fix(ui): set MAX_MEMBERS_PER_TEAM in team forms The "Maximum Members" input in the Create Team and Edit Team modals had max="1000" hardcoded, causing two problems: - non-admin users could set max_members above MAX_MEMBERS_PER_TEAM - when MAX_MEMBERS_PER_TEAM > 1000, the browser rejected valid values Fix by surfacing the setting to the UI with role-aware behaviour: - admins: no max attribute (no browser-side cap, matching skip_limits in the backend) - non-admins: max attribute set to MAX_MEMBERS_PER_TEAM (default 100) The default value in the Create modal is min(50, MAX_MEMBERS_PER_TEAM) so tighter operator-configured limits are respected out of the box. Closes #3589 Signed-off-by: Gabriel Costa <gabrielcg@proton.me> * fix(teams): use extracted is_admin var in update_team, add boundary test Use the already-extracted `is_admin` variable for `skip_limits` in update_team instead of re-evaluating `bool(current_user.get("is_admin"))`. Add missing boundary test for non-admin update at exactly the configured max_members_per_team limit, matching the create-path coverage. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(ui): handle over-limit max_members in non-admin team edit form When a non-admin user edits a team whose max_members already exceeds the configured MAX_MEMBERS_PER_TEAM (e.g. set by an admin), the HTML form would render value > max, causing browser validation to block submission. Fix by showing an empty field with a hint displaying the current value and the allowed limit. Submitting empty preserves the existing value server-side. Admins continue to see the actual value with no cap. Add regression tests for both admin and non-admin over-limit edit cases. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(docs): add missing docstring sections in passthrough_headers Add missing Args and Returns to _loopback_skip_set, safe_extract_headers_for_loopback, and safe_extract_and_filter_for_loopback to satisfy flake8 DAR checks. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Gabriel Costa <gabrielcg@proton.me> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 5025efa commit cd241b2

7 files changed

Lines changed: 217 additions & 14 deletions

File tree

mcpgateway/admin.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3982,6 +3982,7 @@ def _to_dict_and_filter(raw_list):
39823982
# Token policy flags
39833983
"require_token_expiration": getattr(settings, "require_token_expiration", True),
39843984
"sri_hashes": load_sri_hashes(),
3985+
"max_members_per_team": getattr(settings, "max_members_per_team", 100),
39853986
},
39863987
)
39873988

@@ -6011,6 +6012,22 @@ async def admin_get_team_edit(
60116012

60126013
safe_team_name = html.escape(team.name, quote=True)
60136014
safe_description = html.escape(team.description or "")
6015+
is_admin_edit = isinstance(_user, dict) and _user.get("is_admin")
6016+
max_members_limit = getattr(settings, "max_members_per_team", 100)
6017+
current_exceeds_limit = bool(team.max_members and team.max_members > max_members_limit)
6018+
max_attr = "" if is_admin_edit else f'max="{max_members_limit}"'
6019+
# When the existing value exceeds the configured limit for a non-admin,
6020+
# show an empty field to avoid browser validation blocking form submission.
6021+
# Submitting empty preserves the current value server-side.
6022+
if is_admin_edit:
6023+
max_members_value = team.max_members if team.max_members else ""
6024+
max_members_hint = "Admins can set any limit. Leave empty to keep current value."
6025+
elif current_exceeds_limit:
6026+
max_members_value = ""
6027+
max_members_hint = f"Current: {team.max_members} (above max {max_members_limit}). Leave empty to keep, or set a new value \u2264 {max_members_limit}."
6028+
else:
6029+
max_members_value = team.max_members if team.max_members else ""
6030+
max_members_hint = f"Max {max_members_limit}. Leave empty to keep current value."
60146031
edit_form = rf"""
60156032
<div class="space-y-4">
60166033
<h3 class="text-lg font-semibold text-gray-900 dark:text-white mb-4">Edit Team</h3>
@@ -6043,9 +6060,9 @@ async def admin_get_team_edit(
60436060
</div>
60446061
<div>
60456062
<label class="block text-sm font-medium text-gray-700 dark:text-gray-300">Maximum Members</label>
6046-
<input type="number" name="max_members" min="1" max="1000" value="{team.max_members if team.max_members else ''}"
6063+
<input type="number" name="max_members" min="1" {max_attr} value="{max_members_value}"
60476064
class="mt-1 block w-full px-3 py-2 border border-gray-300 dark:border-gray-600 rounded-md shadow-sm focus:outline-none focus:ring-indigo-500 focus:border-indigo-500 dark:bg-gray-700 text-gray-900 dark:text-white">
6048-
<p class="text-xs text-gray-500 dark:text-gray-400 mt-1">Leave empty to keep current value</p>
6065+
<p class="text-xs text-gray-500 dark:text-gray-400 mt-1">{max_members_hint}</p>
60496066
</div>
60506067
<div class="flex justify-end space-x-3">
60516068
<button type="button" onclick="hideTeamEditModal()"

mcpgateway/routers/teams.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,28 @@ async def create_team(request: TeamCreateRequest, current_user_ctx: dict = Depen
9696
True
9797
"""
9898
try:
99-
if not settings.allow_team_creation and not current_user_ctx.get("is_admin"):
99+
is_admin = bool(current_user_ctx.get("is_admin"))
100+
101+
if not settings.allow_team_creation and not is_admin:
100102
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Team creation is currently disabled")
101103

104+
# Enforce max_members_per_team limit for non-admin users
105+
if not is_admin and request.max_members is not None:
106+
limit = getattr(settings, "max_members_per_team", 100)
107+
if request.max_members > limit:
108+
raise HTTPException(
109+
status_code=status.HTTP_400_BAD_REQUEST,
110+
detail=f"max_members cannot exceed {limit} for non-admin users",
111+
)
112+
102113
service = TeamManagementService(db)
103114
team = await service.create_team(
104115
name=request.name,
105116
description=request.description,
106117
created_by=current_user_ctx["email"],
107118
visibility=request.visibility,
108119
max_members=request.max_members,
109-
skip_limits=bool(current_user_ctx.get("is_admin")),
120+
skip_limits=is_admin,
110121
)
111122

112123
# Build response BEFORE closing session to avoid lazy-load issues with get_member_count()
@@ -359,16 +370,24 @@ async def update_team(team_id: str, request: TeamUpdateRequest, current_user: di
359370
HTTPException: If team not found, access denied, or update fails
360371
"""
361372
try:
373+
is_admin = bool(current_user.get("is_admin"))
362374
service = TeamManagementService(db)
363375

364376
# Check if user is team owner
365377
role = await service.get_user_role_in_team(current_user["email"], team_id)
366378
if role != "owner":
367379
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=_ACCESS_DENIED_MSG)
368380

369-
success = await service.update_team(
370-
team_id=team_id, name=request.name, description=request.description, visibility=request.visibility, max_members=request.max_members, skip_limits=bool(current_user.get("is_admin"))
371-
)
381+
# Enforce max_members_per_team limit for non-admin users
382+
if not is_admin and request.max_members is not None:
383+
limit = getattr(settings, "max_members_per_team", 100)
384+
if request.max_members > limit:
385+
raise HTTPException(
386+
status_code=status.HTTP_400_BAD_REQUEST,
387+
detail=f"max_members cannot exceed {limit} for non-admin users",
388+
)
389+
390+
success = await service.update_team(team_id=team_id, name=request.name, description=request.description, visibility=request.visibility, max_members=request.max_members, skip_limits=is_admin)
372391

373392
if not success:
374393
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Team not found or update failed")

mcpgateway/templates/admin.html

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14645,12 +14645,16 @@ <h3 class="text-lg font-medium text-gray-900 dark:text-white">
1464514645
name="max_members"
1464614646
id="team-max-members"
1464714647
min="1"
14648-
max="1000"
14649-
value="50"
14648+
{% if not is_admin %}max="{{ max_members_per_team }}"{% endif %}
14649+
value="{{ [50, max_members_per_team] | min }}"
1465014650
class="block w-full border-gray-300 rounded-md shadow-sm focus:ring-indigo-500 focus:border-indigo-500 dark:bg-gray-700 dark:border-gray-600 dark:text-white"
1465114651
/>
1465214652
<p class="mt-1 text-xs text-gray-500 dark:text-gray-400">
14653-
Optional, defaults to 50
14653+
{% if is_admin %}
14654+
Optional. Admins can set any limit.
14655+
{% else %}
14656+
Optional, defaults to {{ [50, max_members_per_team] | min }}.
14657+
{% endif %}
1465414658
</p>
1465514659
</div>
1465614660
</div>

mcpgateway/utils/passthrough_headers.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,9 @@ def _loopback_skip_set() -> frozenset[str]:
568568
passthrough headers can never overwrite the gateway-internal proxy
569569
user identity — even if that header name is added to the passthrough
570570
allowlist by mistake.
571+
572+
Returns:
573+
frozenset[str]: Header names to skip during loopback forwarding.
571574
"""
572575
proxy = settings.proxy_user_header.lower()
573576
if proxy in _LOOPBACK_SKIP_HEADERS:
@@ -787,6 +790,13 @@ def safe_extract_headers_for_loopback(request_headers: Dict[str, str], transport
787790
Wraps :func:`extract_headers_for_loopback` so that SSE / WebSocket setup
788791
is never blocked by passthrough configuration issues. ``ImportError``
789792
propagates (broken deployment should fail loudly).
793+
794+
Args:
795+
request_headers: Incoming HTTP headers to extract from.
796+
transport_name: Label for warning logs on failure.
797+
798+
Returns:
799+
Dict[str, str]: Extracted passthrough headers, or empty dict on error.
790800
"""
791801
try:
792802
return extract_headers_for_loopback(request_headers)
@@ -801,6 +811,12 @@ def safe_extract_and_filter_for_loopback(request_headers: Dict[str, str]) -> Dic
801811
Combines :func:`extract_headers_for_loopback` and
802812
:func:`filter_loopback_skip_headers` with error handling so that
803813
Streamable HTTP affinity loopback calls degrade gracefully.
814+
815+
Args:
816+
request_headers: Incoming HTTP headers to extract and filter.
817+
818+
Returns:
819+
Dict[str, str]: Filtered passthrough headers, or empty dict on error.
804820
"""
805821
try:
806822
return filter_loopback_skip_headers(extract_headers_for_loopback(request_headers))

tests/unit/mcpgateway/routers/test_teams.py

Lines changed: 118 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ async def test_get_team_access_denied(self, mock_user_context, mock_db, mock_tea
433433
async def test_update_team_success(self, mock_user_context, mock_db, mock_team):
434434
"""Test updating a team successfully."""
435435
team_id = mock_team.id
436-
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=200)
436+
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=50)
437437

438438
with patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
439439
mock_service = AsyncMock(spec=TeamManagementService)
@@ -453,7 +453,7 @@ async def test_update_team_success(self, mock_user_context, mock_db, mock_team):
453453
async def test_update_team_insufficient_permissions(self, mock_user_context, mock_db):
454454
"""Test updating a team without owner permissions."""
455455
team_id = str(uuid4())
456-
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=200)
456+
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=50)
457457

458458
with patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
459459
mock_service = AsyncMock(spec=TeamManagementService)
@@ -472,7 +472,7 @@ async def test_update_team_insufficient_permissions(self, mock_user_context, moc
472472
async def test_update_team_not_found(self, mock_user_context, mock_db):
473473
"""Test updating a non-existent team."""
474474
team_id = str(uuid4())
475-
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=200)
475+
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=50)
476476

477477
with patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
478478
mock_service = AsyncMock(spec=TeamManagementService)
@@ -1126,6 +1126,121 @@ async def test_invitation_with_value_error(self, mock_user_context, mock_db):
11261126
assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
11271127
assert "Invalid email format" in str(exc_info.value.detail)
11281128

1129+
# =========================================================================
1130+
# max_members Enforcement Tests
1131+
# =========================================================================
1132+
1133+
@pytest.mark.asyncio
1134+
async def test_create_team_non_admin_max_members_too_high(self, mock_user_context, mock_db):
1135+
"""Non-admin users cannot set max_members above the configured limit."""
1136+
request = TeamCreateRequest(name="Test Team", description="desc", visibility="private", max_members=9999)
1137+
1138+
with patch("mcpgateway.routers.teams.settings") as mock_settings:
1139+
mock_settings.allow_team_creation = True
1140+
mock_settings.max_members_per_team = 100
1141+
1142+
from mcpgateway.routers.teams import create_team
1143+
1144+
with pytest.raises(HTTPException) as exc_info:
1145+
await create_team(request, current_user_ctx=mock_user_context, db=mock_db)
1146+
1147+
assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
1148+
assert "max_members cannot exceed 100" in str(exc_info.value.detail)
1149+
1150+
@pytest.mark.asyncio
1151+
async def test_create_team_non_admin_max_members_at_limit(self, mock_user_context, mock_team, mock_db):
1152+
"""Non-admin users can set max_members exactly at the configured limit."""
1153+
request = TeamCreateRequest(name="Test Team", description="desc", visibility="private", max_members=100)
1154+
1155+
with patch("mcpgateway.routers.teams.settings") as mock_settings, patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
1156+
mock_settings.allow_team_creation = True
1157+
mock_settings.max_members_per_team = 100
1158+
mock_service = AsyncMock(spec=TeamManagementService)
1159+
mock_service.create_team = AsyncMock(return_value=mock_team)
1160+
MockService.return_value = mock_service
1161+
1162+
from mcpgateway.routers.teams import create_team
1163+
1164+
result = await create_team(request, current_user_ctx=mock_user_context, db=mock_db)
1165+
assert result.id == mock_team.id
1166+
1167+
@pytest.mark.asyncio
1168+
async def test_create_team_admin_can_exceed_max_members(self, mock_admin_context, mock_team, mock_db):
1169+
"""Admin users can set max_members above the configured limit."""
1170+
request = TeamCreateRequest(name="Test Team", description="desc", visibility="private", max_members=9999)
1171+
1172+
with patch("mcpgateway.routers.teams.settings") as mock_settings, patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
1173+
mock_settings.allow_team_creation = True
1174+
mock_settings.max_members_per_team = 100
1175+
mock_service = AsyncMock(spec=TeamManagementService)
1176+
mock_service.create_team = AsyncMock(return_value=mock_team)
1177+
MockService.return_value = mock_service
1178+
1179+
from mcpgateway.routers.teams import create_team
1180+
1181+
result = await create_team(request, current_user_ctx=mock_admin_context, db=mock_db)
1182+
assert result.id == mock_team.id
1183+
mock_service.create_team.assert_called_once()
1184+
1185+
@pytest.mark.asyncio
1186+
async def test_update_team_non_admin_max_members_too_high(self, mock_user_context, mock_db):
1187+
"""Non-admin users cannot set max_members above the configured limit on update."""
1188+
team_id = str(uuid4())
1189+
request = TeamUpdateRequest(max_members=9999)
1190+
1191+
with patch("mcpgateway.routers.teams.settings") as mock_settings, patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
1192+
mock_settings.max_members_per_team = 100
1193+
mock_service = AsyncMock(spec=TeamManagementService)
1194+
mock_service.get_user_role_in_team = AsyncMock(return_value="owner")
1195+
MockService.return_value = mock_service
1196+
1197+
from mcpgateway.routers.teams import update_team
1198+
1199+
with pytest.raises(HTTPException) as exc_info:
1200+
await update_team(team_id, request, current_user=mock_user_context, db=mock_db)
1201+
1202+
assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
1203+
assert "max_members cannot exceed 100" in str(exc_info.value.detail)
1204+
1205+
@pytest.mark.asyncio
1206+
async def test_update_team_non_admin_max_members_at_limit(self, mock_user_context, mock_team, mock_db):
1207+
"""Non-admin users can set max_members exactly at the configured limit on update."""
1208+
team_id = str(uuid4())
1209+
request = TeamUpdateRequest(max_members=100)
1210+
1211+
with patch("mcpgateway.routers.teams.settings") as mock_settings, patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
1212+
mock_settings.max_members_per_team = 100
1213+
mock_service = AsyncMock(spec=TeamManagementService)
1214+
mock_service.get_user_role_in_team = AsyncMock(return_value="owner")
1215+
mock_service.update_team = AsyncMock(return_value=True)
1216+
mock_service.get_team_by_id = AsyncMock(return_value=mock_team)
1217+
MockService.return_value = mock_service
1218+
1219+
from mcpgateway.routers.teams import update_team
1220+
1221+
result = await update_team(team_id, request, current_user=mock_user_context, db=mock_db)
1222+
assert result.id == mock_team.id
1223+
1224+
@pytest.mark.asyncio
1225+
async def test_update_team_admin_can_exceed_max_members(self, mock_admin_context, mock_team, mock_db):
1226+
"""Admin users can set max_members above the configured limit on update."""
1227+
team_id = str(uuid4())
1228+
request = TeamUpdateRequest(max_members=9999)
1229+
1230+
with patch("mcpgateway.routers.teams.settings") as mock_settings, patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
1231+
mock_settings.max_members_per_team = 100
1232+
mock_service = AsyncMock(spec=TeamManagementService)
1233+
mock_service.get_user_role_in_team = AsyncMock(return_value="owner")
1234+
mock_service.update_team = AsyncMock(return_value=True)
1235+
mock_service.get_team_by_id = AsyncMock(return_value=mock_team)
1236+
MockService.return_value = mock_service
1237+
1238+
from mcpgateway.routers.teams import update_team
1239+
1240+
result = await update_team(team_id, request, current_user=mock_admin_context, db=mock_db)
1241+
assert result.id == mock_team.id
1242+
mock_service.update_team.assert_called_once()
1243+
11291244
@pytest.mark.skip(reason="RBAC mocking complex - functionality covered by test_teams_v2.py")
11301245
@pytest.mark.asyncio
11311246
async def test_member_operations_with_invalid_role(self, mock_user_context, mock_db):

tests/unit/mcpgateway/routers/test_teams_v2.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ async def test_get_team_not_found(self, mock_user_context, mock_db):
194194
async def test_update_team_success(self, mock_user_context, mock_db, mock_team):
195195
"""Test updating a team successfully."""
196196
team_id = mock_team.id
197-
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=200)
197+
request = TeamUpdateRequest(name="Updated Team", description="Updated description", visibility="public", max_members=100)
198198

199199
with patch("mcpgateway.routers.teams.TeamManagementService") as MockService:
200200
mock_service = AsyncMock(spec=TeamManagementService)

0 commit comments

Comments
 (0)