Exclude realm-specific clients from client checks (#22)

Exclude realm-specific clients from client checks. Fixes #16 #17

kcwarden/auditors/client/client_authentication_via_mtls_or_jwt_recommended.py

@@ -12,7 +12,12 @@ def should_consider_client(self, client) -> bool:
         # We are interested in clients that are:
         # - OIDC Clients
         # - Confidential Clients
-        return self.is_not_ignored(client) and client.is_oidc_client() and not client.is_public()
+        return (
+            self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
+            and client.is_oidc_client()
+            and not client.is_public()
+        )
 
     def client_does_not_use_mtls_or_jwt_auth(self, client) -> bool:
         # If the clientAuthenticatorType is client-secret, basic client secret authentication is used.

kcwarden/auditors/client/client_has_erroneously_configured_wildcard_uri.py

@@ -16,6 +16,7 @@ def should_consider_client(self, client) -> bool:
         # - At least one flow that uses the redirect_uri active
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and (client.has_standard_flow_enabled() or client.has_implicit_flow_enabled())
         )

kcwarden/auditors/client/client_has_undefined_base_domain_and_schema.py

@@ -17,6 +17,7 @@ def should_consider_client(self, client) -> bool:
         # TODO Are there more flows that use redirect_uri?
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and (client.has_standard_flow_enabled() or client.has_implicit_flow_enabled())
         )

kcwarden/auditors/client/client_must_not_use_unencrypted_nonlocal_redirect_uri.py

@@ -17,6 +17,7 @@ def should_consider_client(self, client) -> bool:
         # TODO Are there more flows that use redirect_uri?
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and (client.has_standard_flow_enabled() or client.has_implicit_flow_enabled())
         )

kcwarden/auditors/client/client_should_disable_implicit_grant_flow.py

@@ -11,7 +11,7 @@ class ClientShouldDisableImplicitGrantFlow(Auditor):
     def should_consider_client(self, client) -> bool:
         # We are interested in clients that are:
         # - OIDC Clients
-        return self.is_not_ignored(client) and client.is_oidc_client()
+        return self.is_not_ignored(client) and client.is_oidc_client() and not client.is_realm_specific_client()
 
     def client_uses_implicit_grant_flow(self, client) -> bool:
         # All clients that have implicit flow enabled are considered suspect

kcwarden/auditors/client/client_should_not_use_wildcard_redirect_uri.py

@@ -15,6 +15,7 @@ def should_consider_client(self, client) -> bool:
         # TODO Are there more flows that use redirect_uri?
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and (client.has_standard_flow_enabled() or client.has_implicit_flow_enabled())
         )

kcwarden/auditors/client/client_uses_custom_redirect_uri_scheme.py

@@ -17,6 +17,7 @@ def should_consider_client(self, client) -> bool:
         # TODO Are there more flows that use redirect_uri?
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and (client.has_standard_flow_enabled() or client.has_implicit_flow_enabled())
         )

kcwarden/auditors/client/client_with_default_offline_access_scope.py

@@ -12,7 +12,7 @@ class ClientWithDefaultOfflineAccessScope(Auditor):
     REFERENCE = ""
 
     def should_consider_client(self, client) -> bool:
-        return self.is_not_ignored(client)
+        return self.is_not_ignored(client) and not client.is_realm_specific_client()
 
     def client_can_generate_offline_tokens(self, client) -> bool:
         # Check if the "offline_access" scope is in the default scopes

kcwarden/auditors/client/client_with_full_scope_allowed.py

@@ -9,7 +9,11 @@ class ClientWithFullScopeAllowed(Auditor):
     REFERENCE = ""
 
     def should_consider_client(self, client) -> bool:
-        return self.is_not_ignored(client) and client.allows_user_authentication()
+        return (
+            self.is_not_ignored(client)
+            and client.allows_user_authentication()
+            and not client.is_realm_specific_client()
+        )
 
     def client_has_full_scope_allowed(self, client) -> bool:
         return client.has_full_scope_allowed()

kcwarden/auditors/client/client_with_optional_offline_access_scope.py

@@ -9,7 +9,7 @@ class ClientWithOptionalOfflineAccessScope(Auditor):
     REFERENCE = ""
 
     def should_consider_client(self, client) -> bool:
-        return self.is_not_ignored(client)
+        return self.is_not_ignored(client) and not client.is_realm_specific_client()
 
     def client_can_generate_offline_tokens(self, client) -> bool:
         # Check if the "offline_access" scope is in the optional scopes

kcwarden/auditors/client/client_with_service_account_and_other_flow_enabled.py

@@ -15,6 +15,7 @@ def should_consider_client(self, client) -> bool:
         # - Has service account
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and (not client.is_public())
             and client.has_service_account_enabled()

kcwarden/auditors/client/confidential_client_should_disable_direct_access_grants.py

@@ -12,7 +12,12 @@ def should_consider_client(self, client) -> bool:
         # We are interested in clients that are:
         # - OIDC clients
         # - Are confidential clients
-        return self.is_not_ignored(client) and client.is_oidc_client() and not client.is_public()
+        return (
+            self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
+            and client.is_oidc_client()
+            and not client.is_public()
+        )
 
     def client_uses_direct_access_grants(self, client) -> bool:
         # All clients with direct access grants should be reported

kcwarden/auditors/client/confidential_client_should_enforce_pkce.py

@@ -16,6 +16,7 @@ def should_consider_client(self, client) -> bool:
         return (
             self.is_not_ignored(client)
             and client.is_oidc_client()
+            and not client.is_realm_specific_client()
             and (not client.is_public())
             and client.has_standard_flow_enabled()
         )

kcwarden/auditors/client/public_client_should_disable_direct_access_grants.py

@@ -12,7 +12,12 @@ def should_consider_client(self, client) -> bool:
         # We are interested in clients that are:
         # - OIDC clients
         # - Are public clients
-        return self.is_not_ignored(client) and client.is_oidc_client() and client.is_public()
+        return (
+            self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
+            and client.is_oidc_client()
+            and client.is_public()
+        )
 
     def client_uses_direct_access_grants(self, client) -> bool:
         # All clients with direct access grants should be reported

kcwarden/auditors/client/public_clients_must_enforce_pkce.py

@@ -15,6 +15,7 @@ def should_consider_client(self, client) -> bool:
         # - Have the standard flow enabled
         return (
             self.is_not_ignored(client)
+            and not client.is_realm_specific_client()
             and client.is_oidc_client()
             and client.is_public()
             and client.has_standard_flow_enabled()

kcwarden/custom_types/keycloak_object.py

@@ -505,13 +505,29 @@ def get_service_account_name(self) -> str | None:
         return "service-account-" + self.get_client_id().lower()
 
     # More Specific Properties
+    def is_realm_specific_client(self) -> bool:
+        # Each realm in Keycloak will get a realm-specific client created in the
+        # master realm. This is used to hold realm-specific roles, like the user
+        # management permissions. These clients behave differently from other
+        # clients, so we need to exclude them from some of our standard checks.
+        return (
+            self.get_realm().get_name() == "master" and self.get_name().endswith("-realm") and "protocol" not in self._d
+        )
+
     def get_protocol(self) -> str:
         # Every client should have the "protocol" field set, but the "master-realm"
         # client in the "master" realm for some reason does not include this field.
         # This code works around that by returning openid-connect in that case.
-        if self.get_name() == "master-realm" and self.get_realm().get_name() == "master":
-            return "openid-connect"
-        return self._d["protocol"]
+        try:
+            return self._d["protocol"]
+        except KeyError:
+            # If the client is a realm-specific client, it for some reason does not
+            # have a "protocol" set. Return openid-connect anyway.
+            if self.is_realm_specific_client():
+                return "openid-connect"
+            # This case should never happen, so instead of blindly returning something,
+            # we'd like to know about it. Raise an exception.
+            raise RuntimeError("'protocol' field of Client {} is not set, aborting".format(self.get_name()))
 
     def is_oidc_client(self) -> bool:
         return self.get_protocol() == "openid-connect"
@@ -568,7 +584,6 @@ def is_default_keycloak_client(self) -> bool:
             "broker",
             "realm-management",
             "security-admin-console",
-            "master-realm",
         ]
 
     def allows_user_authentication(self) -> bool:

tests/auditors/client/test_client_with_default_offline_access_scope.py

@@ -79,6 +79,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client1.allows_user_authentication.return_value = True
         client1.get_attributes.return_value = {"use.refresh.tokens": "true"}
         client1.is_public.return_value = False
+        client1.is_realm_specific_client.return_value = False
 
         client2 = Mock()
         client2.get_default_client_scopes.return_value = []
@@ -89,6 +90,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client2.get_attributes.return_value = {"use.refresh.tokens": "false"}
         client2.is_public.return_value = True
         client2.allows_user_authentication.return_value = True
+        client2.is_realm_specific_client.return_value = False
 
         client3 = Mock()
         client3.get_default_client_scopes.return_value = ["offline_access"]
@@ -99,6 +101,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client3.get_attributes.return_value = {"use.refresh.tokens": "true"}
         client3.is_public.return_value = False
         client3.allows_user_authentication.return_value = True
+        client3.is_realm_specific_client.return_value = False
 
         auditor._DB.get_all_clients.return_value = [client1, client2, client3]
         results = list(auditor.audit())

tests/auditors/client/test_client_with_full_scope_allowed.py

@@ -58,16 +58,19 @@ def test_audit_function_multiple_clients(self, auditor):
         client1.has_full_scope_allowed.return_value = True
         client1.get_default_client_scopes.return_value = ["email"]
         client1.get_optional_client_scopes.return_value = ["profile"]
+        client1.is_realm_specific_client.return_value = False
 
         client2 = Mock()
         client2.has_full_scope_allowed.return_value = False
         client2.get_default_client_scopes.return_value = ["email", "profile"]
         client2.get_optional_client_scopes.return_value = ["address"]
+        client2.is_realm_specific_client.return_value = False
 
         client3 = Mock()
         client3.has_full_scope_allowed.return_value = True
         client3.get_default_client_scopes.return_value = ["offline_access"]
         client3.get_optional_client_scopes.return_value = []
+        client3.is_realm_specific_client.return_value = False
 
         auditor._DB.get_all_clients.return_value = [client1, client2, client3]
         results = list(auditor.audit())

tests/auditors/client/test_client_with_optional_offline_access_scope.py

@@ -76,6 +76,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client1.has_implicit_flow_enabled.return_value = False
         client1.get_attributes.return_value = {"use.refresh.tokens": "true"}
         client1.is_public.return_value = False
+        client1.is_realm_specific_client.return_value = False
 
         client2 = Mock()
         client2.get_optional_client_scopes.return_value = []
@@ -85,6 +86,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client2.has_implicit_flow_enabled.return_value = False
         client2.get_attributes.return_value = {"use.refresh.tokens": "false"}
         client2.is_public.return_value = True
+        client2.is_realm_specific_client.return_value = False
 
         client3 = Mock()
         client3.get_optional_client_scopes.return_value = ["offline_access"]
@@ -94,6 +96,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client3.has_implicit_flow_enabled.return_value = True
         client3.get_attributes.return_value = {"use.refresh.tokens": "true"}
         client3.is_public.return_value = False
+        client3.is_realm_specific_client.return_value = False
 
         auditor._DB.get_all_clients.return_value = [client1, client2, client3]
         results = list(auditor.audit())

tests/auditors/client/test_client_with_service_account_and_other_flow_enabled.py

@@ -90,6 +90,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client1.has_device_authorization_grant_flow_enabled.return_value = True
         client1.has_standard_flow_enabled.return_value = True
         client1.allows_user_authentication.return_value = True
+        client1.is_realm_specific_client.return_value = False
 
         client2 = Mock()
         client2.is_oidc_client.return_value = True
@@ -99,7 +100,8 @@ def test_audit_function_multiple_clients(self, auditor):
         client2.has_implicit_flow_enabled.return_value = False
         client2.has_device_authorization_grant_flow_enabled.return_value = False
         client2.has_standard_flow_enabled.return_value = False
-        client1.allows_user_authentication.return_value = False
+        client2.allows_user_authentication.return_value = False
+        client2.is_realm_specific_client.return_value = False
 
         client3 = Mock()
         client3.is_oidc_client.return_value = True
@@ -110,6 +112,7 @@ def test_audit_function_multiple_clients(self, auditor):
         client3.has_standard_flow_enabled.return_value = True
         client3.has_device_authorization_grant_flow_enabled.return_value = False
         client3.allows_user_authentication.return_value = True
+        client3.is_realm_specific_client.return_value = False
 
         auditor._DB.get_all_clients.return_value = [client1, client2, client3]
         results = list(auditor.audit())

tests/conftest.py

@@ -135,6 +135,7 @@ def mock_client(mock_realm):
     client.get_default_client_scopes.return_value = []
     client.get_optional_client_scopes.return_value = []
     client.is_oidc_client.return_value = True
+    client.is_realm_specific_client.return_value = False
     return client