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

@@ -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).

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:

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

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,
         ),
     ),
 ]