improve ssh key validation (#163)

Author: simonLeary42

Diff for: .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
+        tools: composer:v2
+    - name: Install dependencies
+      run: composer install --prefer-dist --no-progress
+    - name: Run PHPUnit tests
+      run: vendor/bin/phpunit

Diff for: 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"
     }
 }

Diff for: phpunit.xml

@@ -1,9 +1,9 @@
+  <!-- restrictWarnings="true" -->
 <phpunit
   bootstrap="test/unit/bootstrap.php"
   failOnWarning="true"
   failOnDeprecation="true"
   failOnNotice="true"
-  restrictWarnings="true"
 >
   <testsuites>
     <testsuite name="unit">

Diff for: resources/lib/UnitySite.php

@@ -52,6 +52,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 for: test/unit/AjaxSshValidateTest.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace UnityWebPortal\lib;
+
+use PHPUnit\Framework\TestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+
+class AjaxSshValidateTest extends TestCase
+{
+    public static function providerTestSshValidate()
+    {
+        // sanity check only, see UnitySiteTest for more comprehensive test cases
+        return [
+            [false, "foobar"],
+            // phpcs:disable
+            [true, "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIB+XqO25MUB9x/pS04I3JQ7rMGboWyGXh0GUzkOrTi7a"],
+            // phpcs:enable
+        ];
+    }
+
+    #[DataProvider("providerTestSshValidate")]
+    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();
+        if ($is_valid) {
+            $this->assertEquals("true", $output);
+        } else {
+            $this->assertEquals("false", $output);
+        }
+    }
+}

Diff for: 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"]
+CMD ["apache2ctl", "-D", "FOREGROUND"]

Diff for: webroot/js/ajax/ssh_validate.php

@@ -1,12 +1,6 @@
 <?php
 
-require "../../../resources/autoload.php";
+require_once __DIR__ . "/../../../resources/lib/UnitySite.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";