add the 'user' as default value for the 'type' query parameter and 'G…

…ET /principals/' (RedHatInsights#1503)
petracihalova · Feb 11, 2025 · ca5763f · ca5763f
1 parent 55bae0f
commit ca5763f
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 19 deletions.
diff --git a/docs/source/specs/openapi.json b/docs/source/specs/openapi.json
@@ -645,7 +645,7 @@
             "in": "query",
             "name": "type",
             "required": false,
-            "description": "Parameter for selecting the type of principal to be returned.",
+            "description": "Parameter for selecting the type of principal to be returned. Defaults to 'user'.",
             "schema": {
               "type": "string",
               "enum": [

diff --git a/rbac/management/principal/view.py b/rbac/management/principal/view.py
@@ -42,7 +42,9 @@
 VALID_BOOLEAN_VALUE = ["true", "false"]
 USERNAME_ONLY_KEY = "username_only"
 PRINCIPAL_TYPE_KEY = "type"
-VALID_PRINCIPAL_TYPE_VALUE = ["service-account", "user"]
+USER_KEY = "user"
+SA_KEY = "service-account"
+VALID_PRINCIPAL_TYPE_VALUE = [SA_KEY, USER_KEY]
 
 
 class PrincipalView(APIView):
@@ -126,12 +128,12 @@ def get(self, request):
         # Attempt validating and obtaining the "principal type" query
         # parameter.
         principal_type = validate_and_get_key(
-            query_params, PRINCIPAL_TYPE_KEY, VALID_PRINCIPAL_TYPE_VALUE, required=False
+            query_params, PRINCIPAL_TYPE_KEY, VALID_PRINCIPAL_TYPE_VALUE, default_value=USER_KEY, required=False
         )
         options["principal_type"] = principal_type
 
         # Get either service accounts or user principals, depending on what the user specified.
-        if principal_type == "service-account":
+        if principal_type == SA_KEY:
             options["email"] = query_params.get(EMAIL_KEY)
             options["match_criteria"] = validate_and_get_key(
                 query_params, MATCH_CRITERIA_KEY, VALID_MATCH_VALUE, required=False
@@ -173,7 +175,7 @@ def get(self, request):
         response_data = {}
         if status_code == status.HTTP_200_OK:
             data = resp.get("data", [])
-            if principal_type == "service-account":
+            if principal_type == SA_KEY:
                 count = sa_count
             elif isinstance(data, dict):
                 count = data.get("userCount")

diff --git a/tests/management/principal/test_view.py b/tests/management/principal/test_view.py
@@ -99,7 +99,7 @@ def test_non_admin_can_read_principal_list_with_permissions(self, mock_request):
                 "status": "enabled",
                 "admin_only": "false",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer["org_id"],
         )
@@ -163,7 +163,7 @@ def test_read_principal_list_success(self, mock_request):
                 "status": "enabled",
                 "admin_only": "false",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -258,7 +258,7 @@ def test_check_principal_admin(self, mock_request):
                 "status": "enabled",
                 "admin_only": "false",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -301,7 +301,7 @@ def test_read_principal_filtered_list_success_without_cross_account_user(self, m
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
         )
         # Cross account user won't be returned.
@@ -339,7 +339,7 @@ def test_read_principal_filtered_list_success(self, mock_request):
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
         )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
@@ -373,7 +373,7 @@ def test_read_principal_partial_matching(self, mock_request):
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -408,7 +408,7 @@ def test_read_principal_multi_filter(self, mock_request):
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -478,7 +478,7 @@ def test_read_principal_list_account(self, mock_request):
                 "sort_order": "desc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
         )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
@@ -571,7 +571,7 @@ def test_read_principal_list_by_email(self, mock_request):
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -606,7 +606,7 @@ def test_read_users_of_desired_status(self, mock_request):
                 "status": "disabled",
                 "admin_only": "false",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -639,7 +639,7 @@ def test_principal_default_status_enabled(self, mock_request):
                 "admin_only": "false",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -672,7 +672,7 @@ def test_read_list_of_admins(self, mock_request):
                 "status": "enabled",
                 "admin_only": "true",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -731,7 +731,7 @@ def test_read_principal_list_by_email_partial_matching(self, mock_request):
                 "sort_order": "asc",
                 "status": "enabled",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )
@@ -780,7 +780,7 @@ def test_read_principal_users(self, mock_request):
                 "status": "enabled",
                 "admin_only": "false",
                 "username_only": "false",
-                "principal_type": None,
+                "principal_type": "user",
             },
             org_id=self.customer_data["org_id"],
         )