From c6906a3f97f776b7c6bf83ac51ee6f2456fa7ae5 Mon Sep 17 00:00:00 2001 From: Markus Bergholz Date: Thu, 23 Feb 2023 15:51:23 +0100 Subject: [PATCH] elbv2: respect UseExistingClientSecret (#1270) elbv2: respect UseExistingClientSecret SUMMARY Since amazon.aws 5.0.0, elb_application_lb runs into an exception, when using Type: authenticate-oidc in a rule, even when UseExistingClientSecret: True parameter is given. That works as expected with amazon.aws 4.x.x. The logic gets broken in #940 Basically AWS won't return both, UseExistingClientSecret and ClientSecret. But when requesting against boto3, both parameters are mutually exclusive! When the user set UseExistingClientSecret: True, the ClientSecret must be removed for the request. When the user does not set UseExistingClientSecret or set it to False, the UseExistingClientSecret must be included in the request. While diving deeper, I've noticed a basic change detection problem for default values, that are not requested, but AWS will return them. I've summerized it in #1284 However, this PR does not target #1284, it just fixes the exception and restores the functionality and hotfix the change-detection only for Type: authenticate-oidc. origin PR description The error was: botocore.errorfactory.InvalidLoadBalancerActionException: An error occurred (InvalidLoadBalancerAction) when calling the ModifyRule operation: You must either specify a client secret or set UseExistingClientSecret to true UseExistingClientSecret is not respected anymore since a.a 5 Introduced in #940 Furthermore, AWS returns also Scope and SessionTimeout parameters that are filled with default values if not requested. 'Scope': 'openid', 'SessionTimeout': 604800, That make the module always returns a change, if they are not requested. This fix does not break backwards compatibility, because the values are already set by aws, when not requested yet. ISSUE TYPE Bugfix Pull Request COMPONENT NAME plugins/module_utils/elbv2.yml ADDITIONAL INFORMATION - Conditions: - Field: host-header Values: - some.tld - Field: path-pattern Values: - "/admin/*" Actions: - Type: authenticate-oidc Order: 1 AuthenticateOidcConfig: Issuer: https://login.microsoftonline.com/32rw-ewad53te-ef/v2.0 AuthorizationEndpoint: https://login.microsoftonline.com/324re-dafs6-6tw/oauth2/v2.0/authorize TokenEndpoint: https://login.microsoftonline.com/432535ez-rfes-32543ter/oauth2/v2.0/token UserInfoEndpoint: https://graph.microsoft.com/oidc/userinfo ClientId: fasgd-235463-fsgd-243 ClientSecret: "{{ lookup('onepassword', 'some cool secret', vault='some important vault') }}" SessionCookieName: AWSELBAuthSessionCookie OnUnauthenticatedRequest: authenticate UseExistingClientSecret: True - TargetGroupName: "{{ some_tg }}" Type: forward Order: 2 Reviewed-by: Alina Buzachis Reviewed-by: Mark Chappell --- changelogs/fragments/1270-elbv2-fixes.yml | 3 +++ plugins/module_utils/elbv2.py | 28 ++++++++++++++++----- plugins/modules/elb_application_lb.py | 3 +++ tests/unit/module_utils/elbv2/test_prune.py | 6 +++++ 4 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/1270-elbv2-fixes.yml diff --git a/changelogs/fragments/1270-elbv2-fixes.yml b/changelogs/fragments/1270-elbv2-fixes.yml new file mode 100644 index 00000000000..5da7d386699 --- /dev/null +++ b/changelogs/fragments/1270-elbv2-fixes.yml @@ -0,0 +1,3 @@ +bugfixes: + - module_utils/elbv2 - respect ``UseExistingClientSecret`` parameter in ``authenticate-oidc`` rules (https://github.com/ansible-collections/amazon.aws/pull/1270). + - module_utils/elbv2 - fix change detection by adding default values for ``Scope`` and ``SessionTimeout`` parameters in ``authenticate-oidc`` rules (https://github.com/ansible-collections/amazon.aws/pull/1270). \ No newline at end of file diff --git a/plugins/module_utils/elbv2.py b/plugins/module_utils/elbv2.py index 7ab5c4c84fb..490c266d5c2 100644 --- a/plugins/module_utils/elbv2.py +++ b/plugins/module_utils/elbv2.py @@ -89,16 +89,31 @@ def _prune_ForwardConfig(action): return newAction -# the AWS api won't return the client secret, so we'll have to remove it -# or the module will always see the new and current actions as different -# and try to apply the same config +# remove the client secret if UseExistingClientSecret, because aws won't return it +# add default values when they are not requested def _prune_secret(action): if action['Type'] != 'authenticate-oidc': return action - action['AuthenticateOidcConfig'].pop('ClientSecret', None) + if not action['AuthenticateOidcConfig'].get('Scope', False): + action['AuthenticateOidcConfig']['Scope'] = 'openid' + + if not action['AuthenticateOidcConfig'].get('SessionTimeout', False): + action['AuthenticateOidcConfig']['SessionTimeout'] = 604800 + if action['AuthenticateOidcConfig'].get('UseExistingClientSecret', False): - action['AuthenticateOidcConfig'].pop('UseExistingClientSecret') + action['AuthenticateOidcConfig'].pop('ClientSecret', None) + + return action + + +# while AWS api also won't return UseExistingClientSecret key +# it must be added, because it's requested and compared +def _append_use_existing_client_secretn(action): + if action['Type'] != 'authenticate-oidc': + return action + + action['AuthenticateOidcConfig']['UseExistingClientSecret'] = True return action @@ -993,9 +1008,10 @@ def _compare_rule(self, current_rule, new_rule): current_actions_sorted = _sort_actions(current_rule['Actions']) new_actions_sorted = _sort_actions(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] - if [_prune_ForwardConfig(i) for i in current_actions_sorted] != [_prune_ForwardConfig(i) for i in new_actions_sorted_no_secret]: + if [_prune_ForwardConfig(i) for i in new_current_actions_sorted] != [_prune_ForwardConfig(i) for i in new_actions_sorted_no_secret]: modified_rule['Actions'] = new_rule['Actions'] # If the action lengths are different, then replace with the new actions else: diff --git a/plugins/modules/elb_application_lb.py b/plugins/modules/elb_application_lb.py index ffa2db034bc..fe95331b6fa 100644 --- a/plugins/modules/elb_application_lb.py +++ b/plugins/modules/elb_application_lb.py @@ -138,6 +138,9 @@ - A list of ALB Listener Rules. - 'For the complete documentation of possible Conditions and Actions please see the boto3 documentation:' - 'https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/elbv2.html#ElasticLoadBalancingv2.Client.create_rule' + - > + Keep in mind that AWS uses default values for parameters that are not requested. For example for I(Scope) + and I(SessionTimeout) when the action type is C(authenticate-oidc). suboptions: Conditions: type: list diff --git a/tests/unit/module_utils/elbv2/test_prune.py b/tests/unit/module_utils/elbv2/test_prune.py index 72d354ecd17..bcab94ae844 100644 --- a/tests/unit/module_utils/elbv2/test_prune.py +++ b/tests/unit/module_utils/elbv2/test_prune.py @@ -106,6 +106,9 @@ TokenEndpoint='https://idp.ansible.test/token', UserInfoEndpoint='https://idp.ansible.test/user', ClientId='ExampleClient', + Scope='openid', + SessionTimeout=604800, + UseExistingClientSecret=True, ), ) oidc_actions = [ @@ -118,6 +121,8 @@ UserInfoEndpoint='https://idp.ansible.test/user', ClientId='ExampleClient', UseExistingClientSecret=True, + Scope='openid', + SessionTimeout=604800 ), ), dict( @@ -129,6 +134,7 @@ UserInfoEndpoint='https://idp.ansible.test/user', ClientId='ExampleClient', ClientSecret='MyVerySecretString', + UseExistingClientSecret=True, ), ), ]