Skip to content

Commit 52914cd

Browse files
simonLeary42simonLeary42
committed
optimize IDNumber selection, avoid conflicts, remove site vars, rewrite custom mapping, add tests
1 parent ebac9cf commit 52914cd

File tree

12 files changed

+145
-139
lines changed

12 files changed

+145
-139
lines changed

‎.gitignore‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
# don't track custom user mappings
2-
resources/custom_user_mappings/*.csv
3-
41
# vscode project files
52
.vscode/
63

@@ -14,6 +11,10 @@ composer.lock
1411

1512
# don't track site configs
1613
deployment/*
14+
!deployment/overrides/
15+
!deployment/overrides/phpunit/
16+
!deployment/overrides/phpunit/config/
17+
!deployment/overrides/phpunit/config/config.ini
1718
!deployment/**/README.md
1819

1920
.phpunit.result.cache

‎README.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ See the Docker Compose environment (`tools/docker-dev/`) for an (unsafe for prod
6262
* Make sure this file is not world readable!
6363
1. If using mulitple domains, create `deployment/overrides/<domain>/config/config.ini`
6464
1. If using custom UIDNumber/GIDNumber mappings, create `deployment/custom_user_mappings/*.csv`
65+
* The 1st column is UID, the 2nd column is both UIDNumber and GIDNumber
6566
1. Add logos to `webroot/assets/footer_logos/`
6667

6768
## Integration
@@ -105,6 +106,15 @@ rm "$prod" && ln -s "$old" "$prod"
105106

106107
### Version-specific update instructions:
107108

109+
### 1.2 -> 1.3
110+
* SQL:
111+
* remove the `sitevars` table
112+
* `defaults/config.ini.default` has some new fields that need to be overriden:
113+
* `offset_UIDGID`
114+
* `offset_PIGID`
115+
* `offset_ORGGID`
116+
* `custom_user_mappings` can no longer match with just the 1st segment of the logged in user's UID, an exact match is required
117+
108118
### 1.2.0 -> 1.2.1
109119
* SQL:
110120
* Add new columns to the `requests` table:

‎defaults/config.ini.default‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ account_policy_url = "https://github.com" ; this can be external or a portal pag
1818
uri = "ldap://identity" ; URI of remote LDAP server
1919
user = "cn=admin,dc=unityhpc,dc=test" ; Admin bind DN LDAP user
2020
pass = "password" ; Admin bind password
21+
custom_user_mappings_dir = "deployment/custom_user_mappings" ; for internal use only
2122
basedn = "dc=unityhpc,dc=test" ; Base search DN
2223
user_ou = "ou=users,dc=unityhpc,dc=test" ; User organizational unit (may contain more than user group)
2324
user_group = "cn=unityusers,dc=unityhpc,dc=test" ; User group
@@ -26,6 +27,9 @@ pigroup_ou = "ou=pi_groups,dc=unityhpc,dc=test" ; PI Group organizational unit
2627
orggroup_ou = "ou=org_groups,dc=unityhpc,dc=test" ; ORG group organizational unit
2728
admin_group = "cn=web_admins,dc=unityhpc,dc=test" ; admin dn (members of this group are admins on the web portal)
2829
def_user_shell = "/bin/bash" ; Default shell for new users
30+
offset_UIDGID = 1000000 ; start point when allocating new UID/GID pairs for a new user
31+
offset_PIGID = 2000000 ; start point when allocating new GID for a new PI group
32+
offset_ORGGID = 3000000 ; start point when allocating new GID for a new org group
2933

3034
[sql]
3135
host = "sql" ; mariadb hostname

‎deployment/overrides/phpunit/config/config.ini‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[ldap]
2+
custom_user_mappings_dir = "test/custom_user_mappings"

‎resources/lib/UnityGroup.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ private function init()
464464
{
465465
$owner = $this->getOwner();
466466
assert(!$this->entry->exists());
467-
$nextGID = $this->LDAP->getNextPiGIDNumber($this->SQL);
467+
$nextGID = $this->LDAP->getNextPIGIDNumber();
468468
$this->entry->setAttribute("objectclass", UnityLDAP::POSIX_GROUP_CLASS);
469469
$this->entry->setAttribute("gidnumber", strval($nextGID));
470470
$this->entry->setAttribute("memberuid", array($owner->uid));

‎resources/lib/UnityLDAP.php‎

Lines changed: 74 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ class UnityLDAP extends ldapConn
2424
"top"
2525
);
2626

27-
private $custom_mappings_path = __DIR__ . "/../../deployment/custom_user_mappings";
27+
private $custom_mappings_path = CONFIG["ldap"]["custom_user_mappings_dir"];
28+
private $def_user_shell = CONFIG["ldap"]["def_user_shell"];
29+
private $offset_UIDGID = CONFIG["ldap"]["offset_UIDGID"];
30+
private $offset_PIGID = CONFIG["ldap"]["offset_PIGID"];
31+
private $offset_ORGGID = CONFIG["ldap"]["offset_ORGGID"];
2832

2933
// Instance vars for various ldapEntry objects
3034
private $baseOU;
@@ -34,7 +38,6 @@ class UnityLDAP extends ldapConn
3438
private $org_groupOU;
3539
private $adminGroup;
3640
private $userGroup;
37-
private $def_user_shell;
3841

3942
public function __construct()
4043
{
@@ -46,7 +49,6 @@ public function __construct()
4649
$this->org_groupOU = $this->getEntry(CONFIG["ldap"]["orggroup_ou"]);
4750
$this->adminGroup = $this->getEntry(CONFIG["ldap"]["admin_group"]);
4851
$this->userGroup = $this->getEntry(CONFIG["ldap"]["user_group"]);
49-
$this->def_user_shell = CONFIG["ldap"]["def_user_shell"];
5052
}
5153

5254
public function getUserOU()
@@ -84,104 +86,100 @@ public function getDefUserShell()
8486
return $this->def_user_shell;
8587
}
8688

