Skip to content

Commit 4171c3c

Browse files
simonLeary42simonLeary42
committed
Merge branch 'main' into testing6
2 parents 33c4d86 + 513d875 commit 4171c3c

31 files changed

+322
-151
lines changed

‎.pre-commit-config.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,9 @@ repos:
4848
language: system
4949
files: \.php$
5050
args: [-l]
51+
- id: assert-no-die-exit
52+
name: Assert no die()/exit()
53+
entry: ./test/assert-no-die-exit.bash
54+
language: system
55+
files: \.php$
56+
exclude: resources/lib/UnitySite\.php$

‎defaults/config.ini.default‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ user_ou = "ou=users,dc=unityhpc,dc=test" ; User organizational unit
2222
group_ou = "ou=groups,dc=unityhpc,dc=test" ; Group organizational unit
2323
pigroup_ou = "ou=pi_groups,dc=unityhpc,dc=test" ; PI Group organizational unit
2424
orggroup_ou = "ou=org_groups,dc=unityhpc,dc=test" ; ORG group organizational unit
25-
admin_group = "cn=sudo,dc=unityhpc,dc=test" ; admin dn (members of this group are admins on the web portal)
25+
admin_group = "cn=web_admins,dc=unityhpc,dc=test" ; admin dn (members of this group are admins on the web portal)
2626
def_user_shell = "/bin/bash" ; Default shell for new users
2727

2828
[sql]

