Skip to content

Commit 83a837f

Browse files
celladorJenkins
authored andcommitted
Require a pick in the site auth "Use the following"
An empty explicit connection list is semantically "Disabled" behind a misleading label, so require at least one entry. CMK-37102 Change-Id: Ib715c8155e382c14868aa2f7ad61340ad6823a00
1 parent 0ad87e0 commit 83a837f

4 files changed

Lines changed: 34 additions & 1 deletion

File tree

cmk/gui/watolib/sites.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,11 @@ def _editable_connections_form_spec(
547547
title=Title("Authentication"),
548548
add_element_label=Label("Add connection"),
549549
editable_order=False,
550+
custom_validate=[
551+
not_empty(
552+
Message("Please add at least one connection or choose a different option.")
553+
)
554+
],
550555
)
551556

552557
@classmethod

cmk/update_config/plugins/actions/migrate_user_sync_to_auth_connections.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ def _derive_new_values(
134134
# "all", a remote like "disabled".
135135
return (all_value, "all") if is_central_site else ("disabled", "disabled")
136136
if isinstance(user_sync, tuple) and user_sync[0] == "list":
137-
conn_ids: list[str] = list(user_sync[1])
137+
if not (conn_ids := list(user_sync[1])):
138+
# An empty explicit list is semantically "disabled", and the site editor
139+
# now rejects an empty list.
140+
return "disabled", "disabled"
138141
auth_entries: list[AuthenticationConnectionEntry] = [
139142
("ldap", conn_id) for conn_id in conn_ids
140143
]

tests/unit/cmk/gui/watolib/test_sites.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import pytest
2424

2525
from cmk.ccc.site import omd_site, SiteId
26+
from cmk.gui.form_specs import get_visitor, RawDiskData, VisitorOptions
2627
from cmk.gui.form_specs.unstable.legacy_converter import (
2728
TransformDataForLegacyFormatOrRecomposeFunction,
2829
)
@@ -368,3 +369,21 @@ def test_editable_connections_form_spec_omits_saml_when_not_supported() -> None:
368369
"""Without distributed SAML support the nested "list" widget offers only the LDAP pick."""
369370
elements = _editable_connection_elements(saml_supported=False)
370371
assert [element.name for element in elements] == ["ldap"]
372+
373+
374+
def test_editable_connections_form_spec_rejects_empty_list(request_context: None) -> None:
375+
"""Choosing "Use the following" requires at least one connection entry —
376+
an empty list would be semantically "disabled" behind a misleading label."""
377+
visitor = get_visitor(
378+
SiteManagement._editable_connections_form_spec(
379+
ldap_choices=[("ldap_a", "LDAP A")], saml_choices=None
380+
),
381+
VisitorOptions(migrate_values=False, mask_values=False),
382+
)
383+
384+
validation_messages = visitor.validate(RawDiskData([]))
385+
assert [message.message for message in validation_messages] == [
386+
"Please add at least one connection or choose a different option."
387+
]
388+
389+
assert visitor.validate(RawDiskData([("ldap", "ldap_a")])) == []

tests/unit/cmk/update_config/plugins/actions/test_migrate_user_sync_to_auth_connections.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ def test_legacy_list_migrates_to_plain_lists() -> None:
6969
assert attr == ["ldap_a", "ldap_b"]
7070

7171

72+
def test_legacy_empty_list_disables_both_fields() -> None:
73+
auth, attr = _derive_new_values(("list", []), is_central_site=True, saml_supported=True)
74+
assert auth == "disabled"
75+
assert attr == "disabled"
76+
77+
7278
def test_legacy_none_on_central_disables_both_fields() -> None:
7379
"""Explicit ``user_sync = None`` was the legacy "Disable automatic user
7480
synchronization" choice; both new fields mirror it as explicitly

0 commit comments

Comments
 (0)