fix: order of role and attr in saml user_flags (#7050)

https://issues.redhat.com/browse/AAP-51127

Co-authored-by: Peter Braun <pbraun@redhat.com>

awx/sso/tests/unit/test_saml_migrator.py

@@ -73,24 +73,24 @@ def test_get_controller_config_with_mapper(saml_config_user_flags_no_value):
         {
             'map_type': 'is_superuser',
             'role': None,
-            'name': 'Role-is_superuser-attr',
+            'name': 'Role-is_superuser',
             'organization': None,
             'team': None,
             'revoke': True,
             'order': 5,
             'authenticator': -1,
-            'triggers': {'attributes': {'friends': {}, 'join_condition': 'or'}},
+            'triggers': {'attributes': {'Role': {'in': ['wilma']}, 'join_condition': 'or'}},
         },
         {
             'map_type': 'is_superuser',
             'role': None,
-            'name': 'Role-is_superuser',
+            'name': 'Role-is_superuser-attr',
             'organization': None,
             'team': None,
             'revoke': True,
             'order': 6,
             'authenticator': -1,
-            'triggers': {'attributes': {'Role': {'in': ['wilma']}, 'join_condition': 'or'}},
+            'triggers': {'attributes': {'friends': {}, 'join_condition': 'or'}},
         },
     ]
     assert result[0]['team_mappers'] == expected_maps
@@ -153,28 +153,6 @@ def test_get_controller_config_with_roles(basic_saml_config):
             'triggers': {'attributes': {'group_name': {'in': ['developers']}, 'join_condition': 'or'}},
             'order': 4,
         },
-        {
-            'map_type': 'is_superuser',
-            'role': None,
-            'name': 'Role-is_superuser-attr',
-            'organization': None,
-            'team': None,
-            'revoke': False,
-            'order': 5,
-            'authenticator': -1,
-            'triggers': {'attributes': {'friends': {'in': ['barney', 'fred']}, 'join_condition': 'or'}},
-        },
-        {
-            'map_type': 'role',
-            'role': 'Platform Auditor',
-            'name': 'Role-Platform Auditor-attr',
-            'organization': None,
-            'team': None,
-            'revoke': True,
-            'order': 6,
-            'authenticator': -1,
-            'triggers': {'attributes': {'auditor': {'in': ['bamm-bamm']}, 'join_condition': 'or'}},
-        },
         {
             'map_type': 'is_superuser',
             'role': None,
@@ -182,7 +160,7 @@ def test_get_controller_config_with_roles(basic_saml_config):
             'organization': None,
             'team': None,
             'revoke': False,
-            'order': 7,
+            'order': 5,
             'authenticator': -1,
             'triggers': {'attributes': {'Role': {'in': ['wilma']}, 'join_condition': 'or'}},
         },
@@ -193,10 +171,32 @@ def test_get_controller_config_with_roles(basic_saml_config):
             'organization': None,
             'team': None,
             'revoke': True,
-            'order': 8,
+            'order': 6,
             'authenticator': -1,
             'triggers': {'attributes': {'Role': {'in': ['fred']}, 'join_condition': 'or'}},
         },
