Merge pull request RedHatInsights#1050 from RedHatInsights/RHCLOUD-29641

RHCLOUD-29641: Validate Service Account usernames before requests

rbac/management/principal/it_service.py

@@ -35,6 +35,7 @@
 LOGGER = logging.getLogger(__name__)
 SERVICE_ACCOUNT_CLIENT_IDS_KEY = "service_account_client_ids"
 TYPE_SERVICE_ACCOUNT = "service-account"
+KEY_SERVICE_ACCOUNT = "service-account-"
 
 # IT path to fetch the service accounts.
 IT_PATH_GET_SERVICE_ACCOUNTS = "/service_accounts/v1"
@@ -191,7 +192,7 @@ def is_service_account_valid_by_username(self, user: User, service_account_usern
             return True
 
         if self.is_username_service_account(service_account_username):
-            client_id = service_account_username.replace("service-account-", "")
+            client_id = service_account_username.replace(KEY_SERVICE_ACCOUNT, "")
         else:
             client_id = service_account_username
 
@@ -376,25 +377,32 @@ def get_service_accounts_group(self, group: Group, user: User, options: dict = {
     @staticmethod
     def is_username_service_account(username: str) -> bool:
         """Check if the given username belongs to a service account."""
-        return username.startswith("service-account-")
+        starts_with = username.startswith(KEY_SERVICE_ACCOUNT)
+
+        # Validate the UUID for the ClientID reference
+        if starts_with:
+            try:
+                if username.count(KEY_SERVICE_ACCOUNT) != 1:
+                    raise ValueError
+
+                uuid.UUID(username.replace(KEY_SERVICE_ACCOUNT, ""))
+            except ValueError:
+                raise serializers.ValidationError({"detail": "Invalid format for a Service Account username"})
+
+        return starts_with
 
     @staticmethod
     def extract_client_id_service_account_username(username: str) -> uuid.UUID:
         """Extract the client ID from the service account's username."""
         # If it has the "service-account" prefix, we just need to strip it and return the rest of the username, which
         # contains the client ID. Else, we have just received the client ID.
-        try:
-            if ITService.is_username_service_account(username=username):
-                return uuid.UUID(username.replace("service-account-", ""))
-            else:
+        if ITService.is_username_service_account(username=username):
+            return uuid.UUID(username.replace(KEY_SERVICE_ACCOUNT, ""))
+        else:
+            try:
                 return uuid.UUID(username)
-        except ValueError:
-            raise serializers.ValidationError(
-                {
-                    "detail": "unable to extract the client ID from the service account's username because the"
-                    " provided UUID is invalid"
-                }
-            )
+            except ValueError:
+                raise serializers.ValidationError({"detail": "Invalid ClientId for a Service Account username"})
 
     def generate_service_accounts_report_in_group(self, group: Group, client_ids: set[str]) -> dict[str, bool]:
         """Check if the given service accounts are in the specified group."""

tests/management/principal/test_it_service.py

@@ -239,15 +239,28 @@ def test_extract_client_id_service_account_username(self) -> None:
             "the client ID was not correctly extracted from a full username",
         )
 
+        # Call the function under test with a username without client ID (UUID).
+        try:
+            self.assertFalse(ITService.extract_client_id_service_account_username(username="abcde"))
+            self.fail(
+                "when providing an invalid UUID as the client ID to be extracted, the function under test should raise an error"
+            )
+        except serializers.ValidationError as ve:
+            self.assertEqual(
+                "Invalid ClientId for a Service Account username",
+                str(ve.detail.get("detail")),
+                "unexpected error message when providing an invalid UUID as the client ID",
+            )
+
         # Call the function under test with an invalid username which contains a bad formed UUID.
         try:
-            ITService.extract_client_id_service_account_username(username="abcde")
+            ITService.extract_client_id_service_account_username(username="service-account-xxxxx")
             self.fail(
                 "when providing an invalid UUID as the client ID to be extracted, the function under test should raise an error"
             )
         except serializers.ValidationError as ve:
             self.assertEqual(
-                "unable to extract the client ID from the service account's username because the provided UUID is invalid",
+                "Invalid format for a Service Account username",
                 str(ve.detail.get("detail")),
                 "unexpected error message when providing an invalid UUID as the client ID",
             )