Merge pull request lightspeed-core#1851 from major/fix/rh-identity-developer-subscription

RSPEED-3167: accept System identities with empty account_number

Author: tisnik

docs/auth/rh-identity.md

@@ -114,7 +114,13 @@ directly (e.g., Insights client, subscription-manager).
 
 **Identity extraction:**
 - `user_id`: From `identity.system.cn` (certificate Common Name)
-- `username`: From `identity.account_number`
+- `username`: From `identity.account_number` when present and non-empty,
+  otherwise falls back to `identity.system.cn`
+
+`org_id` is the required organizational identifier for System identities.
+`account_number` is optional: no-cost RHEL developer subscriptions send an
+empty `account_number` with a populated `org_id`. A System identity is rejected
+only when `org_id` or `system.cn` is empty or absent.
 
 **Header structure:**
 ```json
@@ -134,6 +140,22 @@ directly (e.g., Insights client, subscription-manager).
 }
 ```
 
+A developer-subscription System identity omits the account number:
+```json
+{
+  "identity": {
+    "account_number": "",
+    "org_id": "18939564",
+    "type": "System",
+    "auth_type": "cert-auth",
+    "system": {
+      "cn": "14b75b86-6f99-411d-b41d-f400268b5807",
+      "cert_type": "system"
+    }
+  }
+}
+```
+
 **Available System fields:**
 
 | Field | Type | Description |
@@ -147,8 +169,8 @@ Both identity types share these top-level fields:
 
 | Field | Type | Description |
 |-------|------|-------------|
-| `account_number` | string | Red Hat account number |
-| `org_id` | string | Organization ID |
+| `account_number` | string | Red Hat account number (optional for System identities; may be empty for developer subscriptions) |
+| `org_id` | string | Organization ID (required for System identities) |
 | `type` | string | Identity type: "User" or "System" |
 
 ## Entitlements
@@ -269,7 +291,7 @@ curl http://localhost:8080/v1/query \
 | 400 | Missing `username` in user | `{"detail": "Missing 'username' in user data"}` |
 | 400 | Missing `system` for System type | `{"detail": "Missing 'system' field for System type"}` |
 | 400 | Missing `cn` in system | `{"detail": "Missing 'cn' in system data"}` |
-| 400 | Missing `account_number` for System | `{"detail": "Missing 'account_number' for System type"}` |
+| 400 | Missing `org_id` for System | `{"detail": "Missing 'org_id' for System type"}` |
 | 400 | Unsupported identity type | `{"detail": "Unsupported identity type: X"}` |
 | 403 | Missing required entitlements | `{"detail": "Missing required entitlement: rhel"}` |
 

src/authentication/rh_identity.py

@@ -135,14 +135,23 @@ def _validate_system_fields(self, identity: dict) -> None:
         if "cn" not in system:
             logger.warning("Identity validation failed: missing 'cn' in system data")
             raise HTTPException(status_code=400, detail="Invalid identity data")
-        if "account_number" not in identity:
+        self._validate_string_field("cn", system["cn"])
+
+        # org_id is the required organizational identifier for System identities
+        # (per the canonical Red Hat identity spec). account_number is optional:
+        # no-cost RHEL developer subscriptions send an empty account_number.
+        org_id = identity.get("org_id")
+        if org_id is None or org_id == "":
             logger.warning(
-                "Identity validation failed: "
-                "missing 'account_number' for System type"
+                "Identity validation failed: missing 'org_id' for System type"
             )
             raise HTTPException(status_code=400, detail="Invalid identity data")
