From 5c7b2dbe4b02331b2c5ca1b66ccd7411e1fabb11 Mon Sep 17 00:00:00 2001 From: yyuki Date: Mon, 22 Sep 2025 00:57:44 +0900 Subject: [PATCH 1/2] authentication/claims: fix per-attribute ANY semantics & empty attr handling When a user attribute has multiple values, _process_user_value now evaluates them with ANY semantics (short-circuit on first match) and joins the result once via has_access_with_join. Previously we folded each element into has_access on every iteration, which effectively behaved like an AND across values and produced false negatives. Also treat empty/falsey user_value as no match and log a debug line, then join False for the attribute. This aligns attribute-level behavior with expected trigger semantics. Tests: update the explicit 'and' + list case to expect ALLOW on single match; add regression cases for cross-attribute AND/OR (cn contains 'ldap', employeeType contains 'manager'). Signed-off-by: yyuki --- ansible_base/authentication/utils/claims.py | 11 +++++-- .../tests/authentication/utils/test_claims.py | 33 +++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/ansible_base/authentication/utils/claims.py b/ansible_base/authentication/utils/claims.py index 8f2e21413..bdf9d95bb 100644 --- a/ansible_base/authentication/utils/claims.py +++ b/ansible_base/authentication/utils/claims.py @@ -562,21 +562,28 @@ def _process_user_value( evaluate_fn = operators[operator] + if not user_value: + _prefixed_debug(map_id, tracking_id, f"Attr [{attribute}] present but empty, skipping") + return has_access_with_join(has_access, False, join_condition) + + per_attr_match = False for a_user_value in user_value: # Normalize user value for comparison user_str = f"{a_user_value}".casefold() if _is_case_insensitivity_enabled() else f"{a_user_value}" # Evaluate condition result = evaluate_fn(user_str, trigger_value) - has_access = has_access_with_join(has_access, result, join_condition) # Log result header = f"Attr [{attribute}] value [{user_str}]" message = _get_operator_messages(operator, result) _prefixed_debug(map_id, tracking_id, f"{header} {message} [{trigger_value}], {_result_suffix(result)}") - return has_access + if result: + per_attr_match = True + break + return has_access_with_join(has_access, per_attr_match, join_condition) def _result_suffix(result: bool) -> str: return "allowing" if result else "skipping" diff --git a/test_app/tests/authentication/utils/test_claims.py b/test_app/tests/authentication/utils/test_claims.py index 41658db57..994ba64ee 100644 --- a/test_app/tests/authentication/utils/test_claims.py +++ b/test_app/tests/authentication/utils/test_claims.py @@ -747,8 +747,8 @@ def test_has_access_with_join(current_access, new_access, condition, expected): {"email": {"equals": "foo@example.com"}, "join_condition": "and"}, {"email": ["bar@example.com", "foo@example.com"]}, False, - claims.TriggerResult.SKIP, - id="user attribute is list, one match, explicit 'and', negative", + claims.TriggerResult.ALLOW, + id="user attribute is list, one match, explicit 'and', positive", ), pytest.param( {"email": {"equals": "foo@example.com"}, "join_condition": "and"}, @@ -911,6 +911,35 @@ def test_has_access_with_join(current_access, new_access, condition, expected): claims.TriggerResult.ALLOW, id="all attribute required by 'and' condition should result in allow", ), + pytest.param( + { + "cn": {"contains": "ldap"}, + "employeeType": {"contains": "manager"}, + "join_condition": "and", + }, + {"cn": ["ldap_admin"], "employeeType": ["manager", "executor"]}, + False, + claims.TriggerResult.ALLOW, + id="regression_any_semantics_and_across_attrs_positive", + ), + pytest.param( + { + "cn": {"contains": "ldap"}, + "employeeType": {"contains": "manager"}, + "join_condition": "and", + }, + {"cn": ["ldap_admin"], "employeeType": ["executor"]}, + False, + claims.TriggerResult.SKIP, + id="regression_any_semantics_and_across_attrs_negative", + ), + pytest.param( + {"cn": {"contains": "ldap"}, "employeeType": {"contains": "manager"}, "join_condition": "or"}, + {"cn": ["zzz"], "employeeType": ["manager", "executor"]}, + False, + claims.TriggerResult.ALLOW, + id="regression_any_semantics_or_across_attrs_positive_one_side", + ), ], ) @pytest.mark.django_db From 6212372755b0a37a8b5d32ed6a1820d5889ed414 Mon Sep 17 00:00:00 2001 From: yyuki Date: Mon, 22 Sep 2025 10:56:37 +0900 Subject: [PATCH 2/2] Add optional-dependencies.dev in pyproject.toml --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 2a87db0bc..0fbdcb538 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,6 +59,8 @@ optional-dependencies.redis_client = { file = [ "requirements/requirements_redis optional-dependencies.oauth2_provider = { file = [ "requirements/requirements_oauth2_provider.in" ] } optional-dependencies.resource_registry = { file = [ "requirements/requirements_resource_registry.in" ] } optional-dependencies.feature_flags = { file = [ "requirements/requirements_feature_flags.in" ] } +optional-dependencies.dev = { file = [ "requirements/requirements_dev.txt" ] } + [build-system] requires = ["setuptools>=64", "setuptools_scm>=8"]