‎resources/lib/UnityLDAP.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@
1111
class UnityLDAP extends ldapConn
1212
{
1313
// User Specific Constants
14-
private const ID_MAP = array(1000, 9999);
15-
private const PI_ID_MAP = array(10000, 19999);
16-
private const ORG_ID_MAP = array(20000, 29999);
17-
1814
private const RDN = "cn"; // The defauls RDN for LDAP entries is set to "common name"
1915

2016
public const POSIX_ACCOUNT_CLASS = array(
@@ -315,21 +311,25 @@ public function getAllOrgGroups($UnitySQL, $UnityMailer, $UnityRedis, $UnityWebh
315311

316312
public function getUserEntry($uid)
317313
{
314+
$uid = ldap_escape($uid, LDAP_ESCAPE_DN);
318315
return $this->getEntry(unityLDAP::RDN . "=$uid," . $this->STR_USEROU);
319316
}
320317

321318
public function getGroupEntry($gid)
322319
{
320+
$uid = ldap_escape($gid, LDAP_ESCAPE_DN);
323321
return $this->getEntry(unityLDAP::RDN . "=$gid," . $this->STR_GROUPOU);
324322
}
325323

326324
public function getPIGroupEntry($gid)
327325
{
326+
$uid = ldap_escape($gid, LDAP_ESCAPE_DN);
328327
return $this->getEntry(unityLDAP::RDN . "=$gid," . $this->STR_PIGROUPOU);
329328
}
330329

331330
public function getOrgGroupEntry($gid)
332331
{
332+
$uid = ldap_escape($gid, LDAP_ESCAPE_DN);
333333
return $this->getEntry(unityLDAP::RDN . "=$gid," . $this->STR_ORGGROUPOU);
334334
}
335335
}

‎resources/lib/UnitySQL.php‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ class UnitySQL
88
{
99
private const TABLE_REQS = "requests";
1010
private const TABLE_NOTICES = "notices";
11-
private const TABLE_SSOLOG = "sso_log";
1211
private const TABLE_PAGES = "pages";
13-
private const TABLE_EVENTS = "events";
1412
private const TABLE_AUDIT_LOG = "audit_log";
1513
private const TABLE_ACCOUNT_DELETION_REQUESTS = "account_deletion_requests";
1614
private const TABLE_SITEVARS = "sitevars";
@@ -21,7 +19,8 @@ class UnitySQL
2119
private const TABLE_GROUP_JOIN_REQUESTS = "groupJoinRequests";
2220

2321

24-
private const REQUEST_ADMIN = "admin";
22+
// FIXME this string should be changed to something more intuitive, requires production sql change
23+
private const REQUEST_BECOME_PI = "admin";
2524

2625
private $conn;
2726

@@ -39,7 +38,7 @@ public function getConn()
3938
//
4039
// requests table methods
4140
//
42-
public function addRequest($requestor, $dest = self::REQUEST_ADMIN)
41+
public function addRequest($requestor, $dest = self::REQUEST_BECOME_PI)
4342
{
4443
if ($this->requestExists($requestor, $dest)) {
4544
return;
@@ -54,7 +53,7 @@ public function addRequest($requestor, $dest = self::REQUEST_ADMIN)
5453
$stmt->execute();
5554
}
5655

57-
public function removeRequest($requestor, $dest = self::REQUEST_ADMIN)
56+
public function removeRequest($requestor, $dest = self::REQUEST_BECOME_PI)
5857
{
5958
if (!$this->requestExists($requestor, $dest)) {
6059
return;
@@ -69,7 +68,7 @@ public function removeRequest($requestor, $dest = self::REQUEST_ADMIN)
6968
$stmt->execute();
7069
}
7170

72-
public function removeRequests($dest = self::REQUEST_ADMIN)
71+
public function removeRequests($dest = self::REQUEST_BECOME_PI)
7372
{
7473
$stmt = $this->conn->prepare(
7574
"DELETE FROM " . self::TABLE_REQS . " WHERE request_for=:request_for"
@@ -79,7 +78,7 @@ public function removeRequests($dest = self::REQUEST_ADMIN)
7978
$stmt->execute();
8079
}
8180

82-
public function requestExists($requestor, $dest = self::REQUEST_ADMIN)
81+
public function requestExists($requestor, $dest = self::REQUEST_BECOME_PI)
8382
{
8483
$stmt = $this->conn->prepare(
8584
"SELECT * FROM " . self::TABLE_REQS . " WHERE uid=:uid and request_for=:request_for"
@@ -92,7 +91,7 @@ public function requestExists($requestor, $dest = self::REQUEST_ADMIN)
9291
return count($stmt->fetchAll()) > 0;
9392
}
9493

95-
public function getRequests($dest = self::REQUEST_ADMIN)
94+
public function getRequests($dest = self::REQUEST_BECOME_PI)
9695
{
9796
$stmt = $this->conn->prepare(
9897
"SELECT * FROM " . self::TABLE_REQS . " WHERE request_for=:request_for"
@@ -234,18 +233,6 @@ public function editPage($id, $content, $operator)
234233
);
235234
}
236235

237-
public function addEvent($operator, $action, $entity)
238-
{
239-
$stmt = $this->conn->prepare(
240-
"INSERT INTO " . self::TABLE_EVENTS . " (operator, action, entity) VALUE (:operator, :action, :entity)"
241-
);
242-
$stmt->bindParam(":operator", $operator);
243-
$stmt->bindParam(":action", $action);
244-
$stmt->bindParam(":entity", $entity);
245-
246-
$stmt->execute();
247-
}
248-
249236
// audit log table methods
250237
public function addLog($operator, $operator_ip, $action_type, $recipient)
251238
{

‎resources/lib/UnitySite.php‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,70 @@
33
namespace UnityWebPortal\lib;
44

55
use phpseclib3\Crypt\PublicKeyLoader;
6+
use UnityWebPortal\lib\exceptions\PhpUnitNoDieException;
67

78
class UnitySite
89
{
10+
public static function die($x = null)
11+
{
12+
if (@$GLOBALS["PHPUNIT_NO_DIE_PLEASE"] == true) {
13+
if (is_null($x)) {
14+
throw new PhpUnitNoDieException();
15+
} else {
16+
throw new PhpUnitNoDieException($x);
17+
}
18+
} else {
19+
if (is_null($x)) {
20+
die();
21+
} else {
22+
die($x);
23+
}
24+
}
25+
}
26+
927
public static function redirect($destination)
1028
{
1129
if ($_SERVER["PHP_SELF"] != $destination) {
1230
header("Location: $destination");
13-
die("Redirect failed, click <a href='$destination'>here</a> to continue.");
31+
self::die("Redirect failed, click <a href='$destination'>here</a> to continue.");
1432
}
1533
}
1634

35+
private static function headerResponseCode(int $code, string $reason)
36+
{
37+
$protocol = @$_SERVER["SERVER_PROTOCOL"] ?? "HTTP/1.1";
38+
$msg = $protocol . " " . strval($code) . " " . $reason;
39+
header($msg, true, $code);
40+
}
41+
42+
public static function errorLog(string $title, string $message)
43+
{
44+
error_log(
45+
"$title: " . json_encode(
46+
[
47+
"message" => $message,
48+
"REMOTE_USER" => @$_SERVER["REMOTE_USER"],
49+
"REMOTE_ADDR" => @$_SERVER["REMOTE_ADDR"],
50+
"trace" => (new \Exception())->getTraceAsString()
51+
]
52+
)
53+
);
54+
}
55+
56+
public static function badRequest($message)
57+
{
58+
self::headerResponseCode(400, "bad request");
59+
self::errorLog("bad request", $message);
60+
self::die();
61+
}
62+
63+
public static function forbidden($message)
64+
{
65+
self::headerResponseCode(403, "forbidden");
66+
self::errorLog("forbidden", $message);
67+
self::die();
68+
}
69+
1770
public static function removeTrailingWhitespace($arr)
1871
{
1972
$out = array();

‎resources/lib/UnityUser.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ public function getHomeDir($ignorecache = false)
538538
}
539539

540540
/**
541-
* Checks if the current account is an admin (in the sudo group)
541+
* Checks if the current account is an admin
542542
*
543543
* @return boolean true if admin, false if not
544544
*/

‎resources/lib/exceptions/PhpUnitNoDieException.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
namespace UnityWebPortal\lib\exceptions;
3+
4+
class PhpUnitNoDieException extends \Exception
5+
{
6+
}

‎resources/templates/header.php‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
use UnityWebPortal\lib\UnitySite;
44

5+
if ((@$_SESSION["is_admin"] ?? false) == true
6+
&& $_SERVER["REQUEST_METHOD"] == "POST"
7+
&& (@$_POST["form_name"] ?? null) == "clearView"
8+
) {
9+
unset($_SESSION["viewUser"]);
10+
UnitySite::redirect($CONFIG["site"]["prefix"] . "/admin/user-mgmt.php");
11+
}
12+
513
if (isset($SSO)) {
614
if (!$_SESSION["user_exists"]) {
715
UnitySite::redirect($CONFIG["site"]["prefix"] . "/panel/new_account.php");
@@ -116,23 +124,20 @@
116124
<main>
117125

118126
<?php
119-
if (isset($_SESSION["is_admin"]) && $_SESSION["is_admin"]) {
120-
if ($_SERVER["REQUEST_METHOD"] == "POST" && isset($_POST["form_name"]) && $_POST["form_name"] == "clearView") {
121-
unset($_SESSION["viewUser"]);
122-
UnitySite::redirect($CONFIG["site"]["prefix"] . "/admin/user-mgmt.php");
123-
}
124-
125-
if (isset($_SESSION["viewUser"])) {
126-
echo "<div id='viewAsBar'>";
127-
echo "<span>You are accessing the web portal as the user <strong>" .
128-
$_SESSION["viewUser"] . "</strong></span>";
129-
echo
130-
"<form method='POST' action=''>
131-
<input type='hidden' name='form_name' value='clearView'>
132-
<input type='hidden' name='uid' value='" . $_SESSION["viewUser"] . "'>
133-
<input type='submit' value='Return to My User'>
134-
</form>";
135-
echo "</div>";
136-
}
127+
if (isset($_SESSION["is_admin"])
128+
&& $_SESSION["is_admin"]
129+
&& isset($_SESSION["viewUser"])
130+
) {
131+
$viewUser = $_SESSION["viewUser"];
132+
echo "
133+
<div id='viewAsBar'>
134+
<span>You are accessing the web portal as the user <strong>$viewUser</strong></span>
135+
<form method='POST' action=''>
136+
<input type='hidden' name='form_name' value='clearView'>
137+
<input type='hidden' name='uid' value='$viewUser'>
138+
<input type='submit' value='Return to My User'>
139+
</form>
140+
</div>
141+
";
137142
}
138-
?>
143+
?>

‎test/assert-no-die-exit.bash‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#!/bin/bash
2+
set -euo pipefail
3+
if [[ $# -lt 1 ]]; then
4+
echo "at least one argument required"
5+
exit 1
6+
fi
7+
8+
rc=0
9+
10+
# --color=never because magit git output log doesn't support it
11+
die_occurrences="$(
12+
grep -H --color=never --line-number -P '\bdie\s*[\(;]' "$@" | grep -v -P 'UnitySite::die'
13+
)" || true
14+
if [ -n "$die_occurrences" ]; then
15+
echo "die is not allowed! use UnitySite::die() instead."
16+
echo "$die_occurrences"
17+
rc=1
18+
fi
19+
20+
# --color=never because magit git output log doesn't support it
21+
exit_occurrences="$(grep -H --color=never --line-number -P '\bexit\s*[\(;]' "$@")" || true
22+
if [ -n "$exit_occurrences" ]; then
23+
echo "exit is not allowed! use UnitySite::die() instead."
24+
echo "$exit_occurrences"
25+
rc=1
26+
fi
27+
28+
exit "$rc"

‎test/functional/AccountDeletionRequestTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ public function testRequestAccountDeletionUserHasNoGroups()
3838
$this->assertEmpty($USER->getGroups());
3939
$this->assertNumberAccountDeletionRequests(0);
4040
try {
41-
post(
41+
http_post(
4242
__DIR__ . "/../../webroot/panel/account.php",
4343
["form_type" => "account_deletion_request"]
4444
);
4545
$this->assertNumberAccountDeletionRequests(1);
46-
post(
46+
http_post(
4747
__DIR__ . "/../../webroot/panel/account.php",
4848
["form_type" => "account_deletion_request"]
4949
);
@@ -62,7 +62,7 @@ public function testRequestAccountDeletionUserHasGroup()
6262
$this->assertNotEmpty($USER->getGroups());
6363
$this->assertNumberAccountDeletionRequests(0);
6464
try {
65-
post(
65+
http_post(
6666
__DIR__ . "/../../webroot/panel/account.php",
6767
["form_type" => "account_deletion_request"]
6868
);

0 commit comments

Comments
 (0)