Skip to content

Commit f2001fa

Browse files
simonLeary42simonLeary42
authored
Revert "disband, reinstate PI groups (#520)"
This reverts commit be1a3a5.
1 parent be1a3a5 commit f2001fa

20 files changed

+70
-645
lines changed

‎LDAP.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,3 @@ Terminology:
55
- **native user**: a user created by this account portal
66
- **non-native user**: inverse of native
77
- users created for administrative purposes should not be mixed with native users in the LDAP OUs given in `config.ini` or else this account portal may get confused
8-
- **disabled group**: a PI group that was disabled by its owner or had its owner disabled
9-
- memberuid attribute should be empty

‎README.md‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,6 @@ rm "$prod" && ln -s "$old" "$prod"
132132
- the `home` page can be copied over to `deployment/templates_overrides/home.php`
133133
- the `support` page should be moved over to wherever you host your documentation
134134
- the `notices` SQL table should be droppped
135-
- a new LDAP schema needs to be added:
136-
```shell
137-
scp tools/docker-dev/identity/unity-cluster-schema.ldif root@your-ldap-server:/root/unity-cluster-schema.ldif
138-
ssh root@your-ldap-server ldapadd -Y EXTERNAL -H ldapi:/// -f /root/unity-cluster-schema.ldif
139-
```
140135

141136
### 1.5 -> 1.6
142137

‎resources/lib/UnityGroup.php‎

