AAP-45047: Fix allow map to process grant case in create_claims

ansible_base/authentication/utils/claims.py

@@ -120,9 +120,8 @@ def create_claims(authenticator: Authenticator, username: str, attrs: dict, grou
         rule_responses.append({mpk: has_permission, 'enabled': auth_map.enabled})
 
         understood_map = False
-        if auth_map.map_type == 'allow' and not has_permission:
-            # If any rule does not allow we don't want to return this to true
-            access_allowed = False
+        if auth_map.map_type == 'allow':
+            access_allowed = has_permission
             understood_map = True
         elif auth_map.map_type == 'is_superuser':
             is_superuser = has_permission

test_app/tests/authentication/utils/test_claims.py

@@ -71,6 +71,18 @@
             [{1: False, 'enabled': True}],
             id="map_type 'allow' with trigger 'never' sets 'access_allowed' to False",
         ),
+        pytest.param(
+            {"always": {}},
+            "allow",
+            "",
+            {},
+            [],
+            True,
+            None,
+            {"team_membership": {}, "organization_membership": {}, 'rbac_roles': {'system': {'roles': {}}, 'organizations': {}}},
+            [{1: True, 'enabled': True}],
+            id="map_type 'allow' with trigger 'always' keeps 'access_allowed' True",
+        ),
         pytest.param(
             {"always": {}},
             "team",
@@ -274,6 +286,107 @@ def test_create_claims_bad_map_type_logged(
     f"Map type bad_map_type of rule {local_authenticator_map.name} does not know how to be processed" in logger.error.call_args
 
 
+@mock.patch("ansible_base.authentication.utils.claims.logger")
+def test_create_claims_allow_grant_no_error_logged(
+    logger,
+    local_authenticator_map,
+    shut_up_logging,
+):
+    """
+    Test that map_type 'allow' with a firing trigger does NOT log an error.
+    Regression test for AAP-45047.
+    """
+    local_authenticator_map.triggers = {"always": {}}
+    local_authenticator_map.map_type = "allow"
+    local_authenticator_map.save()
+
+    authenticator = local_authenticator_map.authenticator
+    claims.create_claims(authenticator, "username", {}, [])
+
+    logger.error.assert_not_called()
+
+
+def test_create_claims_deny_all_then_allow_override(
+    local_authenticator_map,
+    local_authenticator_map_1,
+):
+    """
+    Test that a later allow map can override a deny-all map.
+    This is the documented pattern equivalent to AUTH_LDAP_REQUIRE_GROUP.
+    Regression test for AAP-45394.
+    """
+    # Map 1 (order 1): deny all
+    local_authenticator_map.triggers = {"never": {}}
+    local_authenticator_map.map_type = "allow"
+    local_authenticator_map.order = 1
+    local_authenticator_map.save()
+
+    # Map 2 (order 2): allow override
+    local_authenticator_map_1.triggers = {"always": {}}
+    local_authenticator_map_1.map_type = "allow"
+    local_authenticator_map_1.order = 2
+    local_authenticator_map_1.save()
+
+    authenticator = local_authenticator_map.authenticator
+    res = claims.create_claims(authenticator, "username", {}, [])
+
+    assert res["access_allowed"] is True
+
+
+def test_create_claims_deny_all_not_overridden_without_match(
+    local_authenticator_map,
+    local_authenticator_map_1,
+):
+    """
+    Test that a deny-all is NOT overridden when the allow map trigger does not fire.
+    Ensures the fix does not accidentally grant access to non-matching users.
+    """
+    # Map 1 (order 1): deny all
+    local_authenticator_map.triggers = {"never": {}}
+    local_authenticator_map.map_type = "allow"
+    local_authenticator_map.order = 1
+    local_authenticator_map.save()
+
+    # Map 2 (order 2): allow only users in group "aap-users"
+    local_authenticator_map_1.triggers = {"groups": {"has_or": ["aap-users"]}}
+    local_authenticator_map_1.map_type = "allow"
+    local_authenticator_map_1.order = 2
+    local_authenticator_map_1.save()
+
+    authenticator = local_authenticator_map.authenticator
+    # User is NOT in the required group
+    res = claims.create_claims(authenticator, "username", {}, ["other-group"])
+
+    assert res["access_allowed"] is False
+
+
+def test_create_claims_deny_all_overridden_with_group_match(
+    local_authenticator_map,
+    local_authenticator_map_1,
+):
+    """
+    Test that a deny-all IS overridden when the user matches the allow map group trigger.
+    End-to-end test of the deny-all + group-based allow pattern.
+    """
+    # Map 1 (order 1): deny all
+    local_authenticator_map.triggers = {"never": {}}
+    local_authenticator_map.map_type = "allow"
+    local_authenticator_map.order = 1
+    local_authenticator_map.save()
+
+    # Map 2 (order 2): allow users in group "aap-users"
+    local_authenticator_map_1.triggers = {"groups": {"has_or": ["aap-users"]}}
+    local_authenticator_map_1.map_type = "allow"
+    local_authenticator_map_1.order = 2
+    local_authenticator_map_1.save()
+
+    authenticator = local_authenticator_map.authenticator
+    # User IS in the required group
+    res = claims.create_claims(authenticator, "username", {}, ["aap-users"])
+
+    assert res["access_allowed"] is True
+
+
 def test_create_claims_multiple_same_org(
     local_authenticator_map,
     local_authenticator_map_1,