Fixed #19518 - added support for SASL LDAP

app/Console/Commands/LdapTroubleshooter.php

@@ -165,18 +165,27 @@ public function handle()
             }
             $output[] = 'ldapsearch';
             $output[] = '-H '.$settings->ldap_server;
-            $output[] = '-x';
             $output[] = '-b '.escapeshellarg($settings->ldap_basedn);
-            $output[] = '-D '.escapeshellarg($settings->ldap_uname);
 
-            try {
-                $w = Crypt::decrypt($settings->ldap_pword);
-            } catch (Exception $e) {
-                $this->warn('Could not decrypt password. This usually means an LDAP password was not set or the APP_KEY was changed since the LDAP pasword was last saved.  Aborting.');
-                exit(0);
+            if (Ldap::shouldUseSaslExternal($settings)) {
+                // SASL EXTERNAL identifies the client TLS
+                // cert loaded above (LDAPTLS_CERT / LDAPTLS_KEY) instead
+                // of a bind DN + password. See the ldapsearch man page's -Y flag.
+                $output[] = '-Y EXTERNAL';
+            } else {
+                $output[] = '-x';
+                $output[] = '-D '.escapeshellarg($settings->ldap_uname);
+
+                try {
+                    $w = Crypt::decrypt($settings->ldap_pword);
+                } catch (Exception $e) {
+                    $this->warn('Could not decrypt password. This usually means an LDAP password was not set or the APP_KEY was changed since the LDAP pasword was last saved.  Aborting.');
+                    exit(0);
+                }
+
+                $output[] = '-w '.escapeshellarg($w);
             }
 
-            $output[] = '-w '.escapeshellarg($w);
             $output[] = escapeshellarg(parenthesized_filter($settings->ldap_filter));
             if ($settings->ldap_tls) {
                 $this->line('# adding STARTTLS option');
@@ -386,6 +395,14 @@ public function handle()
 
         $this->line('STAGE 4: Test Administrative Bind for LDAP Sync');
         foreach ($ldap_urls as $ldap_url) {
+            if (Ldap::shouldUseSaslExternal($settings)) {
+                // SASL EXTERNAL uses the client TLS cert already loaded
+                // in connect_to_ldap() as the auth identity. No username
+                // or password gets sent. See GH #19518.
+                $this->test_sasl_external_bind($ldap_url[0], $ldap_url[1], $ldap_url[2]);
+
+                continue;
+            }
             try {
                 $w = Crypt::decrypt($settings->ldap_pword);
             } catch (Exception $e) {
@@ -407,14 +424,24 @@ public function handle()
         $this->debugout('LDAP constants are: '.print_r($ldap_constants, true));
 
         foreach ($ldap_urls as $ldap_url) {
-            try {
-                $w = Crypt::decrypt($settings->ldap_pword);
-            } catch (Exception $e) {
-                $this->warn('Could not decrypt password. This usually means an LDAP password was not set or the APP_KEY was changed since the LDAP pasword was last saved.  Aborting.');
-                exit(0);
+            if (Ldap::shouldUseSaslExternal($settings)) {
+                // Password decrypt + username don't apply under SASL
+                // EXTERNAL - both are null'd in the bind call. The
+                // informational read after the bind uses the same $settings
+                // path either way, so the branch is only around the bind.
+                $w = '';
+                $uname = null;
+            } else {
+                try {
+                    $w = Crypt::decrypt($settings->ldap_pword);
+                } catch (Exception $e) {
+                    $this->warn('Could not decrypt password. This usually means an LDAP password was not set or the APP_KEY was changed since the LDAP pasword was last saved.  Aborting.');
+                    exit(0);
+                }
+                $uname = $settings->ldap_uname;
             }
 
-            if ($this->test_informational_bind($ldap_url[0], $ldap_url[1], $ldap_url[2], $settings->ldap_uname, $w, $settings)) {
+            if ($this->test_informational_bind($ldap_url[0], $ldap_url[1], $ldap_url[2], $uname, $w, $settings)) {
                 $this->info('Success getting informational bind!');
             } else {
                 $this->error('Unable to get information from bind.');
@@ -526,18 +553,51 @@ public function test_authed_bind($ldap_url, $check_cert, $start_tls, $username,
         });
     }
 
+    public function test_sasl_external_bind($ldap_url, $check_cert, $start_tls)
+    {
+        return $this->timed_boolean_execute(function () use ($ldap_url, $check_cert, $start_tls) {
+            try {
+                $lconn = $this->connect_to_ldap($ldap_url, $check_cert, $start_tls);
+                $bind_results = ldap_sasl_bind($lconn, null, null, 'EXTERNAL');
+                ldap_close($lconn);
+                if (! $bind_results) {
+                    $this->error("WARNING: Failed to bind to $ldap_url via SASL EXTERNAL");
+
+                    return false;
+                }
+                $this->info("SUCCESS - Able to bind to $ldap_url via SASL EXTERNAL");
+
+                return (bool) $lconn;
+            } catch (Exception $e) {
+                $this->error('WARNING: Exception caught during SASL EXTERNAL bind - '.$e->getMessage());
+
+                return false;
+            }
+        });
+    }
+
     public function test_informational_bind($ldap_url, $check_cert, $start_tls, $username, $password, $settings)
     {
         return $this->timed_boolean_execute(function () use ($ldap_url, $check_cert, $start_tls, $username, $password, $settings) {
             try { // TODO - copypasta'ed from test_authed_bind
                 $conn = $this->connect_to_ldap($ldap_url, $check_cert, $start_tls);
-                $bind_results = ldap_bind($conn, $username, $password);
+                // Null $username signals the SASL EXTERNAL branch. The
+                // Stage 5 caller sets it that way when the auto-detect
+                // in Ldap::shouldUseSaslExternal() matches. Post-bind logic
+                // is identical either way.
+                if ($username === null) {
+                    $bind_results = ldap_sasl_bind($conn, null, null, 'EXTERNAL');
+                    $identityLabel = 'the SASL EXTERNAL client certificate';
+                } else {
+                    $bind_results = ldap_bind($conn, $username, $password);
+                    $identityLabel = $username;
+                }
                 if (! $bind_results) {
-                    $this->error("WARNING: Failed to bind to $ldap_url as $username");
+                    $this->error("WARNING: Failed to bind to $ldap_url as $identityLabel");
 
                     return false;
                 }
-                $this->info("SUCCESS - Able to bind to $ldap_url as $username");
+                $this->info("SUCCESS - Able to bind to $ldap_url as $identityLabel");
                 $cleaned_results = [];
                 try {
                     // This _may_ only work for Active Directory?

app/Livewire/LdapSettings.php

@@ -640,9 +640,24 @@
         $this->persistAndAdvance($setting);
     }
 
+    /**
+     * Live-form wrapper around Ldap::shouldUseSaslExternal(). The
+     * component is passed as-is because it carries the four properties
+     * the predicate reads. Blade-accessible via #[Computed].
+     */
+    #[Computed]
+    public function isSaslExternalCandidate(): bool
+    {
+        return Ldap::shouldUseSaslExternal($this);
+    }
+
     protected function canAdvanceStep2(): bool
     {
-        if (trim($this->ldap_uname) === '') {
+        // SASL EXTERNAL (auto-detected in bindAdminToLdap when client
+        // cert + key are populated and uname/pword are blank) skips
+        // the empty-uname gate: those fields are meant to be blank on
+        // that path.
+        if (trim($this->ldap_uname) === '' && ! $this->isSaslExternalCandidate()) {
             return false;
         }
         if (trim($this->ldap_basedn) === '') {
@@ -674,9 +689,18 @@
         $normalizeDn = fn ($dn) => strtolower(preg_replace('/\s*,\s*/', ',', trim((string) $dn)));
         $bindDn = $normalizeDn($this->ldap_uname);
 
+        // Auto-detected SASL EXTERNAL (client cert + key populated,
+        // uname/pword blank) uses the TLS cert as the bind identity,
+        // so ldap_uname / ldap_pword are optional on that path.
+        $sasl = $this->isSaslExternalCandidate();
+        $unameRule = $sasl ? ['nullable', 'max:191'] : 'required|max:191';
+        $pwordRule = $sasl
+            ? 'nullable'
+            : \Illuminate\Validation\Rule::when(! $canReusePersisted, 'required');
+
         return [
-            'ldap_uname' => 'required|max:191',
-            'ldap_pword' => \Illuminate\Validation\Rule::when(! $canReusePersisted, 'required'),
+            'ldap_uname' => $unameRule,
+            'ldap_pword' => $pwordRule,
             'ldap_basedn' => [
                 'required',
                 // Guard against the common misconfiguration where the base
@@ -732,29 +756,42 @@
             return;
         }
 
-        // Bind, always with credentials (uname required in step2SyntaxRules).
-        // Password resolution: form value if provided, otherwise fall back
-        // to the persisted encrypted password when the username matches.
+        // Bind. SASL EXTERNAL uses the client cert loaded by
+        // openLdapConnectionForTest() (via LDAP_OPT_X_TLS_CERTFILE /
+        // _KEYFILE) as the auth identity, so no username / password
+        // gets passed. Simple bind path resolves the password from the
+        // form value first, otherwise falls back to the persisted
+        // encrypted password when the username matches.
         $settings = Setting::getSettings();
         $server = (string) $settings->ldap_server;
-        $uname = trim($this->ldap_uname);
-        $pword = $this->ldap_pword;
-        if ($pword === '' && $uname === trim((string) $settings->ldap_uname) && $settings->ldap_pword) {
-            try {
-                $pword = Crypt::decrypt($settings->ldap_pword);
-            } catch (\Exception $e) {
-                @ldap_unbind($conn);
-                $this->recordTestResult(
-                    'error',
-                    trans('admin/settings/general.ldap_wizard.bind.pword_decrypt_failed'),
-                    'ldap bind test',
-                );
 
-                return;
+        if ($this->isSaslExternalCandidate()) {
+            // Success message uses $uname for the "Bound as ..."
+            // interpolation. Under SASL EXTERNAL there is no bind
+            // username, the client cert is the identity, so surface
+            // that instead of leaving $uname undefined.
+            $uname = trans('admin/settings/general.ldap_wizard.bind.sasl_external_identity');
+            $bindOk = @ldap_sasl_bind($conn, null, null, 'EXTERNAL');
+        } else {
+            $uname = trim($this->ldap_uname);
+            $pword = $this->ldap_pword;
+            if ($pword === '' && $uname === trim((string) $settings->ldap_uname) && $settings->ldap_pword) {
+                try {
+                    $pword = Crypt::decrypt($settings->ldap_pword);
+                } catch (\Exception $e) {
+                    @ldap_unbind($conn);
+                    $this->recordTestResult(
+                        'error',
+                        trans('admin/settings/general.ldap_wizard.bind.pword_decrypt_failed'),
+                        'ldap bind test',
+                    );
+
+                    return;
+                }
             }
-        }
 
-        $bindOk = @ldap_bind($conn, $uname, $pword);
+            $bindOk = @ldap_bind($conn, $uname, $pword);
+        }
         if (! $bindOk) {
             $errno = ldap_errno($conn);
             $ldapError = Ldap::bindError($conn);

app/Models/Ldap.php

@@ -258,40 +258,61 @@ public static function findAndBindUserLdap($username, $password)
     }
 
     /**
-     * Binds/authenticates an admin to LDAP for LDAP searching/syncing.
-     * Here we also return a better error if the app key is donked.
+     * Returns true when the given config source (Setting model, live
+     * Livewire component state, whatever) matches the auto-detect
+     * condition that routes bindAdminToLdap through SASL EXTERNAL:
+     * client cert + key are populated AND both bind DN and bind
+     * password are blank.
      *
-     * @author [A. Gianotto] [<snipe@snipe.net>]
+     * Runtime, LDAP troubleshooter, and LDAP Livewire wizard all
+     * evaluate the same.
+     */
+    public static function shouldUseSaslExternal(object $config): bool
+    {
+        return !empty($config->ldap_client_tls_cert)
+            && !empty($config->ldap_client_tls_key)
+            && empty($config->ldap_uname)
+            && empty($config->ldap_pword);
+    }
+
+    /**
+     * Binds/authenticates an admin to LDAP for LDAP searching/syncing.
+     * Modifies state on the passed-in $connection. Throws on any bind
+     * failure with a decoded message (including a friendlier "app key
+     * changed" message for the encrypted-password decrypt path).
+     * Callers wrap in try/catch and rely on the exception, not a
+     * return value.
      *
+     * @throws Exception on any bind failure
      * @since  [v3.0]
      *
-     * @param  bool|false  $user
-     * @return bool true    if the username and/or password provided are valid
-     *              false   if the username and/or password provided are invalid
+     * @author [A. Gianotto] [<snipe@snipe.net>]
+     *
      */
-    public static function bindAdminToLdap($connection)
+    public static function bindAdminToLdap($connection): void
     {
-        $ldap_username = Setting::getSettings()->ldap_uname;
+        $settings = Setting::getSettings();
+
+        $ldap_username = $settings->ldap_uname;
 
-        if ($ldap_username) {
+        if (self::shouldUseSaslExternal($settings)) {
+            if (! @ldap_sasl_bind($connection, null, null, 'EXTERNAL')) {
+                throw new Exception('Could not bind to LDAP via SASL EXTERNAL: '.self::bindError($connection));
+            }
+        } elseif ($ldap_username) {
             // Lets return some nicer messages for users who donked their app key, and disable LDAP
             try {
-                $ldap_pass = Crypt::decrypt(Setting::getSettings()->ldap_pword);
+                $ldap_pass = Crypt::decrypt($settings->ldap_pword);
             } catch (Exception $e) {
                 throw new Exception('Your app key has changed! Could not decrypt LDAP password using your current app key, so LDAP authentication has been disabled. Login with a local account, update the LDAP password and re-enable it in Admin > Settings.');
             }
 
-            if (! $ldapbind = @ldap_bind($connection, $ldap_username, $ldap_pass)) {
+            if (! @ldap_bind($connection, $ldap_username, $ldap_pass)) {
                 throw new Exception('Could not bind to LDAP: '.self::bindError($connection));
             }
-            // TODO - this just "falls off the end" but the function states that it should return true or false
-            // unfortunately, one of the use cases for this function is wrong and *needs* for that failure mode to fire
-            // so I don't want to fix this right now.
-            // this method MODIFIES STATE on the passed-in $connection and just returns true or false (or, in this case, undefined)
-            // at the next refactor, this should be appropriately modified to be more consistent.
         } else {
             // LDAP should also work with anonymous bind (no dn, no password available)
-            if (! $ldapbind = @ldap_bind($connection)) {
+            if (! @ldap_bind($connection)) {
                 throw new Exception('Could not bind to LDAP: '.self::bindError($connection));
             }
         }

phpstan-baseline.neon

@@ -7899,7 +7899,7 @@ parameters:
 		-
 			message: '#^Method App\\Models\\Ldap\:\:bindAdminToLdap\(\) should return bool but return statement is missing\.$#'
 			identifier: return.missing
-			count: 2
+			count: 3
 			path: app/Models/Ldap.php
 
 		-
@@ -8970,18 +8970,6 @@ parameters:
 			count: 1
 			path: app/Notifications/AuditNotification.php
 
-		-
-			message: '#^Access to an undefined property App\\Notifications\\CheckinAccessoryNotification\:\:\$admin\.$#'
-			identifier: property.notFound
-			count: 2
-			path: app/Notifications/CheckinAccessoryNotification.php
-
-		-
-			message: '#^Constructor of class App\\Notifications\\CheckinAccessoryNotification has an unused parameter \$admin\.$#'
-			identifier: constructor.unusedParameter
-			count: 1
-			path: app/Notifications/CheckinAccessoryNotification.php
-
 		-
 			message: '#^Ternary operator condition is always true\.$#'
 			identifier: ternary.alwaysTrue

resources/lang/en-US/admin/settings/general.php

@@ -106,9 +106,11 @@
         'step_connection_help' => 'Configure how to reach your LDAP or Active Directory server. Once saved, you can test the connection before moving on to bind credentials.',
         'ldap_uname_help' => 'The distinguished name (DN) or User Principal Name (UPN) of an admin service account that can search your directory. For Active Directory this is usually a UPN like <code>admin@example.com</code>. For OpenLDAP and most other directories this is a full DN like <code>cn=admin,dc=example,dc=com</code>. Enter the entire path including the base DN portion, not just the account name.',
         'ldap_pword_help' => 'Leave blank to keep the previously-saved password.',
+        'sasl_external_step2_hint' => 'A client TLS certificate is configured on the previous step. Leave the Bind DN/UPN and Bind Password fields blank to authenticate with the certificate (SASL EXTERNAL). Fill them in only if the server requires cert-plus-bind together.',
         'bind' => [
             'pword_decrypt_failed' => 'Could not decrypt the stored bind password. This usually means the app key changed since the password was saved. Enter the password again to fix.',
             'rejected' => 'The LDAP server rejected the bind (:error). Double-check the username and password.',
+            'sasl_external_identity' => 'the SASL EXTERNAL client certificate',
         ],
         'ldap_basedn_help' => 'The distinguished name (DN) where the user search begins. Everything below this DN is in scope. Typically the OU or container that holds your synced users.',
         'ldap_filter_help' => 'LDAP filter expression used to narrow the search to synceable users. Omit the outer parentheses, they are wrapped automatically. Leave blank to accept all objects under the base DN, though this is rarely what you want.',
@@ -196,7 +198,7 @@
     'ldap_enabled' => 'LDAP enabled',
     'ldap_integration' => 'LDAP Integration',
     'ldap_settings' => 'LDAP Settings',
-    'ldap_client_tls_cert_help' => 'Client-Side TLS Certificate and Key for LDAP connections are usually only useful in Google Workspace configurations with "Secure LDAP." Both are required.',
+    'ldap_client_tls_cert_help' => 'Client-Side TLS Certificate and Key for LDAP connections are usually only useful in Google Workspace configurations with "Secure LDAP." Both are required. For certificate-based (SASL EXTERNAL) authentication, leave the Bind DN/UPN and Bind Password fields on the next step blank.',
     'ldap_location' => 'Location Field',
     'ldap_location_help' => 'The LDAP Location field should be used if <strong>an OU is not being used in the Base Bind DN.</strong> Leave this blank if an OU search is being used.',
     'ldap_login_test_help' => 'Enter a valid LDAP username and password from the base DN you specified above to test whether your LDAP login is configured correctly. YOU MUST SAVE YOUR UPDATED LDAP SETTINGS FIRST.',