-        self._validate_string_field("cn", system["cn"])
-        self._validate_string_field("account_number", identity["account_number"])
+
+        # account_number is optional, but when present and non-empty it must be
+        # a well-formed string.
+        account_number = identity.get("account_number")
+        if account_number is not None and account_number != "":
+            self._validate_string_field("account_number", account_number)
 
     def _validate_string_field(
         self, field_name: str, value: Any, max_length: int = 256
@@ -207,14 +216,24 @@ def get_user_id(self) -> str:
     def get_username(self) -> str:
         """Extract username based on identity type.
 
+        For System identities the account_number is preferred when present and
+        non-empty, falling back to the system common name (cn). No-cost RHEL
+        developer subscriptions send an empty account_number, so the cn provides
+        a stable non-empty identifier in that case.
+
         Returns:
-            Username (user.username for User type, account_number for System type)
+            Username (user.username for User type; account_number or system.cn
+            for System type)
         """
         identity = self.identity_data["identity"]
 
         if self._get_identity_type() == "User":
             return identity["user"]["username"]
-        return identity["account_number"]
+
+        account_number = identity.get("account_number")
+        if account_number:
+            return account_number
+        return identity["system"]["cn"]
 
     def get_org_id(self) -> str:
         """Extract organization ID from identity data.

tests/unit/authentication/test_rh_identity.py

@@ -282,11 +282,22 @@ def test_validate_entitlements(
                 {"identity": {"type": "System", "org_id": "123", "system": {}}},
                 "Invalid identity data",
             ),
+            # System identity without org_id is rejected (org_id is required).
             (
                 {
                     "identity": {
                         "type": "System",
-                        "org_id": "123",
+                        "system": {"cn": "test"},
+                    }
+                },
+                "Invalid identity data",
+            ),
+            # System identity with empty org_id is rejected.
+            (
+                {
+                    "identity": {
+                        "type": "System",
+                        "org_id": "",
                         "system": {"cn": "test"},
                     }
                 },
@@ -719,9 +730,9 @@ def test_user_non_string_types_rejected(
             (("system", "cn"), True),
             (("system", "cn"), []),
             (("system", "cn"), {}),
-            (("account_number",), None),
+            # account_number is optional: None/absent is allowed, but a present,
+            # non-empty, non-string value is still rejected.
             (("account_number",), 12345),
-            (("account_number",), False),
             (("account_number",), []),
             (("account_number",), {}),
         ],
@@ -826,3 +837,89 @@ def test_valid_user_data_still_passes(self, user_identity_data: dict) -> None:
     def test_valid_system_data_still_passes(self, system_identity_data: dict) -> None:
         """Regression: valid System identity data passes validation."""
         RHIdentityData(system_identity_data)
+
+    @pytest.mark.parametrize("account_number", ["", None])
+    def test_system_empty_or_absent_account_number_accepted(
+        self, system_identity_data: dict, account_number: Optional[str]
+    ) -> None:
+        """Accept System identity with empty or absent account_number.
+
+        No-cost RHEL developer subscriptions send an empty account_number with a
+        populated org_id. These must authenticate successfully, falling back to
+        the system cn as the username.
+        """
+        identity = system_identity_data["identity"]
+        if account_number is None:
+            identity.pop("account_number", None)
+        else:
+            identity["account_number"] = account_number
+
+        rh_identity = RHIdentityData(system_identity_data)
+        # username falls back to the system cn when account_number is empty/absent
+        assert rh_identity.get_username() == identity["system"]["cn"]
+        assert rh_identity.get_user_id() == identity["system"]["cn"]
+
+    def test_system_developer_subscription_identity_accepted(self) -> None:
+        """Accept a real developer-subscription System identity end-to-end.
+
+        Mirrors the cert-auth payload sent by no-cost RHEL developer
+        subscriptions: empty account_number, populated org_id, valid system cn.
+        """
+        identity_data = {
+            "identity": {
+                "system": {
+                    "cert_type": "system",
+                    "cn": "14b75b86-6f99-411d-b41d-f400268b5807",
+                },
+                "auth_type": "cert-auth",
+                "account_number": "",
+                "type": "System",
+                "org_id": "18939564",
+                "internal": {
+                    "cross_access": False,
+                    "auth_time": 0,
+                    "org_id": "18939564",
+                },
+            }
+        }
+
+        rh_identity = RHIdentityData(identity_data)
+        assert rh_identity.get_username() == "14b75b86-6f99-411d-b41d-f400268b5807"
+        assert rh_identity.get_user_id() == "14b75b86-6f99-411d-b41d-f400268b5807"
+        assert rh_identity.get_org_id() == "18939564"
+
+    def test_system_present_account_number_unchanged(
+        self, system_identity_data: dict
+    ) -> None:
+        """Regression: System with a present, non-empty account_number is unchanged.
+
+        get_username() must still return the account_number when it is present
+        and non-empty.
+        """
+        rh_identity = RHIdentityData(system_identity_data)
+        assert rh_identity.get_username() == "123"
+
+    @pytest.mark.parametrize("org_id", ["", None])
+    def test_system_missing_org_id_rejected(
+        self, system_identity_data: dict, org_id: Optional[str]
+    ) -> None:
+        """Reject System identity when org_id is empty or absent.
+
+        org_id is the required organizational identifier for System identities.
+        """
+        identity = system_identity_data["identity"]
+        if org_id is None:
+            identity.pop("org_id", None)
+        else:
+            identity["org_id"] = org_id
+
+        with pytest.raises(HTTPException) as exc_info:
+            RHIdentityData(system_identity_data)
+        assert exc_info.value.status_code == 400
+
+    def test_system_empty_cn_rejected(self, system_identity_data: dict) -> None:
+        """Reject System identity when system cn is empty."""
+        system_identity_data["identity"]["system"]["cn"] = ""
+        with pytest.raises(HTTPException) as exc_info:
+            RHIdentityData(system_identity_data)
+        assert exc_info.value.status_code == 400