87-
public function getNextUIDNumber($UnitySQL)
89+
public function getNextUIDGIDNumber($uid)
8890
{
89-
$max_uid = $UnitySQL->getSiteVar('MAX_UID');
90-
$new_uid = $max_uid + 1;
91-
92-
while ($this->IDNumInUse($new_uid)) {
93-
$new_uid++;
91+
$IDNumsInUse = array_merge($this->getAllUIDNumbersInUse(), $this->getAllGIDNumbersInUse());
92+
$start = $this->offset_UIDGID;
93+
$customIDMappings = $this->getCustomIDMappings();
94+
$customMappedID = $customIDMappings[$uid] ?? null;
95+
if (!is_null($customMappedID) && !in_array($customMappedID, $IDNumsInUse)) {
96+
return $customMappedID;
9497
}
95-
96-
$UnitySQL->updateSiteVar('MAX_UID', $new_uid);
97-
98-
return $new_uid;
98+
if (!is_null($customMappedID) && in_array($customMappedID, $IDNumsInUse)) {
99+
UnitySite::errorLog(
100+
"warning",
101+
"user '$uid' has a custom mapped IDNumber $customMappedID but it's already in use!",
102+
);
103+
}
104+
return $this->getNextIDNumber($start, $IDNumsInUse);
99105
}
100106

101-
public function getNextPiGIDNumber($UnitySQL)
107+
public function getNextPIGIDNumber()
102108
{
103-
$max_pigid = $UnitySQL->getSiteVar('MAX_PIGID');
104-
$new_pigid = $max_pigid + 1;
105-
106-
while ($this->IDNumInUse($new_pigid)) {
107-
$new_pigid++;
108-
}
109-
110-
$UnitySQL->updateSiteVar('MAX_PIGID', $new_pigid);
111-
112-
return $new_pigid;
109+
$IDNumsInUse = $this->getAllGIDNumbersInUse();
110+
$start = $this->offset_PIGID;
111+
return $this->getNextIDNumber($start, $IDNumsInUse);
113112
}
114113

115-
public function getNextOrgGIDNumber($UnitySQL)
114+
public function getNextOrgGIDNumber()
116115
{
117-
$max_gid = $UnitySQL->getSiteVar('MAX_GID');
118-
$new_gid = $max_gid + 1;
119-
120-
while ($this->IDNumInUse($new_gid)) {
121-
$new_gid++;
122-
}
123-
124-
$UnitySQL->updateSiteVar('MAX_GID', $new_gid);
125-
126-
return $new_gid;
116+
$IDNumsInUse = array_values($this->getCustomIDMappings());
117+
$start = $this->offset_ORGGID;
118+
return $this->getNextIDNumber($start, $IDNumsInUse);
127119
}
128120

129-
private function IDNumInUse($id)
121+
private function isIDNumberForbidden($id)
130122
{
131123
// 0-99 are probably going to be used for local system accounts instead of LDAP accounts
132124
// 100-999, 60000-64999 are reserved for debian packages
133-
if (($id <= 999) || ($id >= 60000 && $id <= 64999)) {
134-
return true;
135-
}
136-
$users = $this->userOU->getChildrenArray([], true);
137-
foreach ($users as $user) {
138-
if ($user["uidnumber"][0] == $id) {
139-
return true;
140-
}
141-
}
142-
$pi_groups = $this->pi_groupOU->getChildrenArray(["gidnumber"], true);
143-
foreach ($pi_groups as $pi_group) {
144-
if ($pi_group["gidnumber"][0] == $id) {
145-
return true;
146-
}
147-
}
148-
$groups = $this->groupOU->getChildrenArray(["gidnumber"], true);
149-
foreach ($groups as $group) {
150-
if ($group["gidnumber"][0] == $id) {
151-
return true;
152-
}
153-
}
125+
return (($id <= 999) || ($id >= 60000 && $id <= 64999));
126+
}
154127

155-
return false;
128+
private function getNextIDNumber($start, $IDNumsInUse)
129+
{
130+
// custom ID mappings are considered both UIDs and GIDs
131+
$IDNumsInUse = array_merge($IDNumsInUse, array_values($this->getCustomIDMappings()));
132+
$new_id = $start;
133+
while ($this->isIDNumberForbidden($new_id) || in_array($new_id, $IDNumsInUse)) {
134+
$new_id++;
135+
}
136+
return $new_id;
156137
}
157138

158-
public function getUnassignedID($uid, $UnitySQL)
139+
private function getCustomIDMappings()
159140
{
160-
$netid = strtok($uid, "_"); // extract netid
161-
// scrape all files in custom folder
141+
$output = [];
162142
$dir = new \DirectoryIterator($this->custom_mappings_path);
163143
foreach ($dir as $fileinfo) {
144+
$filename = $fileinfo->getFilename();
145+
if ($fileinfo->isDot() || ($filename == "README.md")) {
146+
continue;
147+
}
164148
if ($fileinfo->getExtension() == "csv") {
165-
// found csv file
166149
$handle = fopen($fileinfo->getPathname(), "r");
167-
while (($data = fgetcsv($handle, 1000, ",")) !== false) {
168-
$netid_match = $data[0];
169-
$uid_match = $data[1];
170-
171-
if ($uid == $netid_match || $netid == $netid_match) {
172-
// found a match
173-
if (!$this->IDNumInUse($uid_match)) {
174-
return $uid_match;
175-
}
176-
}
150+
while (($row = fgetcsv($handle, null, ",")) !== false) {
151+
array_push($output, $row);
177152
}
153+
} else {
154+
UnitySite::errorLog(
155+
"warning",
156+
"custom ID mapping file '$filename' ignored, extension != .csv",
157+
);
178158
}
179159
}
160+
$output_map = [];
161+
foreach ($output as [$uid, $uidNumber_str]) {
162+
$output_map[$uid] = intval($uidNumber_str);
163+
}
164+
return $output_map;
165+
}
180166

181-
// didn't find anything from existing mappings, use next available
182-
$next_uid = $this->getNextUIDNumber($UnitySQL);
167+
private function getAllUIDNumbersInUse()
168+
{
169+
// use baseOU for awareness of externally managed entries
170+
return array_map(
171+
fn($x) => $x["uidnumber"][0],
172+
$this->baseOU->getChildrenArray(["uidNumber"], true, "(objectClass=posixAccount)"),
173+
);
174+
}
183175

184-
return $next_uid;
176+
private function getAllGIDNumbersInUse()
177+
{
178+
// use baseOU for awareness of externally managed entries
179+
return array_map(
180+
fn($x) => $x["gidnumber"][0],
181+
$this->baseOU->getChildrenArray(["gidNumber"], true, "(objectClass=posixGroup)"),
182+
);
185183
}
186184

187185
public function getAllUsersUIDs()
@@ -235,7 +233,7 @@ public function getAllUsersAttributes($attributes)
235233
$user_attributes = $this->baseOU->getChildrenArray(
236234
$attributes,
237235
true, // recursive
238-
"objectClass=posixAccount"
236+
"(objectClass=posixAccount)"
239237
);
240238
foreach ($user_attributes as $i => $attributes) {
241239
if (!in_array($attributes["uid"][0], $include_uids)) {

‎resources/lib/UnitySQL.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ class UnitySQL
1111
private const TABLE_PAGES = "pages";
1212
private const TABLE_AUDIT_LOG = "audit_log";
1313
private const TABLE_ACCOUNT_DELETION_REQUESTS = "account_deletion_requests";
14-
private const TABLE_SITEVARS = "sitevars";
1514
private const TABLE_GROUP_ROLES = "groupRoles";
1615
private const TABLE_GROUP_TYPES = "groupTypes";
1716
private const TABLE_GROUP_ROLE_ASSIGNMENTS = "groupRoleAssignments";
@@ -319,29 +318,6 @@ public function deleteAccountDeletionRequest($uid)
319318
$stmt->execute();
320319
}
321320

322-
public function getSiteVar($name)
323-
{
324-
$stmt = $this->conn->prepare(
325-
"SELECT * FROM " . self::TABLE_SITEVARS . " WHERE name=:name"
326-
);
327-
$stmt->bindParam(":name", $name);
328-
329-
$stmt->execute();
330-
331-
return $stmt->fetchAll()[0]['value'];
332-
}
333-
334-
public function updateSiteVar($name, $value)
335-
{
336-
$stmt = $this->conn->prepare(
337-
"UPDATE " . self::TABLE_SITEVARS . " SET value=:value WHERE name=:name"
338-
);
339-
$stmt->bindParam(":name", $name);
340-
$stmt->bindParam(":value", $value);
341-
342-
$stmt->execute();
343-
}
344-
345321
public function getRole($uid, $group)
346322
{
347323
$table = self::TABLE_GROUP_ROLE_ASSIGNMENTS;

‎resources/lib/UnityUser.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function __toString()
6060
public function init($firstname, $lastname, $email, $org, $send_mail = true)
6161
{
6262
$ldapGroupEntry = $this->getGroupEntry();
63-
$id = $this->LDAP->getUnassignedID($this->uid, $this->SQL);
63+
$id = $this->LDAP->getNextUIDGIDNumber($this->uid);
6464
assert(!$ldapGroupEntry->exists());
6565
$ldapGroupEntry->setAttribute("objectclass", UnityLDAP::POSIX_GROUP_CLASS);
6666
$ldapGroupEntry->setAttribute("gidnumber", strval($id));

‎test/custom_user_mappings/test.csv‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
user2001_org998_test,555
2+
foobar0,1000000
3+
foobar1,1000001
4+
foobar2,1000002
5+
foobar3,1000003
6+
foobar4,1000004

0 commit comments

Comments
 (0)