Skip to content

Commit ec8639f

Browse files
committed
System: Access: Servers - some logic changes for "Default groups" option #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
1 parent 37c9dea commit ec8639f

File tree

3 files changed

+12
-5
lines changed

3 files changed

+12
-5
lines changed

src/opnsense/mvc/app/library/OPNsense/Auth/Base.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ protected function setGroupMembership($username, $memberof, $scope = [], $create
206206
in_array((string)$user->uid, (array)$group->member)
207207
&& empty($ldap_groups[$lc_groupname])
208208
) {
209-
unset($group->member[array_search((string)$user->uid, (array)$group->member)]);
209+
while (in_array((string)$user->uid, (array)$group->member)) {
210+
unset($group->member[array_search((string)$user->uid, (array)$group->member)]);
211+
}
210212
syslog(LOG_NOTICE, sprintf(
211213
'User: policy change for %s unlink group %s',
212214
$username,

src/opnsense/mvc/app/library/OPNsense/Auth/LDAP.php

+5-2
Original file line numberDiff line numberDiff line change
@@ -511,12 +511,13 @@ public function authenticate($username, $password)
511511

512512
if ($ldap_is_connected) {
513513
$this->lastAuthProperties['dn'] = $user_dn;
514-
$this->lastAuthProperties['memberOf'] = '';
514+
$this->lastAuthProperties['memberof'] = '';
515515
if ($this->ldapReadProperties) {
516516
$sr = @ldap_read($this->ldapHandle, $user_dn, '(objectclass=*)', ['*', 'memberOf']);
517517
$info = $sr !== false ? @ldap_get_entries($this->ldapHandle, $sr) : [];
518518
if (!empty($info['count'])) {
519519
foreach ($info[0] as $ldap_key => $ldap_value) {
520+
$ldap_key = strtolower($ldap_key); /* enforce lowercase, we expect memberof */
520521
if (!is_numeric($ldap_key) && $ldap_key !== 'count') {
521522
if (isset($ldap_value['count'])) {
522523
unset($ldap_value['count']);
@@ -540,7 +541,9 @@ public function authenticate($username, $password)
540541
$default_groups = explode(",", strtolower($this->ldapSyncDefaultGroups));
541542
}
542543

543-
if ($this->ldapSyncMemberOfConstraint) {
544+
if (!$this->ldapSyncMemberOf) {
545+
$membersOf = $default_groups;
546+
} elseif ($this->ldapSyncMemberOfConstraint) {
544547
// Filter "memberOf" results to those recorded in ldapAuthcontainers, where
545548
// the first part of the member is considered the group name, the rest should be an exact
546549
// (case insensitive) match.

src/opnsense/mvc/app/library/OPNsense/Auth/Radius.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,9 @@ public function authenticate($username, $password)
527527
$this->lastAuthProperties['Framed-Route'][] = $resa['data'];
528528
break;
529529
case RADIUS_CLASS:
530-
if (!empty($this->lastAuthProperties['class'])) {
530+
if (!$this->syncMemberOf) {
531+
break;
532+
} elseif (!empty($this->lastAuthProperties['class'])) {
531533
$this->lastAuthProperties['class'] .= "\n" . $resa['data'];
532534
} else {
533535
$this->lastAuthProperties['class'] = $resa['data'];
@@ -542,7 +544,7 @@ public function authenticate($username, $password)
542544
$this->setGroupMembership(
543545
$username,
544546
$this->lastAuthProperties['class'] ?? '',
545-
$this->syncMemberOfLimit,
547+
!$this->syncMemberOf ? $this->syncMemberOfLimit : $this->syncDefaultGroups,
546548
$this->syncCreateLocalUsers,
547549
$this->syncDefaultGroups
548550
);

0 commit comments

Comments
 (0)