System: Access: Servers - some logic changes for "Default groups" opt…

…ion #8065 * we expect memberof instead of memberOf in our ldap responses, make sure we lowercase the response at all times * make $memberof optional when pushing default groups The scenario's we should support are the following: 1. groups are synchronized via ldap/radius and users are created when they don't exist, which means existing groups will be altered after login to equal "memberOf" + optional default group[s] 2. groups are not synchronized via ldap/radius, but default groups exist, in which case default group[s] will be added when not yet assigned, no groups will be removed
opnsense · Nov 19, 2024 · a7104ab · a7104ab
1 parent 37c9dea
commit a7104ab
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 5 deletions.
diff --git a/src/opnsense/mvc/app/library/OPNsense/Auth/Base.php b/src/opnsense/mvc/app/library/OPNsense/Auth/Base.php
@@ -206,7 +206,9 @@ protected function setGroupMembership($username, $memberof, $scope = [], $create
                         in_array((string)$user->uid, (array)$group->member)
                           && empty($ldap_groups[$lc_groupname])
                     ) {
-                        unset($group->member[array_search((string)$user->uid, (array)$group->member)]);
+                        while (in_array((string)$user->uid, (array)$group->member)) {
+                            unset($group->member[array_search((string)$user->uid, (array)$group->member)]);
+                        }
                         syslog(LOG_NOTICE, sprintf(
                             'User: policy change for %s unlink group %s',
                             $username,

diff --git a/src/opnsense/mvc/app/library/OPNsense/Auth/LDAP.php b/src/opnsense/mvc/app/library/OPNsense/Auth/LDAP.php
@@ -511,12 +511,13 @@ public function authenticate($username, $password)
 
         if ($ldap_is_connected) {
             $this->lastAuthProperties['dn'] = $user_dn;
-            $this->lastAuthProperties['memberOf'] = '';
+            $this->lastAuthProperties['memberof'] = '';
             if ($this->ldapReadProperties) {
                 $sr = @ldap_read($this->ldapHandle, $user_dn, '(objectclass=*)', ['*', 'memberOf']);
                 $info = $sr !== false ? @ldap_get_entries($this->ldapHandle, $sr) : [];
                 if (!empty($info['count'])) {
                     foreach ($info[0] as $ldap_key => $ldap_value) {
+                        $ldap_key = strtolower($ldap_key); /* enforce lowercase, we expect memberof */
                         if (!is_numeric($ldap_key) && $ldap_key !== 'count') {
                             if (isset($ldap_value['count'])) {
                                 unset($ldap_value['count']);
@@ -540,7 +541,9 @@ public function authenticate($username, $password)
                     $default_groups = explode(",", strtolower($this->ldapSyncDefaultGroups));
                 }
 
-                if ($this->ldapSyncMemberOfConstraint) {
+                if (!$this->ldapSyncMemberOf) {
+                    $membersOf = $default_groups;
+                } elseif ($this->ldapSyncMemberOfConstraint) {
                     // Filter "memberOf" results to those recorded in ldapAuthcontainers, where
                     // the first part of the member is considered the group name, the rest should be an exact
                     // (case insensitive) match.

diff --git a/src/opnsense/mvc/app/library/OPNsense/Auth/Radius.php b/src/opnsense/mvc/app/library/OPNsense/Auth/Radius.php
@@ -527,7 +527,9 @@ public function authenticate($username, $password)
                                     $this->lastAuthProperties['Framed-Route'][] = $resa['data'];
                                     break;
                                 case RADIUS_CLASS:
-                                    if (!empty($this->lastAuthProperties['class'])) {
+                                    if (!$this->syncMemberOf) {
+                                        break;
+                                    } elseif (!empty($this->lastAuthProperties['class'])) {
                                         $this->lastAuthProperties['class'] .= "\n" . $resa['data'];
                                     } else {
                                         $this->lastAuthProperties['class'] = $resa['data'];
@@ -542,7 +544,7 @@ public function authenticate($username, $password)
                             $this->setGroupMembership(
                                 $username,
                                 $this->lastAuthProperties['class'] ?? '',
-                                $this->syncMemberOfLimit,
+                                $this->syncMemberOf ? $this->syncMemberOfLimit : $this->syncDefaultGroups,
                                 $this->syncCreateLocalUsers,
                                 $this->syncDefaultGroups
                             );