diff --git a/changelogs/fragments/20240124-module_utils-elbv2-fix-issue-with-authenticated-oidc.yml b/changelogs/fragments/20240124-module_utils-elbv2-fix-issue-with-authenticated-oidc.yml new file mode 100644 index 00000000000..e7cb9c34b68 --- /dev/null +++ b/changelogs/fragments/20240124-module_utils-elbv2-fix-issue-with-authenticated-oidc.yml @@ -0,0 +1,6 @@ +--- +bugfixes: + - >- + module_utils/elbv2 - Fix issue when creating or modifying Loab balancer rule + type authenticate-oidc using ``ClientSecret`` parameter and + ``UseExistingClientSecret=true`` (https://github.com/ansible-collections/amazon.aws/issues/1877). diff --git a/plugins/module_utils/elbv2.py b/plugins/module_utils/elbv2.py index e8bff29a1fd..613db9965da 100644 --- a/plugins/module_utils/elbv2.py +++ b/plugins/module_utils/elbv2.py @@ -107,6 +107,12 @@ def _prune_secret(action): if action["AuthenticateOidcConfig"].get("UseExistingClientSecret", False): action["AuthenticateOidcConfig"].pop("ClientSecret", None) + if not action["AuthenticateOidcConfig"].get("OnUnauthenticatedRequest", False): + action["AuthenticateOidcConfig"]["OnUnauthenticatedRequest"] = "authenticate" + + if not action["AuthenticateOidcConfig"].get("SessionCookieName", False): + action["AuthenticateOidcConfig"]["SessionCookieName"] = "AWSELBAuthSessionCookie" + return action @@ -1010,7 +1016,7 @@ def __init__(self, connection, module, elb_arn, listener_rules, listener_port): # Get listener based on port so we can use ARN self.current_listener = get_elb_listener(connection, module, elb_arn, listener_port) - self.listener_arn = self.current_listener["ListenerArn"] + self.listener_arn = self.current_listener.get("ListenerArn") self.rules_to_add = deepcopy(self.rules) self.rules_to_modify = [] self.rules_to_delete = [] @@ -1141,8 +1147,9 @@ def _compare_rule(self, current_rule, new_rule): if len(current_rule["Actions"]) == len(new_rule["Actions"]): # if actions have just one element, compare the contents and then update if # they're different + copy_new_rule = deepcopy(new_rule) current_actions_sorted = _sort_actions(current_rule["Actions"]) - new_actions_sorted = _sort_actions(new_rule["Actions"]) + new_actions_sorted = _sort_actions(copy_new_rule["Actions"]) new_current_actions_sorted = [_append_use_existing_client_secretn(i) for i in current_actions_sorted] new_actions_sorted_no_secret = [_prune_secret(i) for i in new_actions_sorted] @@ -1193,7 +1200,7 @@ def compare_rules(self): break # If the current rule was not matched against passed rules, mark for removal - if not current_rule_passed_to_module and not current_rule["IsDefault"]: + if not current_rule_passed_to_module and not current_rule.get("IsDefault", False): rules_to_delete.append(current_rule["RuleArn"]) return rules_to_add, rules_to_modify, rules_to_delete @@ -1217,6 +1224,9 @@ def create(self): try: self.rule["ListenerArn"] = self.listener_arn self.rule["Priority"] = int(self.rule["Priority"]) + for action in self.rule.get("Actions", []): + if action.get("AuthenticateOidcConfig", {}).get("UseExistingClientSecret", False): + action["AuthenticateOidcConfig"]["UseExistingClientSecret"] = False AWSRetry.jittered_backoff()(self.connection.create_rule)(**self.rule) except (BotoCoreError, ClientError) as e: self.module.fail_json_aws(e) @@ -1232,6 +1242,10 @@ def modify(self): try: del self.rule["Priority"] + for action in self.rule.get("Actions", []): + if action.get("AuthenticateOidcConfig", {}).get("ClientSecret", False): + # You cannot both specify a client secret and set UseExistingClientSecret to true + action["AuthenticateOidcConfig"]["UseExistingClientSecret"] = False AWSRetry.jittered_backoff()(self.connection.modify_rule)(**self.rule) except (BotoCoreError, ClientError) as e: self.module.fail_json_aws(e) diff --git a/tests/unit/module_utils/elbv2/test_listener_rules.py b/tests/unit/module_utils/elbv2/test_listener_rules.py new file mode 100644 index 00000000000..37fa86d69c0 --- /dev/null +++ b/tests/unit/module_utils/elbv2/test_listener_rules.py @@ -0,0 +1,381 @@ +# +# (c) 2024 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from unittest.mock import MagicMock + +import pytest + +from ansible_collections.amazon.aws.plugins.module_utils import elbv2 + +example_arn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/nlb-123456789abc/abcdef0123456789" +example_arn2 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/nlb-0123456789ab/0123456789abcdef" + + +test_rules = [ + ( + { + "Actions": [ + { + "AuthenticateOidcConfig": { + "AuthorizationEndpoint": "https://samples.auth0.com/authorize", + "ClientId": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "Issuer": "https://samples.auth0.com", + "Scope": "openid", + "SessionTimeout": 604800, + "TokenEndpoint": "https://samples.auth0.com/oauth/token", + "UseExistingClientSecret": True, + "UserInfoEndpoint": "https://samples.auth0.com/userinfo", + "OnUnauthenticatedRequest": "authenticate", + "SessionCookieName": "AWSELBAuthSessionCookie", + }, + "Order": 1, + "Type": "authenticate-oidc", + } + ], + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Priority": 2, + }, + { + "Actions": [ + { + "AuthenticateOidcConfig": { + "AuthorizationEndpoint": "https://samples.auth0.com/authorize", + "ClientId": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "Issuer": "https://samples.auth0.com", + "Scope": "openid", + "SessionTimeout": 604800, + "TokenEndpoint": "https://samples.auth0.com/oauth/token", + "UseExistingClientSecret": True, + "UserInfoEndpoint": "https://samples.auth0.com/userinfo", + }, + "Order": 1, + "Type": "authenticate-oidc", + } + ], + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Priority": 2, + }, + {}, + ), + ( + { + "Actions": [ + { + "AuthenticateOidcConfig": { + "AuthorizationEndpoint": "https://samples.auth0.com/authorize", + "ClientId": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "Issuer": "https://samples.auth0.com", + "Scope": "openid", + "SessionTimeout": 604800, + "TokenEndpoint": "https://samples.auth0.com/oauth/token", + "UseExistingClientSecret": True, + "UserInfoEndpoint": "https://samples.auth0.com/userinfo", + "OnUnauthenticatedRequest": "authenticate", + "SessionCookieName": "AWSELBAuthSessionCookie", + }, + "Order": 1, + "Type": "authenticate-oidc", + } + ], + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Priority": 2, + }, + { + "Actions": [ + { + "AuthenticateOidcConfig": { + "AuthorizationEndpoint": "https://samples.auth0.com/authorize", + "ClientId": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "Issuer": "https://samples.auth0.com", + "Scope": "openid", + "SessionTimeout": 604800, + "TokenEndpoint": "https://samples.auth0.com/oauth/token", + "UseExistingClientSecret": True, + "UserInfoEndpoint": "https://samples.auth0.com/userinfo", + "OnUnauthenticatedRequest": "deny", + }, + "Order": 1, + "Type": "authenticate-oidc", + } + ], + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Priority": 2, + }, + { + "Actions": [ + { + "AuthenticateOidcConfig": { + "AuthorizationEndpoint": "https://samples.auth0.com/authorize", + "ClientId": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "Issuer": "https://samples.auth0.com", + "Scope": "openid", + "SessionTimeout": 604800, + "TokenEndpoint": "https://samples.auth0.com/oauth/token", + "UseExistingClientSecret": True, + "UserInfoEndpoint": "https://samples.auth0.com/userinfo", + "OnUnauthenticatedRequest": "deny", + }, + "Order": 1, + "Type": "authenticate-oidc", + } + ], + }, + ), + ( + { + "Actions": [{"TargetGroupName": "my_target_group", "Type": "forward"}], + "Conditions": [{"Field": "path-pattern", "Values": ["/test", "/prod"]}], + "Priority": 2, + }, + { + "Actions": [{"TargetGroupName": "my_target_group", "Type": "forward"}], + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Priority": 2, + }, + { + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + }, + ), +] + + +@pytest.mark.parametrize("current_rule,new_rule,modified_rule", test_rules) +def test__compare_rule(mocker, current_rule, new_rule, modified_rule): + mocker.patch( + "ansible_collections.amazon.aws.plugins.module_utils.elbv2.ELBListenerRules._get_elb_listener_rules" + ).return_value = MagicMock() + mocker.patch( + "ansible_collections.amazon.aws.plugins.module_utils.elbv2.get_elb_listener" + ).return_value = MagicMock() + module = MagicMock() + connection = MagicMock() + elb_arn = MagicMock() + + elb_listener_rules = elbv2.ELBListenerRules(connection, module, elb_arn, [], []) + + assert modified_rule == elb_listener_rules._compare_rule(current_rule, new_rule) + + +test_listeners_rules = [ + ( + [ + { + "Priority": "1", + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/abc", + }, + { + "Priority": "2", + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/123", + }, + ], + [ + { + "Priority": 2, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + }, + { + "Priority": 1, + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + }, + ], + {}, + ), + ( + [ + { + "Priority": "2", + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/abc", + }, + { + "Priority": "1", + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/123", + }, + ], + [ + { + "Priority": 2, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + }, + { + "Priority": 1, + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + }, + ], + { + "to_modify": [ + { + "Priority": 2, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/abc", + }, + { + "Priority": 1, + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/123", + }, + ] + }, + ), + ( + [ + { + "Priority": "1", + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/abc", + }, + ], + [ + { + "Priority": 1, + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + }, + { + "Priority": 2, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + }, + ], + { + "to_add": [ + { + "Priority": 2, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + }, + ] + }, + ), + ( + [ + { + "Priority": "1", + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/abc", + }, + { + "Priority": "2", + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/123", + }, + ], + [ + { + "Priority": 1, + "Conditions": [{"Field": "host-header", "Values": ["bla.tld"]}], + "Actions": [{"TargetGroupName": "target1", "Type": "forward"}], + }, + { + "Priority": 2, + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Actions": [ + {"TargetGroupName": "oidc-target-01", "Type": "forward", "Order": 2}, + { + "Type": "authenticate-oidc", + "Order": 1, + "AuthenticateOidcConfig": { + "Issuer": "https://sample.oauth.com/issuer", + "AuthorizationEndpoint": "https://sample.oauth.com", + "TokenEndpoint": "https://sample.oauth.com/oauth/token", + "UserInfoEndpoint": "https://sample.oauth.com/userinfo", + "ClientId": "id123645", + "ClientSecret": "testSecret123!@#$", + "UseExistingClientSecret": True, + }, + }, + ], + }, + { + "Priority": 3, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + }, + ], + { + "to_modify": [ + { + "Priority": 2, + "Conditions": [{"Field": "path-pattern", "Values": ["/test"]}], + "Actions": [ + {"TargetGroupName": "oidc-target-01", "Type": "forward", "Order": 2}, + { + "Type": "authenticate-oidc", + "Order": 1, + "AuthenticateOidcConfig": { + "Issuer": "https://sample.oauth.com/issuer", + "AuthorizationEndpoint": "https://sample.oauth.com", + "TokenEndpoint": "https://sample.oauth.com/oauth/token", + "UserInfoEndpoint": "https://sample.oauth.com/userinfo", + "ClientId": "id123645", + "ClientSecret": "testSecret123!@#$", + "UseExistingClientSecret": True, + }, + }, + ], + "RuleArn": "arn:aws:elasticloadbalancing:::listener-rule/app/ansible-test/123", + }, + ], + "to_add": [ + { + "Priority": 3, + "Conditions": [{"Field": "host-header", "Values": ["yolo.rocks"]}], + "Actions": [{"TargetGroupName": "target2", "Type": "forward"}], + } + ], + }, + ), +] + + +@pytest.mark.parametrize("current_rules,rules,expected", test_listeners_rules) +def test_compare_rules(mocker, current_rules, rules, expected): + mocker.patch( + "ansible_collections.amazon.aws.plugins.module_utils.elbv2.get_elb_listener" + ).return_value = MagicMock() + mocker.patch( + "ansible_collections.amazon.aws.plugins.module_utils.elbv2.ELBListenerRules._ensure_rules_action_has_arn" + ).return_value = rules + mocker.patch( + "ansible_collections.amazon.aws.plugins.module_utils.elbv2.ELBListenerRules._get_elb_listener_rules" + ).return_value = current_rules + module = MagicMock() + connection = MagicMock() + elb_arn = MagicMock() + + elb_listener_rules = elbv2.ELBListenerRules(connection, module, elb_arn, rules, 8009) + elb_listener_rules.current_rules = current_rules + rules_to_add, rules_to_modify, rules_to_delete = elb_listener_rules.compare_rules() + + import json + + print("add => ", json.dumps(rules_to_add)) + print("modify => ", json.dumps(rules_to_modify)) + print("delete => ", json.dumps(rules_to_delete)) + + assert sorted(rules_to_add, key=lambda x: x.get("Priority", 0)) == sorted( + expected.get("to_add", []), key=lambda x: x.get("Priority", 0) + ) + assert sorted(rules_to_modify, key=lambda x: x.get("Priority", 0)) == sorted( + expected.get("to_modify", []), key=lambda x: x.get("Priority", 0) + ) + assert sorted(rules_to_delete) == sorted(expected.get("to_delete", [])) diff --git a/tests/unit/module_utils/elbv2/test_prune.py b/tests/unit/module_utils/elbv2/test_prune.py index 1586b4df971..96d1dbbc81c 100644 --- a/tests/unit/module_utils/elbv2/test_prune.py +++ b/tests/unit/module_utils/elbv2/test_prune.py @@ -153,6 +153,8 @@ Scope="openid", SessionTimeout=604800, UseExistingClientSecret=True, + OnUnauthenticatedRequest="authenticate", + SessionCookieName="AWSELBAuthSessionCookie", ), ) oidc_actions = [