🔧 Group arguments by category in both Docker and run.py

jemrobinson · jemrobinson · commit 2d0867844443 · 2025-01-14T15:51:47.000Z
diff --git a/README.md b/README.md
@@ -142,6 +142,8 @@ The value of this attribute should be the same as the `--domain` argument to Apr
 Any users with this attribute missing or set to something else will be ignored by Apricot.
 This allows you to attach multiple Apricot servers to the same Keycloak instance, each with their own set of users.
 
+:exclamation: You can disable user domain verification with the `--disable-user-domain-verification` command line option :exclamation:
+
 #### Client application
 
 You will need to register an application to interact with `Keycloak`.
diff --git a/apricot/apricot_server.py b/apricot/apricot_server.py
@@ -28,6 +28,7 @@ def __init__(
         background_refresh: bool = False,
         debug: bool = False,
         enable_mirrored_groups: bool = True,
+        enable_user_domain_verification: bool = True,
         redis_host: str | None = None,
         redis_port: int | None = None,
         refresh_interval: int = 60,
@@ -45,7 +46,8 @@ def __init__(
         @param port: Port to expose LDAP on
         @param background_refresh: Whether to refresh the LDAP tree in the background
         @param debug: Enable debug output
-        @param enable_mirrored_groups: Create a mirrored LDAP group-of-groups for each group-of-users
+        @param enable_mirrored_groups: Whether to create a mirrored LDAP group-of-groups for each group-of-users
+        @param enable_user_domain_verification: Whether to verify users belong to the correct domain
         @param redis_host: Host for a Redis cache (if used)
         @param redis_port: Port for a Redis cache (if used)
         @param refresh_interval: Interval after which the LDAP information is stale
@@ -93,6 +95,7 @@ def __init__(
             domain,
             oauth_client,
             enable_mirrored_groups=enable_mirrored_groups,
+            enable_user_domain_verification=enable_user_domain_verification,
         )
 
         # Create an LDAPServerFactory
diff --git a/apricot/oauth/oauth_data_adaptor.py b/apricot/oauth/oauth_data_adaptor.py
@@ -32,18 +32,21 @@ def __init__(
         oauth_client: OAuthClient,
         *,
         enable_mirrored_groups: bool,
+        enable_user_domain_verification: bool,
     ) -> None:
         """Initialise an OAuthDataAdaptor.
 
         @param domain: The root domain of the LDAP tree
-        @param enable_mirrored_groups: Create a mirrored LDAP group-of-groups for each group-of-users
+        @param enable_mirrored_groups: Whether to create a mirrored LDAP group-of-groups for each group-of-users
+        @param enable_user_domain_verification: Whether to verify users belong to the correct domain
         @param oauth_client: An OAuth client used to construct the LDAP tree
         """
         self.debug = oauth_client.debug
         self.domain = domain
         self.oauth_client = oauth_client
         self.root_dn = "DC=" + domain.replace(".", ",DC=")
         self.enable_mirrored_groups = enable_mirrored_groups
+        self.enable_user_domain_verification = enable_user_domain_verification
 
     def _dn_from_group_cn(self: Self, group_cn: str) -> str:
         return f"CN={group_cn},OU=groups,{self.root_dn}"
@@ -187,7 +190,6 @@ def _validate_groups(
     def _validate_users(
         self: Self,
         annotated_users: list[tuple[JSONDict, list[type[LDAPObjectClass]]]],
-        domain: str,
     ) -> list[LDAPAttributeAdaptor]:
         """Return a list of LDAPAttributeAdaptors representing validated user data."""
         if self.debug:
@@ -196,18 +198,23 @@ def _validate_users(
         for user_dict, required_classes in annotated_users:
             name = user_dict.get("cn", "unknown")
             try:
-                if (user_domain := user_dict.get("domain", None)) == domain:
-                    output.append(
-                        LDAPAttributeAdaptor.from_attributes(
-                            user_dict,
-                            required_classes=required_classes,
-                        ),
-                    )
-                else:
+                # Verify user domain if enabled
+                if (
+                    self.enable_user_domain_verification
+                    and (user_domain := user_dict.get("domain", None)) != self.domain
+                ):
                     log.msg(f"... user '{name}' failed validation.")
                     log.msg(
-                        f" -> 'domain': expected '{domain}' but '{user_domain}' was provided.",
+                        f" -> 'domain': expected '{self.domain}' but '{user_domain}' was provided.",
                     )
+                    continue
+                # Construct an LDAPAttributeAdaptor from the user attributes
+                output.append(
+                    LDAPAttributeAdaptor.from_attributes(
+                        user_dict,
+                        required_classes=required_classes,
+                    ),
+                )
             except ValidationError as exc:
                 log.msg(f"... user '{name}' failed validation.")
                 for error in exc.errors():
@@ -222,7 +229,7 @@ def retrieve_all(
         """Retrieve and return validated user and group information."""
         annotated_groups, annotated_users = self._retrieve_entries()
         validated_groups = self._validate_groups(annotated_groups)
-        validated_users = self._validate_users(annotated_users, self.domain)
+        validated_users = self._validate_users(annotated_users)
         if self.debug:
             log.msg(
                 f"Validated {len(validated_groups)} groups and {len(validated_users)} users.",
diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
@@ -2,7 +2,37 @@
 # shellcheck disable=SC2086
 # shellcheck disable=SC2089
 
-# Required arguments
+# Optional arguments
+EXTRA_OPTS=""
+
+
+# Common server-level options
+if [ -z "${PORT}" ]; then
+    PORT="1389"
+    echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] PORT environment variable is not set: using default of '${PORT}'"
+fi
+
+if [ -n "${DEBUG}" ]; then
+    EXTRA_OPTS="${EXTRA_OPTS} --debug"
+fi
+
+
+# LDAP tree arguments
+if [ -z "${DOMAIN}" ]; then
+    echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] DOMAIN environment variable is not set"
+    exit 1
+fi
+
+if [ -n "${DISABLE_MIRRORED_GROUPS}" ]; then
+    EXTRA_OPTS="${EXTRA_OPTS} --disable-mirrored-groups"
+fi
+
+if [ -n "${DISABLE_USER_DOMAIN_VERIFICATION}" ]; then
+    EXTRA_OPTS="${EXTRA_OPTS} --disable-user-domain-verification"
+fi
+
+
+# OAuth client arguments
 if [ -z "${BACKEND}" ]; then
     echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] BACKEND environment variable is not set"
     exit 1
@@ -18,27 +48,14 @@ if [ -z "${CLIENT_SECRET}" ]; then
     exit 1
 fi
 
-if [ -z "${DOMAIN}" ]; then
-    echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] DOMAIN environment variable is not set"
-    exit 1
-fi
-
-
-# Arguments with defaults
-if [ -z "${PORT}" ]; then
-    PORT="1389"
-    echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] PORT environment variable is not set: using default of '${PORT}'"
-fi
-
 
-# Optional arguments
-EXTRA_OPTS=""
-if [ -n "${DEBUG}" ]; then
-    EXTRA_OPTS="${EXTRA_OPTS} --debug"
+# LDAP refresh arguments
+if [ -n "${BACKGROUND_REFRESH}" ]; then
+    EXTRA_OPTS="${EXTRA_OPTS} --background-refresh"
 fi
 
-if [ -n "${DISABLE_MIRRORED_GROUPS}" ]; then
-    EXTRA_OPTS="${EXTRA_OPTS} --disable-mirrored-groups"
+if [ -n "${REFRESH_INTERVAL}" ]; then
+    EXTRA_OPTS="${EXTRA_OPTS} --refresh-interval $REFRESH_INTERVAL"
 fi
 
 
@@ -61,13 +78,13 @@ if [ -n "${KEYCLOAK_DOMAIN_ATTRIBUTE}" ]; then
 fi
 
 
-# LDAP refresh arguments
-if [ -n "${BACKGROUND_REFRESH}" ]; then
-    EXTRA_OPTS="${EXTRA_OPTS} --background-refresh"
-fi
-
-if [ -n "${REFRESH_INTERVAL}" ]; then
-    EXTRA_OPTS="${EXTRA_OPTS} --refresh-interval $REFRESH_INTERVAL"
+# Redis arguments
+if [ -n "${REDIS_HOST}" ]; then
+    if [ -z "${REDIS_PORT}" ]; then
+        REDIS_PORT="6379"
+        echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] REDIS_PORT environment variable is not set: using default of '${REDIS_PORT}'"
+    fi
+    EXTRA_OPTS="${EXTRA_OPTS} --redis-host $REDIS_HOST --redis-port $REDIS_PORT"
 fi
 
 