Lines changed: 43 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function __toString(): string
4040
public function requestGroup(?bool $send_mail_to_admins = null, bool $send_mail = true): void
4141
{
4242
$send_mail_to_admins ??= CONFIG["mail"]["send_pimesg_to_admins"];
43-
if ($this->exists() && !$this->getIsDisabled()) {
43+
if ($this->exists()) {
4444
return;
4545
}
4646
if ($this->SQL->accDeletionRequestExists($this->getOwner()->uid)) {
@@ -63,70 +63,18 @@ public function requestGroup(?bool $send_mail_to_admins = null, bool $send_mail
6363
}
6464
}
6565

66-
public function disable(bool $send_mail = true): void
67-
{
68-
if ($this->getIsDisabled()) {
69-
throw new Exception("cannot disable an already disabled group");
70-
}
71-
$this->SQL->addLog("disable_pi_group", $this->gid);
72-
$memberuids = $this->getMemberUIDs();
73-
if ($send_mail) {
74-
$member_attributes = $this->LDAP->getUsersAttributes($memberuids, ["mail"]);
75-
$member_mails = array_map(fn($x) => (string) $x["mail"][0], $member_attributes);
76-
if (count($member_mails) > 0) {
77-
$this->MAILER->sendMail($member_mails, "group_disabled", [
78-
"group_name" => $this->gid,
79-
]);
80-
}
81-
}
82-
$this->setIsDisabled(true);
83-
if (count($memberuids) > 0) {
84-
$this->entry->setAttribute("memberuid", []);
85-
}
86-
// TODO optimize
87-
// UnityUser::__construct() makes one LDAP query for each user
88-
// updateIsQualified() makes one LDAP query for each member
89-
// if user is no longer in any PI group, disqualify them
90-
foreach ($memberuids as $uid) {
91-
$user = new UnityUser($uid, $this->LDAP, $this->SQL, $this->MAILER, $this->WEBHOOK);
92-
$user->updateIsQualified($send_mail);
93-
}
94-
}
95-
96-
private function reenable(bool $send_mail = true): void
97-
{
98-
if (!$this->getIsDisabled()) {
99-
throw new Exception("cannot re-enable a group that is not disabled");
100-
}
101-
$this->SQL->addLog("reenabled_pi_group", $this->gid);
102-
if ($send_mail) {
103-
$this->MAILER->sendMail($this->getOwner()->getMail(), "group_reenabled", [
104-
"group_name" => $this->gid,
105-
]);
106-
}
107-
$this->setIsDisabled(false);
108-
$owner_uid = $this->getOwner()->uid;
109-
if (!$this->memberUIDExists($owner_uid)) {
110-
$this->addMemberUID($owner_uid);
111-
}
112-
$this->getOwner()->updateIsQualified($send_mail);
113-
}
114-
11566
/**
11667
* This method will create the group (this is what is executed when an admin approved the group)
11768
*/
11869
public function approveGroup(bool $send_mail = true): void
11970
{
12071
$uid = $this->getOwner()->uid;
12172
$request = $this->SQL->getRequest($uid, UnitySQL::REQUEST_BECOME_PI);
122-
\ensure($this->getOwner()->exists());
123-
if (!$this->entry->exists()) {
124-
$this->init();
125-
} elseif ($this->getIsDisabled()) {
126-
$this->reenable();
127-
} else {
128-
throw new Exception("cannot approve group that already exists and is not disabled");
73+
if ($this->exists()) {
74+
return;
12975
}
76+
\ensure($this->getOwner()->exists());
77+
$this->init();
13078
$this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI);
13179
$this->SQL->addLog("approved_group", $this->getOwner()->uid);
13280
if ($send_mail) {
@@ -178,6 +126,42 @@ public function cancelGroupJoinRequest(UnityUser $user, bool $send_mail = true):
178126
}
179127
}
180128

129+
// /**
130+
// * This method will delete the group, either by admin action or PI action
131+
// */
132+
// public function removeGroup($send_mail = true)
133+
// {
134+
// // remove any pending requests
135+
// // this will silently fail if the request doesn't exist (which is what we want)
136+
// $this->SQL->removeRequests($this->gid);
137+
138+
// // we don't need to do anything extra if the group is already deleted
139+
// if (!$this->exists()) {
140+
// return;
141+
// }
142+
143+
// // first, we must record the users in the group currently
144+
// $users = $this->getGroupMembers();
145+
146+
// // now we delete the ldap entry
147+
// $this->entry->ensureExists();
148+
// $this->entry->delete();
149+
150+
// // Logs the change
151+
// $this->SQL->addLog("removed_group", $this->gid);
152+
153+
// // send email to every user of the now deleted PI group
154+
// if ($send_mail) {
155+
// foreach ($users as $user) {
156+
// $this->MAILER->sendMail(
157+
// $user->getMail(),
158+
// "group_disband",
159+
// array("group_name" => $this->gid)
160+
// );
161+
// }
162+
// }
163+
// }
164+
181165
/**
182166
* This method is executed when a user is approved to join the group
183167
* (either by admin or the group owner)
@@ -236,7 +220,7 @@ public function removeUser(UnityUser $new_user, bool $send_mail = true): void
236220
return;
237221
}
238222
if ($new_user->uid == $this->getOwner()->uid) {
239-
throw new Exception("Cannot delete group owner from group. Disable group instead");
223+
throw new Exception("Cannot delete group owner from group. Disband group instead");
240224
}
241225
$this->removeMemberUID($new_user->uid);
242226
$this->SQL->addLog(
@@ -342,7 +326,7 @@ private function init(): void
342326
\ensure(!$this->entry->exists());
343327
$nextGID = $this->LDAP->getNextPIGIDNumber();
344328
$this->entry->create([
345-
"objectclass" => ["unityClusterPIGroup", "posixGroup", "top"],
329+
"objectclass" => UnityLDAP::POSIX_GROUP_CLASS,
346330
"gidnumber" => strval($nextGID),
347331
"memberuid" => [$owner->uid],
348332
]);
@@ -392,40 +376,4 @@ public function getGroupMembersAttributes(array $attributes, array $default_valu
392376
$default_values,
393377
);
394378
}
395-
396-
public function getIsDisabled(): bool
397-
{
398-
$value = $this->entry->getAttribute("isDisabled");
399-
switch (count($value)) {
400-
case 0:
401-
return false;
402-
case 1:
403-
switch ($value[0]) {
404-
case "TRUE":
405-
return true;
406-
case "FALSE":
407-
return false;
408-
default:
409-
throw new \RuntimeException(
410-
sprintf(
411-
"unexpected value for isDisabled: '%s'. expected 'TRUE' or 'FALSE'",
412-
$value[0],
413-
),
414-
);
415-
}
416-
default:
417-
throw new \RuntimeException(
418-
sprintf(
419-
"expected value of length 0 or 1, found value %s of length %s",
420-
_json_encode($value),
421-
count($value),
422-
),
423-
);
424-
}
425-
}
426-
427-
public function setIsDisabled(bool $new_value): void
428-
{
429-
$this->entry->setAttribute("isDisabled", $new_value ? "TRUE" : "FALSE");
430-
}
431379
}

‎resources/lib/UnityLDAP.php‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ class UnityLDAP extends LDAPConn
3232
"ldapPublicKey",
3333
];
3434

35-
// isDisabled unset or set to "FALSE"
36-
private static string $NON_DISABLED_FILTER = "(|(!(isDisabled=*))(isDisabled=FALSE))";
35+
public const array POSIX_GROUP_CLASS = ["posixGroup", "top"];
3736

3837
private string $custom_mappings_path =
3938
__DIR__ . "/../../" . CONFIG["ldap"]["custom_user_mappings_dir"];
@@ -199,7 +198,7 @@ public function getAllNativeUsersAttributes(
199198
}
200199

201200
/** @return UnityGroup[] */
202-
public function getAllNonDisabledPIGroups(
201+
public function getAllPIGroups(
203202
UnitySQL $UnitySQL,
204203
UnityMailer $UnityMailer,
205204
UnityWebhook $UnityWebhook,
@@ -208,7 +207,6 @@ public function getAllNonDisabledPIGroups(
208207
$pi_groups_attributes = $this->pi_groupOU->getChildrenArrayStrict(
209208
attributes: ["cn"],
210209
recursive: false,
211-
filter: self::$NON_DISABLED_FILTER,
212210
);
213211
foreach ($pi_groups_attributes as $attributes) {
214212
array_push(
@@ -224,41 +222,35 @@ public function getAllNonDisabledPIGroups(
224222
* @param attributes $default_values
225223
* @return attributes[]
226224
*/
227-
public function getAllNonDisabledPIGroupsAttributes(
228-
array $attributes,
229-
array $default_values = [],
230-
): array {
225+
public function getAllPIGroupsAttributes(array $attributes, array $default_values = []): array
226+
{
231227
return $this->pi_groupOU->getChildrenArrayStrict(
232228
$attributes,
233229
false, // non-recursive
234-
self::$NON_DISABLED_FILTER,
230+
"objectClass=posixGroup",
235231
$default_values,
236232
);
237233
}
238234

239235
/** @return string[] */
240-
public function getNonDisabledPIGroupGIDsWithMemberUID(string $uid): array
236+
public function getPIGroupGIDsWithMemberUID(string $uid): array
241237
{
242238
return array_map(
243239
fn($x) => $x["cn"][0],
244240
$this->pi_groupOU->getChildrenArrayStrict(
245241
["cn"],
246242
false,
247-
sprintf(
248-
"(&(memberuid=%s)%s)",
249-
ldap_escape($uid, flags: LDAP_ESCAPE_FILTER),
250-
self::$NON_DISABLED_FILTER,
251-
),
243+
"(memberuid=" . ldap_escape($uid, flags: LDAP_ESCAPE_FILTER) . ")",
252244
),
253245
);
254246
}
255247

256248
/** @return string[] */
257-
public function getAllNonDisabledPIGroupOwnerUIDs(): array
249+
public function getAllPIGroupOwnerUIDs(): array
258250
{
259251
return array_map(
260-
fn($x) => UnityGroup::GID2OwnerUID((string) $x["cn"][0]),
261-
$this->getAllNonDisabledPIGroupsAttributes(["cn"]),
252+
fn($x) => UnityGroup::GID2OwnerUID($x["cn"][0]),
253+
$this->pi_groupOU->getChildrenArrayStrict(["cn"]),
262254
);
263255
}
264256

@@ -288,7 +280,7 @@ public function getUID2PIGIDs(): array
288280
{
289281
$uid2pigids = [];
290282
// for each PI group, append that GID to the member list for each of its member UIDs
291-
$pi_groups_attributes = $this->getAllNonDisabledPIGroupsAttributes(
283+
$pi_groups_attributes = $this->getAllPIGroupsAttributes(
292284
["cn", "memberuid"],
293285
default_values: ["memberuid" => []],
294286
);

‎resources/lib/UnityOrg.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public function init(): void
2424
\ensure(!$this->entry->exists());
2525
$nextGID = $this->LDAP->getNextOrgGIDNumber();
2626
$this->entry->create([
27-
"objectclass" => ["posixGroup", "top"],
27+
"objectclass" => UnityLDAP::POSIX_GROUP_CLASS,
2828
"gidnumber" => strval($nextGID),
2929
]);
3030
}

‎resources/lib/UnityUser.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function init(
6060
$id = $this->LDAP->getNextUIDGIDNumber($this->uid);
6161
\ensure(!$ldapGroupEntry->exists());
6262
$ldapGroupEntry->create([
63-
"objectclass" => ["posixGroup", "top"],
63+
"objectclass" => UnityLDAP::POSIX_GROUP_CLASS,
6464
"gidnumber" => strval($id),
6565
]);
6666
\ensure(!$this->entry->exists());
@@ -346,7 +346,7 @@ public function getHomeDir(): string
346346
*/
347347
public function isPI(): bool
348348
{
349-
return $this->getPIGroup()->exists() && !$this->getPIGroup()->getIsDisabled();
349+
return $this->getPIGroup()->exists();
350350
}
351351

352352
public function getPIGroup(): UnityGroup
@@ -371,7 +371,7 @@ public function getOrgGroup(): UnityOrg
371371
*/
372372
public function getPIGroupGIDs(): array
373373
{
374-
return $this->LDAP->getNonDisabledPIGroupGIDsWithMemberUID($this->uid);
374+
return $this->LDAP->getPIGroupGIDsWithMemberUID($this->uid);
375375
}
376376

377377
/**

resources/mail/group_disabled.php renamed to resources/mail/group_disband.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
<?php
22

3-
$this->Subject = "PI Group Disabled"; ?>
3+
$this->Subject = "PI Group Disbanded"; ?>
44

55
<p>Hello,</p>
66

7-
<p>Your PI group, <?php echo $data["group_name"]; ?>, has been disabled on the UnityHPC Platform.
7+
<p>Your PI group, <?php echo $data["group_name"]; ?>, has been disbanded on the UnityHPC Platform.
88
Any jobs associated with this PI account have been killed.</p>
99

1010
<p>If you believe this to be a mistake, please reply to this email</p>

‎resources/mail/group_reenabled.php‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎test/functional/PIBecomeApproveTest.php‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -75,27 +75,4 @@ public function testApprovePI()
7575
$this->assertFalse($USER->getFlag(UserFlag::QUALIFIED));
7676
}
7777
}
78-
79-
public function testReenableGroup()
80-
{
81-
global $USER, $SSO, $LDAP, $SQL, $MAILER, $WEBHOOK;
82-
$this->switchUser("ReenabledOwnerOfDisabledPIGroup");
83-
$this->assertFalse($USER->isPI());
84-
$user = $USER;
85-
$pi_group = $USER->getPIGroup();
86-
$approve_uid = $USER->uid;
87-
try {
88-
$this->requestGroupCreation();
89-
$this->assertRequestedPIGroup(true);
90-
$this->switchUser("Admin");
91-
$this->approveGroup($approve_uid);
92-
$this->assertTrue($user->isPI());
93-
} finally {
94-
if ($pi_group->memberUIDExists($approve_uid)) {
95-
$pi_group->removeMemberUID($approve_uid);
96-
$pi_group->setIsDisabled(true);
97-
assert(!$user->isPI());
98-
}
99-
}
100-
}
10178
}

0 commit comments

Comments
 (0)