Skip to content

Commit c0b7e1e

Browse files
bryank-csbryank-cs
authored
Add ability to cancel request, require confirmation of account policy, fix dubious redirect (#223)
* Allow new user to cancel requests * Add link to account policy for PIs * Add test case for cancellation * Minor improvements to phpunit test framework, redirection * Improved New Account UI Layout from Simon
1 parent 414b8c1 commit c0b7e1e

File tree

10 files changed

+251
-88
lines changed

10 files changed

+251
-88
lines changed

‎defaults/config.ini.default‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ url = "https://127.0.0.1:8000/" ; URL of the website
1212
description = "The Unity Web Portal is a lightweight HPC cluster front-end" ; Description of the website
1313
logo = "logo.png" ; path to logo file, in the webroot/assets/branding folder
1414
terms_of_service_url = "https://github.com" ; this can be external or a portal page created with "content management"
15+
account_policy_url = "https://github.com" ; this can be external or a portal page created with "content management"
1516

1617
[ldap]
1718
uri = "ldap://identity" ; URI of remote LDAP server

‎resources/lib/UnityGroup.php‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,41 @@ public function denyGroup($operator = null, $send_mail = true)
198198
}
199199
}
200200

201+
public function cancelGroupRequest($send_mail = true)
202+
{
203+
if (!$this->SQL->requestExists($this->getOwner()->getUID())) {
204+
return;
205+
}
206+
207+
$this->SQL->removeRequest($this->getOwner()->getUID());
208+
209+
if ($send_mail) {
210+
// send email to requestor
211+
$this->MAILER->sendMail(
212+
"admin",
213+
"group_request_cancelled"
214+
);
215+
}
216+
}
217+
218+
public function cancelGroupJoinRequest($user, $send_mail = true)
219+
{
220+
if (!$this->requestExists($user)) {
221+
return;
222+
}
223+
224+
$this->SQL->removeRequest($user->getUID(), $this->pi_uid);
225+
226+
if ($send_mail) {
227+
// send email to requestor
228+
$this->MAILER->sendMail(
229+
$this->getOwner()->getMail(),
230+
"group_join_request_cancelled",
231+
["group" => $this->pi_uid]
232+
);
233+
}
234+
}
235+
201236
// /**
202237
// * This method will delete the group, either by admin action or PI action
203238
// */
@@ -251,7 +286,7 @@ public function approveUser($new_user, $send_mail = true)
251286
$this->addUserToGroup($new_user);
252287

253288
// remove request, this will fail silently if the request doesn't exist
254-
$this->removeRequest($new_user->getUID());
289+
$this->SQL->removeRequest($new_user->getUID(), $this->pi_uid);
255290

256291
// send email to the requestor
257292
if ($send_mail) {
@@ -283,7 +318,7 @@ public function denyUser($new_user, $send_mail = true)
283318
}
284319

285320
// remove request, this will fail silently if the request doesn't exist
286-
$this->removeRequest($new_user->getUID());
321+
$this->SQL->removeRequest($new_user->getUID(), $this->pi_uid);
287322

288323
if ($send_mail) {
289324
// send email to the user
@@ -522,11 +557,6 @@ private function addRequest($uid)
522557
$this->SQL->addRequest($uid, $this->pi_uid);
523558
}
524559

525-
private function removeRequest($uid)
526-
{
527-
$this->SQL->removeRequest($uid, $this->pi_uid);
528-
}
529-
530560
//
531561
// Public helper functions
532562
//

‎resources/lib/UnitySQL.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class UnitySQL
2020

2121

2222
// FIXME this string should be changed to something more intuitive, requires production sql change
23-
private const REQUEST_BECOME_PI = "admin";
23+
public const REQUEST_BECOME_PI = "admin";
2424

2525
private $conn;
2626

‎resources/lib/UnitySite.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,8 @@ public static function die($x = null)
2626

2727
public static function redirect($destination)
2828
{
29-
if ($_SERVER["PHP_SELF"] != $destination) {
30-
header("Location: $destination");
31-
self::die("Redirect failed, click <a href='$destination'>here</a> to continue.");
32-
}
29+
header("Location: $destination");
30+
self::die("Redirect failed, click <a href='$destination'>here</a> to continue.");
3331
}
3432

3533
private static function headerResponseCode(int $code, string $reason)

‎resources/mail/group_join_request_cancelled.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
// This template is sent to the user cancelling the request
4+
$this->Subject = "Unity Account Request Cancelled";
5+
?>
6+
7+
<p>Hello,</p>
8+
9+
<p>Your request to join group '<?php echo $data["group"]; ?>' on the Unity Cluster has been cancelled per your request.
10+
</p>
11+

‎resources/mail/group_request_cancelled.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
// This template is sent to the user cancelling the request
4+
$this->Subject = "Unity PI Account Request Cancelled";
5+
?>
6+
7+
<p>Hello,</p>
8+
9+
<p>Your request for a PI account on the Unity Cluster has been cancelled per your request.</p>
10+

‎resources/templates/header.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
}
1212

1313
if (isset($SSO)) {
14-
if (!$_SESSION["user_exists"]) {
14+
if (!$_SESSION["user_exists"] && !str_ends_with($_SERVER['PHP_SELF'], "/panel/new_account.php")) {
1515
UnitySite::redirect($CONFIG["site"]["prefix"] . "/panel/new_account.php");
1616
}
1717
}

‎test/functional/CancelRequestTest.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
use PHPUnit\Framework\TestCase;
4+
use UnityWebPortal\lib\exceptions\PhpUnitNoDieException;
5+
6+
class CancelRequestTest extends TestCase
7+
{
8+
private function assertNumberGroupRequests(int $x)
9+
{
10+
global $USER, $SQL;
11+
$this->assertEquals($x, count($SQL->getRequestsByUser($USER->getUID())));
12+
}
13+
14+
public function testCancelPIRequest()
15+
{
16+
global $USER, $SQL;
17+
switchUser(...getNonExistentUser());
18+
// First create a request
19+
try {
20+
http_post(
21+
__DIR__ . "/../../webroot/panel/new_account.php",
22+
["new_user_sel" => "pi", "eula" => "agree", "confirm_pi" => "agree"]
23+
);
24+
} catch (PhpUnitNoDieException $e) {
25+
// Ignore the exception from http_post
26+
}
27+
28+
$this->assertNumberGroupRequests(1);
29+
30+
// Now try to cancel it
31+
try {
32+
http_post(
33+
__DIR__ . "/../../webroot/panel/new_account.php",
34+
["cancel" => "true"] # value of cancel is arbitrary
35+
);
36+
} catch (PhpUnitNoDieException $e) {
37+
// Ignore the exception from http_post
38+
}
39+
40+
$this->assertNumberGroupRequests(0);
41+
}
42+
43+
public function testCancelGroupJoinRequest()
44+
{
45+
global $USER, $SQL;
46+
switchUser(...getNonExistentUser());
47+
48+
try {
49+
http_post(
50+
__DIR__ . "/../../webroot/panel/new_account.php",
51+
["new_user_sel" => "not_pi", "eula" => "agree", "pi" => getExistingPI()]
52+
);
53+
} catch (PhpUnitNoDieException $e) {
54+
// Ignore the exception from http_post
55+
}
56+
57+
$this->assertNumberGroupRequests(1);
58+
59+
// Now try to cancel it
60+
try {
61+
http_post(
62+
__DIR__ . "/../../webroot/panel/new_account.php",
63+
["cancel" => "true"] # value of cancel is arbitrary
64+
);
65+
} catch (PhpUnitNoDieException $e) {
66+
// Ignore the exception from http_post
67+
}
68+
69+
$this->assertNumberGroupRequests(0);
70+
}
71+
}

‎test/phpunit-bootstrap.php‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,28 +76,34 @@ function switchUser(
7676
function http_post(string $phpfile, array $post_data): void
7777
{
7878
global $CONFIG, $REDIS, $LDAP, $SQL, $MAILER, $WEBHOOK, $GITHUB, $SITE, $SSO, $OPERATOR, $USER, $SEND_PIMESG_TO_ADMINS, $LOC_HEADER, $LOC_FOOTER;
79+
$_PREVIOUS_SERVER = $_SERVER;
7980
$_SERVER["REQUEST_METHOD"] = "POST";
81+
$_SERVER["PHP_SELF"] = preg_replace("/.*webroot\//", "/", $phpfile);
8082
$_POST = $post_data;
8183
ob_start();
8284
try {
8385
include $phpfile;
8486
} finally {
8587
ob_get_clean(); // discard output
8688
unset($_POST);
87-
unset($_SERVER["REQUEST_METHOD"]);
89+
$_SERVER = $_PREVIOUS_SERVER;
8890
}
8991
}
9092

91-
function http_get(string $phpfile): void
93+
function http_get(string $phpfile, array $get_data = array()): void
9294
{
9395
global $CONFIG, $REDIS, $LDAP, $SQL, $MAILER, $WEBHOOK, $GITHUB, $SITE, $SSO, $OPERATOR, $USER, $SEND_PIMESG_TO_ADMINS, $LOC_HEADER, $LOC_FOOTER;
96+
$_PREVIOUS_SERVER = $_SERVER;
9497
$_SERVER["REQUEST_METHOD"] = "GET";
98+
$_SERVER["PHP_SELF"] = preg_replace("/.*webroot\//", "/", $phpfile);
99+
$_GET = $get_data;
95100
ob_start();
96101
try {
97102
include $phpfile;
98103
} finally {
99104
ob_get_clean(); // discard output
100-
unset($_SERVER["REQUEST_METHOD"]);
105+
unset($_GET);
106+
$_PREVIOUS_SERVER = $_SERVER;
101107
}
102108
}
103109

@@ -153,10 +159,15 @@ function getUserIsPIHasAtLeastOneMember()
153159

154160
function getNonExistentUser()
155161
{
156-
return ["user1@nonexistent.test", "foo", "bar", "user1@nonexistent.test"];
162+
return ["user2000@org2.test", "foo", "bar", "user2000@org2.test"];
157163
}
158164

159165
function getAdminUser()
160166
{
161167
return ["[email protected]", "foo", "bar", "[email protected]"];
162168
}
169+
170+
function getExistingPI()
171+
{
172+
return "pi_user1005_org3_test";
173+
}

0 commit comments

Comments
 (0)