@@ -85,16 +102,6 @@ if [ -n "${TLS_PORT}" ]; then
 fi
 
 
-# Redis arguments
-if [ -n "${REDIS_HOST}" ]; then
-    if [ -z "${REDIS_PORT}" ]; then
-        REDIS_PORT="6379"
-        echo "$(date +'%Y-%m-%d %H:%M:%S+0000') [-] REDIS_PORT environment variable is not set: using default of '${REDIS_PORT}'"
-    fi
-    EXTRA_OPTS="${EXTRA_OPTS} --redis-host $REDIS_HOST --redis-port $REDIS_PORT"
-fi
-
-
 # Run the server
 hatch run python run.py \
     --backend "${BACKEND}" \
diff --git a/run.py b/run.py
@@ -10,68 +10,93 @@
             prog="Apricot",
             description="Apricot is a proxy for delegating LDAP requests to an OpenID Connect backend.",
         )
-        # Common options needed for all backends
+        # Common server-level options
         parser.add_argument(
-            "-b",
-            "--backend",
-            type=OAuthBackend,
-            help="Which OAuth backend to use.",
+            "-p",
+            "--port",
+            type=int,
+            default=1389,
+            help="Port to run on.",
         )
         parser.add_argument(
+            "--debug",
+            action="store_true",
+            help="Enable debug logging.",
+        )
+
+        # LDAP tree settings
+        ldap_group = parser.add_argument_group("LDAP tree settings")
+        ldap_group.add_argument(
             "-d",
             "--domain",
             type=str,
             help="Which domain users belong to.",
+            required=True,
         )
-        parser.add_argument("-i", "--client-id", type=str, help="OAuth client ID.")
-        parser.add_argument(
-            "-p",
-            "--port",
-            type=int,
-            default=1389,
-            help="Port to run on.",
+        ldap_group.add_argument(
+            "--disable-mirrored-groups",
+            action="store_false",
+            default=True,
+            dest="enable_mirrored_groups",
+            help="Disable creation of mirrored groups.",
         )
-        parser.add_argument(
+        ldap_group.add_argument(
+            "--disable-user-domain-verification",
+            action="store_false",
+            default=True,
+            dest="enable_user_domain_verification",
+            help="Disable check that users belong to the correct domain.",
+        )
+
+        # OAuth client settings
+        oauth_group = parser.add_argument_group("OAuth settings")
+        oauth_group.add_argument(
+            "-b",
+            "--backend",
+            type=OAuthBackend,
+            help="Which OAuth backend to use.",
+            required=True,
+        )
+        oauth_group.add_argument(
+            "-i",
+            "--client-id",
+            type=str,
+            help="OAuth client ID.",
+            required=True,
+        )
+        oauth_group.add_argument(
             "-s",
             "--client-secret",
             type=str,
             help="OAuth client secret.",
+            required=True,
         )
-        parser.add_argument(
+
+        # Options for refreshing the tree
+        refresh_group = parser.add_argument_group("Refresh settings")
+        refresh_group.add_argument(
             "--background-refresh",
             action="store_true",
             default=False,
             help="Refresh in the background instead of as needed per request",
         )
-        parser.add_argument(
-            "--debug",
-            action="store_true",
-            help="Enable debug logging.",
-        )
-        parser.add_argument(
-            "--disable-mirrored-groups",
-            action="store_false",
-            default=True,
-            dest="enable_mirrored_groups",
-            help="Disable creation of mirrored groups.",
-        )
-        parser.add_argument(
+        refresh_group.add_argument(
             "--refresh-interval",
             type=int,
             default=60,
             help="How often to refresh the database in seconds",
         )
 
         # Options for Microsoft Entra backend
-        entra_group = parser.add_argument_group("Microsoft Entra")
+        entra_group = parser.add_argument_group("Microsoft Entra backend")
         entra_group.add_argument(
             "--entra-tenant-id",
             type=str,
             help="Microsoft Entra tenant ID.",
         )
 
         # Options for Keycloak backend
-        keycloak_group = parser.add_argument_group("Keycloak")
+        keycloak_group = parser.add_argument_group("Keycloak backend")
         keycloak_group.add_argument(
             "--keycloak-base-url",
             type=str,
@@ -88,6 +113,7 @@
             default="domain",
             help="The attribute in Keycloak that contains the users' domain.",
         )
+
         # Options for Redis cache
         redis_group = parser.add_argument_group("Redis")
         redis_group.add_argument(
@@ -100,6 +126,7 @@
             type=int,
             help="Port for Redis server.",
         )
+
         # Options for TLS
         tls_group = parser.add_argument_group("TLS")
         tls_group.add_argument(
@@ -118,6 +145,7 @@
             type=str,
             help="Location of TLS private key (pem).",
         )
+
         # Parse arguments
         args = parser.parse_args()
 
