Unverified Commit 097fb738 authored by Côme Chilliet's avatar Côme Chilliet
Browse files

:ambulance: fix(core) Fix membership removal workflow

Also improve error messages

issue #6100
Showing with 55 additions and 8 deletions
+55 -8
...@@ -171,27 +171,66 @@ class userRoles extends simplePlugin ...@@ -171,27 +171,66 @@ class userRoles extends simplePlugin
return FALSE; return FALSE;
} }
/*! \brief This function removes the object from LDAP
*/
function remove (bool $fulldelete = FALSE): array
{
if (!$fulldelete) {
/* We are not deleting the object it's just that there are no groups left
* Make sure memberships are empty to avoid surprises and call save. */
$this->rolesMembership = [];
$this->groupsMembership = [];
return $this->save();
} else {
return parent::remove($fulldelete);
}
}
protected function ldap_remove (): array protected function ldap_remove (): array
{ {
if ($this->is_template) { if ($this->is_template) {
return parent::ldap_remove(); return parent::ldap_remove();
} elseif (($this->dn != '') && ($this->dn != 'new')) { } elseif (($this->dn != '') && ($this->dn != 'new')) {
/* Remove all groups */ /* Remove all groups */
foreach ($this->savedGroupsMembership as $ogroupdn) { foreach ($this->savedGroupsMembership as $key => $ogroupdn) {
try { try {
$g = objects::open($ogroupdn, 'ogroup'); $g = objects::open($ogroupdn, 'ogroup');
$g->getBaseObject()->attributesAccess['member']->searchAndRemove($this->dn); $g->getBaseObject()->attributesAccess['member']->searchAndRemove($this->dn);
$g->save(); $msg = $g->save();
if (empty($msg)) {
unset($this->savedGroupsMembership[$key]);
} else {
/* We do not prevent user deletion on error, but still warn the user */
foreach ($msg as $error) {
msg_dialog::display(
_('Warning'),
sprintf(_('Could not remove membership to group %s: %s'), $ogroupdn, $error),
WARNING_DIALOG
);
}
}
} catch (NonExistingLdapNodeException $e) { } catch (NonExistingLdapNodeException $e) {
/* Ignore deleted groups */ /* Ignore deleted groups */
} }
} }
/* Remove all roles */ /* Remove all roles */
foreach ($this->savedRolesMembership as $roledn) { foreach ($this->savedRolesMembership as $key => $roledn) {
try { try {
$r = objects::open($roledn, 'role'); $r = objects::open($roledn, 'role');
$r->getBaseObject()->attributesAccess['roleOccupant']->searchAndRemove($this->dn); $r->getBaseObject()->attributesAccess['roleOccupant']->searchAndRemove($this->dn);
$r->save(); $msg = $r->save();
if (empty($msg)) {
unset($this->savedRolesMembership[$key]);
} else {
/* We do not prevent user deletion on error, but still warn the user */
foreach ($msg as $error) {
msg_dialog::display(
_('Warning'),
sprintf(_('Could not remove membership to role %s: %s'), $roledn, $error),
WARNING_DIALOG
);
}
}
} catch (NonExistingLdapNodeException $e) { } catch (NonExistingLdapNodeException $e) {
/* Ignore deleted roles */ /* Ignore deleted roles */
} }
...@@ -271,7 +310,9 @@ class userRoles extends simplePlugin ...@@ -271,7 +310,9 @@ class userRoles extends simplePlugin
if (empty($msg)) { if (empty($msg)) {
$this->savedGroupsMembership[] = $ogroupdn; $this->savedGroupsMembership[] = $ogroupdn;
} else { } else {
$errors = array_merge($errors, $msg); foreach ($msg as $error) {
$errors[] = sprintf(_('Could not add membership to group %s: %s'), $ogroupdn, $error);
}
} }
} catch (NonExistingLdapNodeException $e) { } catch (NonExistingLdapNodeException $e) {
$errors[] = $e->getMessage(); $errors[] = $e->getMessage();
...@@ -293,7 +334,9 @@ class userRoles extends simplePlugin ...@@ -293,7 +334,9 @@ class userRoles extends simplePlugin
if (empty($msg)) { if (empty($msg)) {
unset($this->savedGroupsMembership[$key]); unset($this->savedGroupsMembership[$key]);
} else { } else {
$errors = array_merge($errors, $msg); foreach ($msg as $error) {
$errors[] = sprintf(_('Could not remove membership to group %s: %s'), $ogroupdn, $error);
}
} }
} catch (NonExistingLdapNodeException $e) { } catch (NonExistingLdapNodeException $e) {
$errors[] = $e->getMessage(); $errors[] = $e->getMessage();
...@@ -316,7 +359,9 @@ class userRoles extends simplePlugin ...@@ -316,7 +359,9 @@ class userRoles extends simplePlugin
if (empty($msg)) { if (empty($msg)) {
$this->savedRolesMembership[] = $roledn; $this->savedRolesMembership[] = $roledn;
} else { } else {
$errors = array_merge($errors, $msg); foreach ($msg as $error) {
$errors[] = sprintf(_('Could not add membership to role %s: %s'), $roledn, $error);
}
} }
} catch (NonExistingLdapNodeException $e) { } catch (NonExistingLdapNodeException $e) {
$errors[] = $e->getMessage(); $errors[] = $e->getMessage();
...@@ -338,7 +383,9 @@ class userRoles extends simplePlugin ...@@ -338,7 +383,9 @@ class userRoles extends simplePlugin
if (empty($msg)) { if (empty($msg)) {
unset($this->savedRolesMembership[$key]); unset($this->savedRolesMembership[$key]);
} else { } else {
$errors = array_merge($errors, $msg); foreach ($msg as $error) {
$errors[] = sprintf(_('Could not remove membership to role %s: %s'), $roledn, $error);
}
} }
} catch (NonExistingLdapNodeException $e) { } catch (NonExistingLdapNodeException $e) {
$errors[] = $e->getMessage(); $errors[] = $e->getMessage();
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment