Skip to content

"view as user" test, rework uses of die() #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 43 commits into from
Apr 30, 2025
Merged

Conversation

simonLeary42
Copy link
Collaborator

@simonLeary42 simonLeary42 commented Apr 29, 2025

  • added new helper functions
    • die: does the same thing unless phpunit is running in which case it throws a special exception
    • errorLog: writes to error log including logged in user, IP address, and backtrace
    • badRequest: sends HTTP header 400, writes to error log, and dies
    • forbidden: sends HTTP header 403, writes to error log, and dies
  • replaced die() in authorization checks with UnitySite::forbidden
  • replaced die() in bad request checks with UnitySite::badRequest
  • replaced silly redirect/include chainadmin/user-mgmt.php -> panel/ -> panel/index.php -> panel/account.php with a direct link admin/user-mgmt.php -> panel/account.php
  • moved the clearView logic in header.php up to the top before nonexistent users are redirected to new_account.php, this allows an admin to viewUser a nonexistent user and not get stuck

testing:

  • updated switchUser to prevent tests from polluting each other's sessions
  • renamed post to http_post, added http_get
  • added a pre-commit hook to make sure that all occurrences of die / exit are replaced with UnitySite::die
  • added tests for "view as user" functionality
  • modified http_post so that it always discards output
    • the idea was to avoid discarding important error messages, but it's more annoying than anything, interferes with PHPUnit expectException, and hasn't come in handy

pre-commit hook tested:

  • exit(), die(), exit, die, die ()
  • phpcbf disabled because it automatically turns die () into die()
$ git diff --staged | cat
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index c1d232a..017c363 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -12,14 +12,14 @@ repos:
   #   hooks:
   #     - id: trailing-whitespace
   #     - id: end-of-file-fixer
-  - repo: local
-    hooks:
-      - id: phpcbf
-        name: PHP Code Beautifier and Fixer
-        entry: phpcbf
-        language: system
-        files: \.php$
-        args: [--standard=PSR2, --colors]
+  # - repo: local
+  #   hooks:
+  #     - id: phpcbf
+  #       name: PHP Code Beautifier and Fixer
+  #       entry: phpcbf
+  #       language: system
+  #       files: \.php$
+  #       args: [--standard=PSR2, --colors]

   # linters (work required) ########################################################################
   # - repo: https://github.com/pre-commit/pre-commit-hooks
diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php
index 02b9cbd..ca592c8 100644
--- a/resources/lib/UnityGroup.php
+++ b/resources/lib/UnityGroup.php
@@ -30,7 +30,7 @@ class UnityGroup
     public function __construct($pi_uid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK)
     {
         $this->pi_uid = $pi_uid;
-
+        die();
         $this->LDAP = $LDAP;
         $this->SQL = $SQL;
         $this->MAILER = $MAILER;
diff --git a/resources/lib/UnityLDAP.php b/resources/lib/UnityLDAP.php
index fc44cd4..a2c9dd8 100644
--- a/resources/lib/UnityLDAP.php
+++ b/resources/lib/UnityLDAP.php
@@ -60,7 +60,7 @@ class UnityLDAP extends ldapConn
         $def_user_shell
     ) {
         parent::__construct($host, $dn, $pass);
-
+        exit();
         $this->STR_USEROU = $user_ou;
         $this->STR_GROUPOU = $group_ou;
         $this->STR_PIGROUPOU = $pigroup_ou;
diff --git a/resources/lib/UnityOrg.php b/resources/lib/UnityOrg.php
index 03f8581..ee32011 100644
--- a/resources/lib/UnityOrg.php
+++ b/resources/lib/UnityOrg.php
@@ -17,7 +17,7 @@ class UnityOrg
     public function __construct($orgid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK)
     {
         $this->orgid = $orgid;
-
+        die   ();
         $this->LDAP = $LDAP;
         $this->SQL = $SQL;
         $this->MAILER = $MAILER;
diff --git a/resources/lib/UnitySQL.php b/resources/lib/UnitySQL.php
index c583582..bf916ef 100644
--- a/resources/lib/UnitySQL.php
+++ b/resources/lib/UnitySQL.php
@@ -27,6 +27,7 @@ class UnitySQL

     public function __construct($db_host, $db, $db_user, $db_pass)
     {
+        exit;
         $this->conn = new PDO("mysql:host=" . $db_host . ";dbname=" . $db, $db_user, $db_pass);
         $this->conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
     }
diff --git a/resources/lib/UnityUser.php b/resources/lib/UnityUser.php
index a2e839d..1fd4622 100644
--- a/resources/lib/UnityUser.php
+++ b/resources/lib/UnityUser.php
@@ -21,7 +21,7 @@ class UnityUser
     public function __construct($uid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK)
     {
         $this->uid = $uid;
-
+        die;
         $this->LDAP = $LDAP;
         $this->SQL = $SQL;
         $this->MAILER = $MAILER;
simon@wheatley:portal $ git commit -m foobar
PHP CodeSniffer..........................................................Failed
- hook id: phpcs
- exit code: 2

FILE: /Users/simon/unity/portal/resources/lib/UnityOrg.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 20 | ERROR | [x] Space before opening parenthesis of function call prohibited
-------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------

Time: 92ms; Memory: 10MB

php -l...................................................................Passed
Assert no die()/exit()...................................................Failed
- hook id: assert-no-die-exit
- exit code: 1

die is not allowed! use UnitySite::die() instead.
resources/lib/UnityGroup.php:33:        die();
resources/lib/UnityOrg.php:20:        die   ();
exit is not allowed! use UnitySite::die() instead.
resources/lib/UnityLDAP.php:63:        exit();
resources/lib/UnitySQL.php:30:        exit;
die is not allowed! use UnitySite::die() instead.
resources/lib/UnityUser.php:24:        die;

UnitySite::forbidden tested:

firefox:

image

chrome:

image

this page only shows when absolutely nothing was returned by the webserver. if I add echo "<html></html>" before die(), I get a white screen in both chrome and firefox.

and in error log:

[Tue Apr 29 22:37:12.337653 2025] [php:notice] [pid 12] [client 192.168.65.1:26684] forbidden: {"message":"not an admin","REMOTE_USER":"[email protected]","REMOTE_ADDR":"192.168.65.1","trace":"#0 \\/var\\/www\\/unity-web-portal\\/resources\\/lib\\/UnitySite.php(66): UnityWebPortal\\\\lib\\\\UnitySite::errorLog()\\n#1 \\/var\\/www\\/unity-web-portal\\/webroot\\/admin\\/user-mgmt.php(8): UnityWebPortal\\\\lib\\\\UnitySite::forbidden()\\n#2 {main}"}

header.php viewUser/clearUser refactor, silly redirect chain tested:

Screen.Recording.2025-04-30.at.9.44.03.AM.mov

@simonLeary42 simonLeary42 merged commit c21d9c3 into main Apr 30, 2025
2 of 3 checks passed
simonLeary42 added a commit that referenced this pull request Apr 30, 2025
* replace die with exception

* use if/then instead of die

* set unique session cache location for each phpunit run

* viewAsUser test

* UnitySite not static

* remove pointless file

* ajax php don't initialize entire stack

* every switchUser has a fresh session by default

* get post always discard output

* Revert "UnitySite not static"

This reverts commit 01e7581.

* add UnitySite::die

* Revert "replace die with exception"

This reverts commit fa5d021.

* add functions errorLog, badRequest

* update ViewAsUserTest

* get test working

* add exceptions

* delete comments

* unused import

* rename post to http_post, get to http_get

* fix die

* add pre commit check for usage of die

* no color in die checker

* dont print to stderr

* ignore phpunit result cache

* don't allow exit() either

* refactor

* show both die and exit

* get -> http_get

* revert conditional

* revert conditional 2

* add message

* remove unused exception class

* exclude UnitySite from die check

* use unauthorized function, don't print anything to use

* allow empty die(), rename unauthorized to forbidden

* don't explode trace

* remove broken magic response code reason

* httpResponseCode private

* more tests

* allow undefined server protocol

* more tests

* clear view before redirect nonexistent user

* remove comment
@simonLeary42 simonLeary42 deleted the die-redirect-exception2 branch June 6, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant