Remove custom shell (#203)

* add check for special characters in login shell

* display invalid reason in <p> like the rest of the page

* remove comment

* update unityuser to do same checks on login shell as frontend does

* update CONTRIBUTING.md

* add check for empty shell

* update tests

* update CONTRIBUTING.md

* remove custom login shell

* remove note from contrib

* update test

Author: simonLeary42

CONTRIBUTING.md

@@ -51,4 +51,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.

defaults/config.ini.default

@@ -75,6 +75,7 @@ title[] = "Test Medium Footer"
 [loginshell]  ; Login shells that show up as options in the account settings page
 shell[] = "/bin/bash"
 shell[] = "/bin/zsh"
+shell[] = "/bin/tcsh"
 
 [menuitems]  ; menu items, add a label and link for each
 labels[] = "Global Menuitem 1"

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

@@ -69,11 +69,7 @@
             $USER->setSSHKeys($keys, $OPERATOR);  // Update user keys
             break;
         case "loginshell":
-            if ($_POST["shellSelect"] == "Custom") {
-                $USER->setLoginShell($_POST["shell"], $OPERATOR);
-            } else {
-                $USER->setLoginShell($_POST["shellSelect"], $OPERATOR);
-            }
+            $USER->setLoginShell($_POST["shellSelect"], $OPERATOR);
             break;
         case "pi_request":
             if (!$USER->isPI()) {
@@ -210,21 +206,11 @@
 foreach ($CONFIG["loginshell"]["shell"] as $shell) {
     echo "<option>$shell</option>";
 }
-echo "<option id='customLoginSelectorOption'>Custom</option>";
 ?>
 </select>
-<?php
-echo "
-    <input
-        id='customLoginBox'
-        type='text'
-        placeholder='Enter login shell path (ie. /bin/bash)'
-        name='shell'
-    />
-";
-?>
 <br>
 <input id='submitLoginShell' type='submit' value='Set Login Shell' />
+<label id='labelSubmitLoginShell'> <!-- value set by JS --> </label>
 </form>
 <hr>
 
@@ -257,7 +243,6 @@
 
 <hr>
 
-
 <script>
     const sitePrefix = '<?php echo $CONFIG["site"]["prefix"]; ?>';
     const ldapLoginShell = '<?php echo $USER->getLoginShell(); ?>';
@@ -266,29 +251,21 @@
         openModal("Add New Key", `${sitePrefix}/panel/modal/new_key.php`);
     });
 
-    var defaultShellSelected = false;
     $("#loginSelector option").each(function(i, e) {
         if ($(this).val() == ldapLoginShell) {
             $(this).prop("selected", true);
-            defaultShellSelected = true;
         }
     });
-    if (!defaultShellSelected) {
-        $("#customLoginBox").val(ldapLoginShell);
-        $("#customLoginSelectorOption").prop("selected", true);
-    }
 
-    function showOrHideCustomLoginBox() {
-        var customBox = $("#customLoginBox");
-        if($("#loginSelector").val() == "Custom") {
-            customBox.show();
+    function enableOrDisableSubmitLoginShell() {
+        if ($("#loginSelector").val() == ldapLoginShell) {
+            $("#submitLoginShell").prop("disabled", true);
         } else {
-            customBox.hide();
+            $("#submitLoginShell").prop("disabled", false);
         }
     }
-    $("#loginSelector").change(showOrHideCustomLoginBox);
-    showOrHideCustomLoginBox();
-
+    $("#loginSelector").change(enableOrDisableSubmitLoginShell);
+    enableOrDisableSubmitLoginShell()
 </script>
 
 <style>