add checks to login shell

CONTRIBUTING.md

@@ -10,6 +10,11 @@ When submitting pull requests, the pull request should be made to the version yo
 
 This code base is currently using PHP version 7.4. All files are required to be linted with PSR-12 standard. This repository will automatically check PRs for linting compliance.
 
+Whenever frontend JS limits the possible values of a user input, PHP must also do the same, and tests must be written.
+There's nothing stopping a user from making custom HTTP POST requests.
+This can be done in `webroot/panel/*.php` while parsing headers, or preferrably in `resources/lib/*.php`.
+For example, both frontend JS and in the `UnityUser` class make sure that a login shell contains only ASCII characters, and this is tested in `test/functional/LoginShellSetTest.php`.
+
 ## Development Environment
 
 ### Setting up your Environment
@@ -51,4 +56,4 @@ The following users are available for testing:
 
 ### Changes to Dev Environment
 
-Should the default schema of the web portal change, the `ldap/bootstrap.ldif` and `sql/bootstrap.sql` must be updated for the LDAP server and the MySQL server, respectively.
+Should the default schema of the web portal change, the `ldap/bootstrap.ldif` and `sql/bootstrap.sql` must be updated for the LDAP server and the MySQL server, respectively.

resources/lib/UnityUser.php

@@ -446,8 +446,16 @@ public function getSSHKeys($ignorecache = false)
      */
     public function setLoginShell($shell, $operator = null, $send_mail = true)
     {
-        // FIXME throw error if shell is not ascii
         // ldap schema syntax is "IA5 String (1.3.6.1.4.1.1466.115.121.1.26)"
+        if (!mb_check_encoding($shell, 'ASCII')) {
+            throw new Exception("non ascii characters are not allowed in a login shell!");
+        }
+        if ($shell != trim($shell)) {
+            throw new Exception("leading/trailing whitespace is not allowed in a login shell!");
+        }
+        if (empty($shell)) {
+            throw new Exception("login shell must not be empty!");
+        }
         $ldapUser = $this->getLDAPUser();
         if ($ldapUser->exists()) {
             $ldapUser->setAttribute("loginshell", $shell);

test/functional/LoginShellSetTest.php

@@ -28,15 +28,22 @@ public static function getShells()
         // phpcs:enable
     }
 
+    private function isShellValid(string $shell)
+    {
+        return (
+            (mb_check_encoding($shell, 'ASCII')) &&
+            ($shell == trim($shell)) &&
+            (!empty($shell))
+        );
+    }
+
     #[DataProvider("getShells")]
     public function testSetLoginShellCustom(string $shell): void
     {
         global $USER;
-        // FIXME add check to avoid warning from ldap_modify
-        if (!mb_check_encoding($shell, 'ASCII')) {
-            $this->expectException("Exception");
+        if (!$this->isShellValid($shell)) {
+            $this->expectException(Exception::class);
         }
-        // FIXME shell is not validated
         post(
             __DIR__ . "/../../webroot/panel/account.php",
             ["form_type" => "loginshell", "shellSelect" => "Custom", "shell" => $shell]
@@ -48,11 +55,9 @@ public function testSetLoginShellCustom(string $shell): void
     public function testSetLoginShellSelect(string $shell): void
     {
         global $USER;
-        // FIXME add check to avoid warning from ldap_modify
-        if (!mb_check_encoding($shell, 'ASCII')) {
+        if (!$this->isShellValid($shell)) {
             $this->expectException("Exception");
         }
-        // FIXME shell is not validated
         post(
             __DIR__ . "/../../webroot/panel/account.php",
             ["form_type" => "loginshell", "shellSelect" => $shell]

webroot/panel/account.php

@@ -225,6 +225,7 @@
 ?>
 <br>
 <input id='submitLoginShell' type='submit' value='Set Login Shell' />
+<label id='labelSubmitLoginShell'> <!-- value set by JS --> </label>
 </form>
 <hr>
 
@@ -257,7 +258,6 @@
 
 <hr>
 
-
 <script>
     const sitePrefix = '<?php echo $CONFIG["site"]["prefix"]; ?>';
     const ldapLoginShell = '<?php echo $USER->getLoginShell(); ?>';
@@ -289,6 +289,62 @@ function showOrHideCustomLoginBox() {
     $("#loginSelector").change(showOrHideCustomLoginBox);
     showOrHideCustomLoginBox();
 
+    function getNewLoginShell() {
+        var loginSelectorVal = $("#loginSelector").val();
+        if (loginSelectorVal != "Custom") {
+            return loginSelectorVal;
+        }
+        return $("#customLoginBox").val();
+    }
+
+    function isLoginShellValid(x) {
+        if (/^\s|\s$/.test(x)) {
+            return [false, "must not have leading or trailing whitespace"];
+        }
+        if (x.length === 0) {
+            return [false, "must not be empty"];
+        }
+        if (!(/^[\x00-\x7F]*$/.test(x))) {
+            return [false, "must only contain ASCII characters"];
+        }
+        return [true, ""];
+    }
+
+    function enableOrDisableCustomLoginBoxHighlight() {
+        if (
+            ($("#customLoginSelectorOption").prop("selected") == true) &&
+            !isLoginShellValid($("#customLoginBox").val())
+        ) {
+            $("#customLoginBox").css("box-shadow", "0 0 0 0.3rem rgba(220, 53, 69, 0.25)");
+        } else {
+            $("#customLoginBox").css("box-shadow", "none");
+        }
+    }
+    $("#customLoginBox").on("input", enableOrDisableCustomLoginBoxHighlight);
+    $("#loginSelector").change(enableOrDisableCustomLoginBoxHighlight);
+    enableOrDisableCustomLoginBoxHighlight();
+
+    function enableOrDisableSubmitLoginShell() {
+        var newLoginShell = getNewLoginShell();
+        isValidArr = isLoginShellValid(newLoginShell);
+        isValid = isValidArr[0];
+        isValidReason = isValidArr[1];
+        if (!isValid) {
+            $("#submitLoginShell").prop("disabled", true);
+            $("#labelSubmitLoginShell").text(`(invalid login shell: ${isValidReason})`);
+            return;
+        }
+        if (newLoginShell == ldapLoginShell) {
+            $("#submitLoginShell").prop("disabled", true);
+            $("#labelSubmitLoginShell").text("(no change)");
+            return;
+        }
+        $("#submitLoginShell").prop("disabled", false);
+        $("#labelSubmitLoginShell").text("");
+    }
+    $("#customLoginBox").on("input", enableOrDisableSubmitLoginShell);
+    $("#loginSelector").change(enableOrDisableSubmitLoginShell);
+    enableOrDisableSubmitLoginShell()
 </script>
 
 <style>