From c675fb9d30f19c4dab571fd69e109985e39e42aa Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Mon, 7 Apr 2025 13:14:46 -0400 Subject: [PATCH 01/15] improve ssh key validation --- resources/lib/UnitySite.php | 12 ++++++++++++ webroot/js/ajax/ssh_validate.php | 8 ++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index 2bedf8a6..961b5c02 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -49,6 +49,18 @@ public static function getGithubKeys($username) public static function testValidSSHKey($key_str) { + $key_str = trim($key_str); + if ($key_str == ""){ + return false; + } + // PHP warning when key_str is digits: Attempt to read property "keys" on int + if (preg_match("/^[0-9]+$/", $key_str)) { + return false; + } + // PHP warning when key_str is JSON: Undefined property: stdClass::$keys + if (!is_null(@json_decode($key_str))){ + return false; + } try { PublicKeyLoader::load($key_str); return true; diff --git a/webroot/js/ajax/ssh_validate.php b/webroot/js/ajax/ssh_validate.php index 7180defb..2fd32173 100644 --- a/webroot/js/ajax/ssh_validate.php +++ b/webroot/js/ajax/ssh_validate.php @@ -2,11 +2,7 @@ require "../../../resources/autoload.php"; +use UnityWebPortal\lib\UnitySite; use phpseclib3\Crypt\PublicKeyLoader; -try { - PublicKeyLoader::load($_POST['key'], $password = false); - echo "true"; -} catch (Exception $e) { - echo "false"; -} +echo (UnitySite::testValidSSHKey($_POST["key"]) ? "true" : "false"); From da45584c754e45c53401e930ffff848cd7f7c8ea Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Mon, 7 Apr 2025 13:42:22 -0400 Subject: [PATCH 02/15] fix style --- resources/lib/UnitySite.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lib/UnitySite.php b/resources/lib/UnitySite.php index 961b5c02..6de0cdeb 100644 --- a/resources/lib/UnitySite.php +++ b/resources/lib/UnitySite.php @@ -50,7 +50,7 @@ public static function getGithubKeys($username) public static function testValidSSHKey($key_str) { $key_str = trim($key_str); - if ($key_str == ""){ + if ($key_str == "") { return false; } // PHP warning when key_str is digits: Attempt to read property "keys" on int @@ -58,7 +58,7 @@ public static function testValidSSHKey($key_str) return false; } // PHP warning when key_str is JSON: Undefined property: stdClass::$keys - if (!is_null(@json_decode($key_str))){ + if (!is_null(@json_decode($key_str))) { return false; } try { From 7433b59465725b2f3727bb1413d0e406b12100f8 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 12 Apr 2025 14:41:33 -0400 Subject: [PATCH 03/15] comment out phpunit config for earlier version --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index b3ac75ad..7bcb38b4 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -3,7 +3,7 @@ failOnWarning="true" failOnDeprecation="true" failOnNotice="true" - restrictWarnings="true" + > From a61f99e8a0b2f99010cc060a3f6b55b5e10f9a88 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 12 Apr 2025 14:55:12 -0400 Subject: [PATCH 04/15] xml syntax --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index 7bcb38b4..e16564c3 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,9 +1,9 @@ + > From 7d462f0c1a48eac2fcc15c418e058b0c025d435a Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 12:16:31 -0400 Subject: [PATCH 05/15] add test for ajax ssh pubkey validation --- test/unit/AjaxSshValidateTest.php | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/unit/AjaxSshValidateTest.php diff --git a/test/unit/AjaxSshValidateTest.php b/test/unit/AjaxSshValidateTest.php new file mode 100644 index 00000000..6f670a43 --- /dev/null +++ b/test/unit/AjaxSshValidateTest.php @@ -0,0 +1,35 @@ +assertEquals("true", $output); + } else { + $this->assertEquals("false", $output); + } + } +} From deb35a9fb57f25c7c27ea1a9e33b7872c81f2167 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 12:27:02 -0400 Subject: [PATCH 06/15] require must be absolute since included file in deeper directory than test --- webroot/js/ajax/ssh_validate.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/webroot/js/ajax/ssh_validate.php b/webroot/js/ajax/ssh_validate.php index 2fd32173..79510460 100644 --- a/webroot/js/ajax/ssh_validate.php +++ b/webroot/js/ajax/ssh_validate.php @@ -1,8 +1,5 @@ Date: Sat, 19 Apr 2025 12:31:00 -0400 Subject: [PATCH 07/15] always close output buffer --- test/unit/AjaxSshValidateTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/unit/AjaxSshValidateTest.php b/test/unit/AjaxSshValidateTest.php index 6f670a43..d320ca5e 100644 --- a/test/unit/AjaxSshValidateTest.php +++ b/test/unit/AjaxSshValidateTest.php @@ -24,8 +24,11 @@ public function testSshValidate(bool $is_valid, string $pubkey) $_SERVER["REQUEST_METHOD"] = "POST"; $_POST["key"] = $pubkey; ob_start(); - include __DIR__ . "/../../webroot/js/ajax/ssh_validate.php"; - $output = ob_get_clean(); + try { + include __DIR__ . "/../../webroot/js/ajax/ssh_validate.php"; + } finally { + $output = ob_get_clean(); + } if ($is_valid) { $this->assertEquals("true", $output); } else { From 9f9ad9e7417f4c1079e56ad4764482a9899133cf Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 12:38:00 -0400 Subject: [PATCH 08/15] make sure php extensions installed in phpunit github action --- .github/workflows/phpunit.yml | 13 +++++++++++-- tools/docker-dev/web/Dockerfile | 3 ++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 5481a007..b5eb296f 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -7,5 +7,14 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: php-actions/composer@v6 - - uses: php-actions/phpunit@v3 + - name: setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: "8.3" + # php extensions also listed in tools/docker-dev/web/Dockerfile + extensions: curl,mysql,ldap,pdo,redis,cli + tools: composer:v2 + - name: Install dependencies + run: composer install --prefer-dist --no-progress + - name: Run PHPUnit tests + run: vendor/bin/phpunit diff --git a/tools/docker-dev/web/Dockerfile b/tools/docker-dev/web/Dockerfile index db322179..5f171085 100644 --- a/tools/docker-dev/web/Dockerfile +++ b/tools/docker-dev/web/Dockerfile @@ -2,6 +2,7 @@ FROM ubuntu:20.04 # Web Server Setup ARG DEBIAN_FRONTEND=noninteractive +# php extensions also listed in .github/workflows/phpunit.yml RUN apt-get update && apt-get install -y \ apache2 \ apache2-utils \ @@ -23,4 +24,4 @@ RUN sed -i '/display_errors/c\display_errors = on' /etc/php/7.4/apache2/php.ini # Start apache2 server EXPOSE 80 -CMD ["apache2ctl", "-D", "FOREGROUND"] \ No newline at end of file +CMD ["apache2ctl", "-D", "FOREGROUND"] From ffd90048d66f193216aba5b17451b6f593ecbe32 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 12:45:03 -0400 Subject: [PATCH 09/15] add phpunit dependency --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index 606629ac..77028b7d 100644 --- a/composer.json +++ b/composer.json @@ -4,5 +4,8 @@ "phpseclib/phpseclib": "3.0.43", "phpmailer/phpmailer": "6.6.4", "hakasapl/phpopenldaper": "1.0.5" + }, + "require-dev": { + "phpunit/phpunit": "^12.1" } } From 945c47b6e91f0aad63bd0179a10bcde783c03cd9 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 12:49:08 -0400 Subject: [PATCH 10/15] remove autoload from ssh_validate --- webroot/js/ajax/ssh_validate.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webroot/js/ajax/ssh_validate.php b/webroot/js/ajax/ssh_validate.php index 79510460..62e0e47f 100644 --- a/webroot/js/ajax/ssh_validate.php +++ b/webroot/js/ajax/ssh_validate.php @@ -1,5 +1,6 @@ Date: Sat, 19 Apr 2025 12:55:52 -0400 Subject: [PATCH 11/15] phpunit is broken now --- .github/workflows/phpunit.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index b5eb296f..59c7c79d 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -12,9 +12,9 @@ jobs: with: php-version: "8.3" # php extensions also listed in tools/docker-dev/web/Dockerfile - extensions: curl,mysql,ldap,pdo,redis,cli + extensions: curl,mysql,ldap,pdo,redis tools: composer:v2 - name: Install dependencies run: composer install --prefer-dist --no-progress - name: Run PHPUnit tests - run: vendor/bin/phpunit + run: vendor/bin/phpunit --debug From b6299ddb3f8702059d2a8bb2be3351d2e41124b4 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 12:58:21 -0400 Subject: [PATCH 12/15] don't hide output --- test/unit/AjaxSshValidateTest.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/unit/AjaxSshValidateTest.php b/test/unit/AjaxSshValidateTest.php index d320ca5e..6f670a43 100644 --- a/test/unit/AjaxSshValidateTest.php +++ b/test/unit/AjaxSshValidateTest.php @@ -24,11 +24,8 @@ public function testSshValidate(bool $is_valid, string $pubkey) $_SERVER["REQUEST_METHOD"] = "POST"; $_POST["key"] = $pubkey; ob_start(); - try { - include __DIR__ . "/../../webroot/js/ajax/ssh_validate.php"; - } finally { - $output = ob_get_clean(); - } + include __DIR__ . "/../../webroot/js/ajax/ssh_validate.php"; + $output = ob_get_clean(); if ($is_valid) { $this->assertEquals("true", $output); } else { From 4fbe53e4a2bc6595acf0138eec8d535cba06f5c4 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 13:00:09 -0400 Subject: [PATCH 13/15] downgrade phpunit --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 77028b7d..006bcf7f 100644 --- a/composer.json +++ b/composer.json @@ -6,6 +6,6 @@ "hakasapl/phpopenldaper": "1.0.5" }, "require-dev": { - "phpunit/phpunit": "^12.1" + "phpunit/phpunit": "<12.1" } } From e9dd8e18346cf8bdc798802043941281a86da9f1 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sat, 19 Apr 2025 13:02:02 -0400 Subject: [PATCH 14/15] require once --- webroot/js/ajax/ssh_validate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webroot/js/ajax/ssh_validate.php b/webroot/js/ajax/ssh_validate.php index 62e0e47f..a62809cb 100644 --- a/webroot/js/ajax/ssh_validate.php +++ b/webroot/js/ajax/ssh_validate.php @@ -1,6 +1,6 @@ Date: Sat, 19 Apr 2025 13:02:44 -0400 Subject: [PATCH 15/15] debug no longer necessary --- .github/workflows/phpunit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 59c7c79d..e2807431 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -17,4 +17,4 @@ jobs: - name: Install dependencies run: composer install --prefer-dist --no-progress - name: Run PHPUnit tests - run: vendor/bin/phpunit --debug + run: vendor/bin/phpunit