+        {
+            'map_type': 'is_superuser',
+            'role': None,
+            'name': 'Role-is_superuser-attr',
+            'organization': None,
+            'team': None,
+            'revoke': False,
+            'order': 7,
+            'authenticator': -1,
+            'triggers': {'attributes': {'friends': {'in': ['barney', 'fred']}, 'join_condition': 'or'}},
+        },
+        {
+            'map_type': 'role',
+            'role': 'Platform Auditor',
+            'name': 'Role-Platform Auditor-attr',
+            'organization': None,
+            'team': None,
+            'revoke': True,
+            'order': 8,
+            'authenticator': -1,
+            'triggers': {'attributes': {'auditor': {'in': ['bamm-bamm']}, 'join_condition': 'or'}},
+        },
         {
             'map_type': 'organization',
             'role': 'Organization Member',
@@ -223,9 +223,13 @@ def test_get_controller_config_with_roles(basic_saml_config):
 
     assert result[0]['team_mappers'] == expected_maps
     extra_data = result[0]['settings']['configuration']['EXTRA_DATA']
-    assert ['member-of', 'member-of'] in extra_data
-    assert ['admin-of', 'admin-of'] in extra_data
-    assert ['Role', 'Role'] in extra_data
-    assert ['auditor', 'auditor'] in extra_data
-    assert ['friends', 'friends'] in extra_data
-    assert ['group_name', 'group_name'] in extra_data
+    extra_data_items = [
+        ['member-of', 'member-of'],
+        ['admin-of', 'admin-of'],
+        ['Role', 'Role'],
+        ['friends', 'friends'],
+        ['group_name', 'group_name'],
+    ]
+    for item in extra_data_items:
+        assert item in extra_data
+        assert extra_data.count(item) == 1

awx/sso/utils/saml_migrator.py

@@ -48,7 +48,6 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
         super().__init__(*args, **kwargs)
         self.next_order = 1
         self.team_mappers = []
-        self.dynamic_extra_data = [["Role", "Role"]]
 
     def get_authenticator_type(self):
         """Get the human-readable authenticator type name."""
@@ -73,7 +72,8 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
         # Get org and team mappings using the new fallback functions
         org_map_value = self.get_social_org_map("SOCIAL_AUTH_SAML_ORGANIZATION_MAP")
         team_map_value = self.get_social_team_map("SOCIAL_AUTH_SAML_TEAM_MAP")
-        extra_data = getattr(settings, "SOCIAL_AUTH_SAML_EXTRA_DATA", None)
+        self.extra_data = getattr(settings, "SOCIAL_AUTH_SAML_EXTRA_DATA", [])
+        self._add_to_extra_data(['Role', 'Role'])
 
         support_contact = getattr(settings, "SOCIAL_AUTH_SAML_SUPPORT_CONTACT", {})
         technical_contact = getattr(settings, "SOCIAL_AUTH_SAML_TECHNICAL_CONTACT", {})
@@ -92,15 +92,10 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
         self.team_mappers, self.next_order = team_map_to_gateway_format(team_map_value, start_order=self.next_order)
 
         self._team_attr_to_gateway_format(saml_team_attr)
-        self._user_flags_by_attr_value_to_gateway_format(user_flags_by_attr)
         self._user_flags_by_role_to_gateway_format(user_flags_by_attr)
+        self._user_flags_by_attr_value_to_gateway_format(user_flags_by_attr)
         self._org_attr_to_gateway_format(org_attr)
 
-        if not extra_data:
-            extra_data = self.dynamic_extra_data
-        elif isinstance(extra_data, list):
-            extra_data.extend(self.dynamic_extra_data)
-
         for name, value in idps.items():
             config_data = {
                 "name": name,
@@ -126,7 +121,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
                     "SUPPORT_CONTACT": support_contact,
                     "SECURITY_CONFIG": security_config,
                     "SP_EXTRA": sp_extra,
-                    "EXTRA_DATA": extra_data,
+                    "EXTRA_DATA": self.extra_data,
                 },
             }
 
@@ -185,7 +180,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
             return
 
         revoke = saml_team_attr.get('remove', True)
-        self.dynamic_extra_data.extend([[saml_attr, saml_attr]])
+        self._add_to_extra_data([saml_attr, saml_attr])
 
         for item in saml_team_attr["team_org_map"]:
             team_list = item["team"]
@@ -251,7 +246,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
 
                 revoke = user_flags_by_attr.get(v['revoke'], True)
                 attr_name = user_flags_by_attr[k]
-                self.dynamic_extra_data.extend([[attr_name, attr_name]])
+                self._add_to_extra_data([attr_name, attr_name])
 
                 if v['role']:
                     name = f"Role-{v['role']}-attr"
@@ -285,7 +280,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
                 organization = "{% " + f"for_attr_value('{attr_name}')" + " %}"
                 revoke = org_attr.get(v['revoke'], True)
 
-                self.dynamic_extra_data.extend([[attr_name, attr_name]])
+                self._add_to_extra_data([attr_name, attr_name])
 
                 name = f"Role-{v['role']}-attr"
                 self.team_mappers.append(
@@ -307,3 +302,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator):
                     }
                 )
                 self.next_order += 1
+
+    def _add_to_extra_data(self, item: list):
+        if item not in self.extra_data:
+            self.extra_data.append(item)