System: fix password validation check, improve the password policy an…

…d add a strength checker
GibbonEdu · Nov 3, 2024 · e24def5 · e24def5
1 parent 7efd0f0
commit e24def5
Show file tree

Hide file tree

Showing 16 changed files with 122 additions and 144 deletions.
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
@@ -39,6 +39,7 @@ v28.0.00
         System: updated Core modules to have Gibbon Foundation as author
         System: updates the timetable navigation to use AJAX reloading rather than page refresh
         System: moved the login form to the bottom of the page when viewing subpages and not logged in
+        System: added a password strength checker and improved password policy display
         Attendance: added an index to the attendance log to help speed up attendance pages
         Attendance: added an indicator to Student Not Present/Onsite when attendance logs conflict
         Activities: updated Activity Attendance to always count participants for dates in the past

diff --git a/installer/install.php b/installer/install.php
@@ -210,7 +210,7 @@
     'gibbonThemeName' => 'Default',
     'absolutePath'    => realpath(dirname(__DIR__)),
     'absoluteURL'     => InstallController::guessAbsoluteUrl(),
-    'sidebar'         => true,
+    'sidebar'         => false,
     'contentClass'    => 'max-w-4xl mx-auto px-12 pt-6 pb-12',
     'step'            => $step,
 ]);

diff --git a/modules/User Admin/user_manage_add.php b/modules/User Admin/user_manage_add.php
@@ -146,15 +146,10 @@
             ->required()
             ->addGenerateUsernameButton($form);
 
-    /** @var PasswordPolicy */
-    $policies = $container->get(PasswordPolicy::class);
-    if (($policiesHTML = $policies->describeHTML()) !== '') {
-        $form->addRow()->addAlert($policiesHTML, 'warning');
-    }
     $row = $form->addRow();
         $row->addLabel('passwordNew', __('Password'));
         $row->addPassword('passwordNew')
-            ->addPasswordPolicy($pdo)
+            ->addPasswordPolicy($container->get(PasswordPolicy::class))
             ->addGeneratePasswordButton($form)
             ->required()
             ->maxLength(30);

diff --git a/modules/User Admin/user_manage_password.php b/modules/User Admin/user_manage_password.php
@@ -68,14 +68,6 @@
                 $page->navigator->addSearchResultsAction(Url::fromModuleRoute('User Admin', 'user_manage.php')->withQueryParam('search', $search));
             }
 
-            /** @var PasswordPolicy */
-            $policies = $container->get(PasswordPolicy::class);
-            if (($policiesHTML = $policies->describeHTML()) !== '') {
-                echo "<div class='warning'>";
-                echo $policiesHTML;
-                echo '</div>';
-            }
-
             $form = Form::create('resetUserPassword', $session->get('absoluteURL').'/modules/'.$session->get('module').'/user_manage_passwordProcess.php?gibbonPersonID='.$gibbonPersonID.'&search='.$search);
 
             $form->addHiddenValue('address', $session->get('address'));
@@ -87,7 +79,7 @@
             $row = $form->addRow();
                 $row->addLabel('passwordNew', __('Password'));
                 $row->addPassword('passwordNew')
-                    ->addPasswordPolicy($pdo)
+                    ->addPasswordPolicy($container->get(PasswordPolicy::class))
                     ->addGeneratePasswordButton($form)
                     ->required()
                     ->maxLength(30);

diff --git a/passwordReset.php b/passwordReset.php
@@ -93,15 +93,15 @@
         $form->addRow()->addHeading('Reset Password', __('Reset Password'));
 
         /** @var PasswordPolicy */
-        $policies = $container->get(PasswordPolicy::class);
-        if (($policiesHTML = $policies->describeHTML()) !== '') {
+        $passwordPolicy = $container->get(PasswordPolicy::class);
+        if (($policiesHTML = $passwordPolicy->describeHTML()) !== '') {
             $form->addRow()->addAlert($policiesHTML, 'warning');
         }
 
         $row = $form->addRow();
             $row->addLabel('passwordNew', __('New Password'));
             $row->addPassword('passwordNew')
-                ->addPasswordPolicy($pdo)
+                ->addPasswordPolicy($passwordPolicy)
                 ->addGeneratePasswordButton($form)
                 ->required()
                 ->maxLength(30);

diff --git a/preferences.php b/preferences.php
@@ -71,12 +71,6 @@
 
     $form->addRow()->addHeading('Reset Password', __('Reset Password'));
 
-    /** @var PasswordPolicy */
-    $policies = $container->get(PasswordPolicy::class);
-    if (($policiesHTML = $policies->describeHTML()) !== '') {
-        $form->addRow()->addAlert($policiesHTML, 'warning');
-    }
-
     $row = $form->addRow();
         $row->addLabel('password', __('Current Password'));
         $row->addPassword('password')
@@ -86,7 +80,7 @@
     $row = $form->addRow();
         $row->addLabel('passwordNew', __('New Password'));
         $row->addPassword('passwordNew')
-            ->addPasswordPolicy($pdo)
+            ->addPasswordPolicy($container->get(PasswordPolicy::class))
             ->addGeneratePasswordButton($form)
             ->required()
             ->maxLength(30);

diff --git a/resources/assets/css/core.min.css b/resources/assets/css/core.min.css
diff --git a/resources/assets/js/core.js b/resources/assets/js/core.js
@@ -253,6 +253,8 @@ htmx.onLoad(function (content) {
         $('input[name="' + $(this).data("confirm") + '"]')
             .val(text)
             .blur();
+        document.getElementById($(this).data("source")).dispatchEvent(new Event('blur'));
+
         prompt($(this).data("alert"), text);
     });
 

diff --git a/resources/assets/js/core.min.js b/resources/assets/js/core.min.js
diff --git a/resources/templates/index.twig.html b/resources/templates/index.twig.html
@@ -27,7 +27,7 @@
             {% block header %}
                 <div id="header" class="relative flex justify-between items-center">
 
-                    <a id="header-logo" hx-boost="true" hx-target="#content-wrap" hx-select="#content-wrap" hx-swap="outerHTML show:no-scroll swap:0s" class="block my-4 max-w-xs sm:max-w-full leading-none" href="{{ absoluteURL }}">
+                    <a id="header-logo" hx-boost="true" hx-target="#wrapOuter" hx-select="#wrapOuter" hx-swap="outerHTML show:no-scroll swap:0s" class="block my-4 max-w-xs sm:max-w-full leading-none" href="{{ absoluteURL }}">
                         <img class="block max-w-full {{ isLoggedIn ? 'h-12' : 'h-20 mt-4 mb-4' }}" alt="{{ organisationName }} Logo" src="{{ absoluteURL }}/{{ organisationLogo|default("themes/Default/img/logo.png")|trim('/', 'left') }}" style="max-height:100px;" />
                     </a>
 

diff --git a/resources/templates/navigation.twig.html b/resources/templates/navigation.twig.html
@@ -14,6 +14,7 @@
 
 {% set sidebarClass = 'shadow bg-white rounded px-6 mb-6' %}
 
+{% if sidebar or menuModule %}
 <div class="mx-4 sm:mx-0 relative sm:static" x-data="{moduleMenu: false}" @keydown.escape.window="moduleMenu = false" @click.outside="moduleMenu = false" x-init="moduleMenu = false">
 
     {% if menuModule %}
@@ -88,3 +89,4 @@ <h5 class="m-0 mb-1 text-sm px-px pt-2 pb-1 text-{{ themeColour }}-600 border-b-
     {% endif %}
 </div>
 
+{% endif %}
diff --git a/src/Data/PasswordPolicy.php b/src/Data/PasswordPolicy.php
@@ -75,6 +75,36 @@ public static function createNilPolicy()
         return $instance;
     }
 
+    /**
+     * Get the regex pattern to use in HTML input fields.
+     *
+     * @return string
+     */
+    public function getValidationPattern() : string
+    {
+        $patterns = [];
+
+        if ($this->alpha == 'Y') {
+            $patterns[] = '(?=.*[a-z])(?=.*[A-Z])';
+        }
+
+        if ($this->numeric == 'Y') {
+            $patterns[] = "(?=.*[0-9])";
+        }
+
+        if ($this->punctuation == 'Y') {
+            $patterns[] = "(?=.*[^a-zA-Z0-9])";
+        }
+
+        if (!empty($this->minLength) && is_numeric($this->minLength)) {
+            $patterns[] = '.{'.$this->minLength.',}';
+        }
+
+        return !empty($patterns)
+            ? implode('', $patterns)
+            : '';
+    }
+
     /**
      * Check if a password is valid.
      *
@@ -189,16 +219,17 @@ public function describe(): array
             throw new \Exception(__('Internal Error: Password policy setting incorrect.'));
         }
         if ($this->alpha) {
-            $rules[] = __('Contain at least one lowercase letter, and one uppercase letter.');
+            $rules['(?=.*[a-z])(?=.*[A-Z])'] = __('Contain at least one lowercase letter, and one uppercase letter.');
         }
         if ($this->numeric) {
-            $rules[] = __('Contain at least one number.');
+            $rules['[0-9]+'] = __('Contain at least one number.');
         }
         if ($this->punctuation) {
-            $rules[] = __('Contain at least one non-alphanumeric character (e.g. a punctuation mark or space).');
+            $rules['[^a-zA-Z0-9]'] = __('Contain at least one non-alphanumeric character (e.g. a punctuation mark or space).');
         }
         if ($this->minLength > 0) {
-            $rules[] = sprintf(__('Must be at least %1$s characters in length.'), $this->minLength);
+            $pattern = '.{'.$this->minLength.',}';
+            $rules[$pattern] = sprintf(__('Must be at least %1$s characters in length.'), $this->minLength);
         }
         return $rules;
     }
@@ -222,7 +253,7 @@ public function describeHTML(): string
         }
 
         $output = __('The password policy stipulates that passwords must:').'<br/>';
-        $output .= '<ul>';
+        $output .= '<ul class="text-xs ml-6">';
         $output .= implode('', array_map(function ($description) {
             return '<li>' . $description . '</li>';
         }, $descriptions));

diff --git a/src/Forms/Input/Password.php b/src/Forms/Input/Password.php
@@ -21,9 +21,8 @@
 
 namespace Gibbon\Forms\Input;
 
-use Gibbon\Contracts\Database\Connection;
-use Gibbon\Domain\System\SettingGateway;
 use Gibbon\View\Component;
+use Gibbon\Data\PasswordPolicy;
 
 /**
  * Password
@@ -33,39 +32,19 @@
  */
 class Password extends TextField
 {
+    protected $policy;
+
     /**
      * Attach the validation requirements for the system-wide password policy.
-     * @param Connection $pdo
+     * @param PasswordPolicy $policy
      * @return self
      */
-    public function addPasswordPolicy(Connection $pdo)
+    public function addPasswordPolicy(PasswordPolicy $policy)
     {
-        global $container;
-
-        $settingGateway = $container->get(SettingGateway::class);
+        $this->policy = $policy;
 
-        $alpha = $settingGateway->getSettingByScope('System', 'passwordPolicyAlpha');
-        $numeric = $settingGateway->getSettingByScope('System', 'passwordPolicyNumeric');
-        $punctuation = $settingGateway->getSettingByScope('System', 'passwordPolicyNonAlphaNumeric');
-        $minLength = $settingGateway->getSettingByScope('System', 'passwordPolicyMinLength');
-
-        $patterns = [];
-        if ($alpha == 'Y') {
-            $patterns[] = '(?=.*[a-z])(?=.*[A-Z])';
-        }
-        if ($numeric == 'Y') {
-            $patterns[] = "(?=.*[0-9])";
-        }
-        if ($punctuation == 'Y') {
-            $patterns[] = "(?=.*[^a-zA-Z0-9])";
-        }
-        if (!empty($minLength) && is_numeric($minLength)) {
-            $patterns[] = '.{'.$minLength.',}';
-        }
-
-        if (!empty($patterns)) {
-            $patternString = '/'.implode('', $patterns).'/';
-            $this->addValidation('Validate.Format', 'pattern: '.$patternString.', failureMessage: "'.__('Does not meet password policy.').'"');
+        if ($patternString = $policy->getValidationPattern()) {
+            $this->addValidation('Validate.Format', 'pattern: /'.$patternString.'/, failureMessage: "'.__('Does not meet password policy.').'"');
         }
 
         return $this;
@@ -109,7 +88,9 @@ public function addConfirmation($fieldName)
     protected function getElement()
     {
         return Component::render(Password::class, $this->getAttributeArray() + [
-            'group' => $this->group,
+            'group'  => $this->group,
+            'policy' => !empty($this->policy) ? $this->policy->describe() : [],
+            'policyPattern' => !empty($this->policy) ? $this->policy->getValidationPattern() : '',
         ]);
     }
 }
diff --git a/src/Forms/Input/Password.template.php b/src/Forms/Input/Password.template.php
@@ -6,23 +6,60 @@
     else $groupClass = 'rounded-md';
 ?>
 
-<div x-data="{ show: true }" class="flex-grow relative flex">
-<input :type="show ? 'password' : 'text'" <?= $attributes; ?> autocomplete="off" 
+<div x-data="{ show: true, policy: false, pw: '' }" class="flex-grow relative flex">
+    <input x-model.fill="pw" :type="show ? 'password' : 'text'" <?= $attributes; ?> autocomplete="off" 
     class="<?= $class; ?> <?= $groupClass; ?> w-full min-w-0  py-2 placeholder:text-gray-500  sm:text-sm sm:leading-6 <?= $type != 'text' ? 'input-icon' : ''; ?>
-
     <?= !empty($readonly) ? 'border-dashed text-gray-600 cursor-not-allowed :ring-0 focus:border-gray-400' : 'text-gray-900 focus:ring-1 focus:ring-inset focus:ring-blue-500'; ?>"
+    <?= !empty($policy) ? ' @focus="policy=true;pw=$el.value" @blur="policy=false;pw=$el.value" @click.away="policy=false"' : ''; ?>
     />
 
     <span class="absolute top-0.5 right-0.5">
 
-        <button type="button" @click="show = !show" :class="{'hidden': !show, 'block':show }">
+        <button type="button" @click="show = !show" :class="{'hidden': !show, 'block':show }" tabindex="-1">
         <?= icon('basic', 'eye', 'pointer-events-none size-9 p-2 rounded bg-white text-gray-500 hover:text-gray-700'); ?>
         </button>
 
-        <button type="button" @click="show = !show" :class="{'block': !show, 'hidden':show }">
+        <button type="button" @click="show = !show" :class="{'block': !show, 'hidden':show }" tabindex="-1">
         <?= icon('basic', 'eye-slash', 'pointer-events-none size-9 p-2 rounded bg-white text-gray-500 hover:text-gray-700'); ?>
         </button>
 
     </span>
 
+    <?php if (!empty($policy)) { ?>
+
+    <div x-cloak x-show="policy" x-transition x-data="{meterColor: 'bg-green-400'}" class="absolute mt-10 w-full z-50 bg-white border rounded-md p-2" x-init="$watch('pw', function(value) { meterColor = value.length >= 16 && pw.match(/<?= $policyPattern ?>/) ? 'bg-green-500' : value.length >= 12 ? 'bg-lime-500' : value.length >= 6 ? 'bg-yellow-500' : 'bg-red-600'; } )">
+
+        <?= __('Password Strength') ?><br>
+        <div class="flex gap-1 w-full my-2" >
+            <div class="h-1 flex-1 rounded"
+                x-bind:class="pw.length > 0 ? meterColor : 'bg-gray-200'"></div>
+            <div class="h-1 flex-1 rounded"
+                x-bind:class="pw.length >= 6 ? meterColor : 'bg-gray-200'"></div>
+            <div class="h-1 flex-1 rounded"
+                x-bind:class="pw.length >= 12 ? meterColor : 'bg-gray-200'"></div>
+            <div class="h-1 flex-1 rounded"
+                x-bind:class="pw.length >= 16 && pw.match(/<?= $policyPattern ?>/) ? meterColor : 'bg-gray-200'"></div>
+        </div>
+
+        <?= __('The password policy stipulates that passwords must:') ?><br>
+
+        <ul class="list-none ml-2 mb-0 text-xs">
+        <?php foreach ($policy as $pattern => $description) { ?>
+
+            <li>
+                <span x-show="pw.match(/<?= $pattern ?>/)" class="align-middle mr-0.5">
+                    <?= icon('solid', 'check', 'pointer-events-none size-3 text-green-600'); ?>
+                </span>
+                <span x-show="!pw.match(/<?= $pattern ?>/)" class="align-middle mr-0.5">
+                    <?= icon('solid', 'help', 'pointer-events-none size-3 text-gray-400'); ?>
+                </span>
+                <?= $description ?>
+            </li>
+        <?php } ?>
+        </ul>
+    </div>
+
+    <?php } ?>
 </div>
+
+