Port Password Policy module to Drupal 5.x and add expiration options

To migrate this module to 5.x, I've made the following changes:

• Added .info file
• Changed form handling to 5.x Form API
• Changed menu paths to place policy settings under Site Configuration
• Altered a placeholder in history constraint file to avoid double-escaping of 'em' tags.

svn diff below.
Files attached.
It would be good to see this module integrated with the password strength indicator for D6. http://drupal.org/node/143026

Index: password_policy/password_policy.module
===================================================================
--- password_policy/password_policy.module      (revision 4264)
+++ password_policy/password_policy.module      (revision 4371)
@@ -10,7 +10,7 @@
                case "admin/modules#description":
                        $output = t("Enforces a password policy on user accounts.");
                        break;
-               case "admin/password_policy":
+               case "admin/settings/password_policy":
                        $output = '<p>' . t('The password policy module allows you to enforce a specific level of password complexity ') .
                        t('for the user passwords on the system.') . '</p>';
 
@@ -23,11 +23,11 @@
                                t("Set default policy") . '</i>' . t(" button.");
                        }
                        else {
-                               $output .= '<p>' . t('No policies are currently defined.  To add a new policy, click <a href="%url">add policy</a>.', array("%url" => url('admin/password_policy/add')));
+                               $output .= '<p>' . t('No policies are currently defined.  To add a new policy, click <a href="@url">add policy</a>.', array("@url" => url('admin/settings/password_policy/add')));
                        }
                        break;
-               case "admin/password_policy/add":
-               case "admin/password_policy/edit/" . arg(3) :
+               case "admin/settings/password_policy/add":
+               case "admin/settings/password_policy/edit/" . arg(3) :
                        $output = '<p>';
                        if (arg(2) == 'add')
                                $output .= t("Give a name and descriptive comment to your new password policy.  ");
@@ -58,52 +58,52 @@
        $items = array();
        $edit = user_access('administer password policies');
 
-       $items[] = array('path' => 'admin/password_policy',
-      'title' => t('password policy'),
+       $items[] = array('path' => 'admin/settings/password_policy',
+      'title' => t('Password Policy'),
       'description' => t('Configures policies for user account passwords.'),
       'callback' => 'password_policy_view',
       'access' => $edit);
 
-       $items[] = array('path' => 'admin/password_policy/add',
-      'title' => t('add policy'),
+       $items[] = array('path' => 'admin/settings/password_policy/add',
+      'title' => t('Add Policy'),
       'callback' => 'password_policy_form_policy',
       'access' => $edit,
       'type' => MENU_LOCAL_TASK);
 
-       $arg3 = arg(3);
-       if (!empty($arg3) && is_numeric($arg3)) {
-               $items[] = array('path' => 'admin/password_policy/edit/'. arg(3),
+       $arg4 = arg(4);
+       if (!empty($arg4) && is_numeric($arg4)) {
+               $items[] = array('path' => 'admin/settings/password_policy/edit/'. arg(4),
                  'title' => t("Edit password policy"),
                  'callback' => 'password_policy_form_policy',
-                 'callback arguments' => array('id' => arg(3)),
+                 'callback arguments' => array('id' => arg(4)),
                  'type' => MENU_CALLBACK,
                  'access' => $edit
                );
 
-               $items[] = array('path' => 'admin/password_policy/delete/'. arg(3),
+               $items[] = array('path' => 'admin/settings/password_policy/delete/'. arg(4),
                 'title' => t("Delete password policy"),
                 'callback' => 'password_policy_delete',
-                'callback arguments' => array('id' => arg(3)),
+                'callback arguments' => array('id' => arg(4)),
                 'type' => MENU_CALLBACK,
                 'access' => $edit
                );
        }
 
        // we display the name of the policy when viewing
-       $arg2 = arg(2);
-       if (!empty($arg2) && is_numeric($arg2)) {
-               $policy = password_policy_load_policy_by_id($arg2);
+       $arg3 = arg(3);
+       if (!empty($arg3) && is_numeric($arg3)) {
+               $policy = password_policy_load_policy_by_id($arg3);
 
-               $items[] = array('path' => 'admin/password_policy/'. $arg2,
+               $items[] = array('path' => 'admin/settings/password_policy/'. $arg3,
           'title' => $policy->name,
           'callback' => 'password_policy_view',
-          'callback arguments' => array('id' => $arg2),
+          'callback arguments' => array('id' => $arg3),
           'type' => MENU_CALLBACK,
           'access' => $edit
                );
        }
 
-       $items[] = array('path' => 'admin/password_policy/list',
+       $items[] = array('path' => 'admin/settings/password_policy/list',
       'title' => t('list'),
       'type' => MENU_DEFAULT_LOCAL_TASK,
       'weight' => -10);
@@ -118,8 +118,8 @@
  * default view, the user can set a new default password policy or clear the default so
  * that no policy is active and the default drupal password mechanism takes affect.
  */
-function password_policy_view_submit($form_id, $form_values) {
-       $op = !empty($_POST['op']) ? $_POST['op'] : '';
+function password_policy_view_form_submit($form_id, $form_values) {
+       $op = !empty($form_values['op']) ? $form_values['op'] : '';
        if ($op == t('Clear default')) {
                _password_policy_clear_default();
                drupal_set_message(t('No policy is active, all user passwords will be accepted (Drupal default).'));
@@ -156,9 +156,9 @@
        if ($pid) {
                $policy = password_policy_load_policy_by_id($pid);
                if (!$policy) {
-                       drupal_goto("admin/password_policy");
+                       drupal_goto("admin/settings/password_policy");
                }
-               $editURL = l(t('editing this policy'), 'admin/password_policy/edit/'. $pid);
+               $editURL = l(t('editing this policy'), 'admin/settings/password_policy/edit/'. $pid);
                $constraints = $policy->constraints;
                $desc = !$constraints ? t('This policy has no constraints set.  You can add constraints by ') . $editURL . '.'
                : t('This policy has the constraints listed below.  You can change the constraints by ') . $editURL . '.</br>' .  $policy->getValidationErrorMessage();
@@ -169,53 +169,62 @@
 
        // load the summary policies (id->name)
        $summaries = _password_policy_load_policy_summaries();
-
+  
        if ($summaries) {
-               foreach ($summaries as $summary) {
-                       $id = $summary['id'];
-                       $name = $summary['name'];
-                       $row = array();
-                       $options[$id] = '';
-                       if ($summary['enabled']) {
-                               $default_id = $id;
-                       }
-                       $form[$name]['id'] = array('#value' => $id);
-                       $form[$name]['view'] = array('#value' => l(t('view'), 'admin/password_policy/'. $id));
-                       $form[$name]['edit'] = array('#value' => l(t('edit'), 'admin/password_policy/edit/'. $id));
-                       $form[$name]['delete'] = array('#value' => l(t('delete'), 'admin/password_policy/delete/'. $id));
-               }
-               $form['default'] = array('#type' => 'radios', '#options' => $options, '#default_value' => $default_id);
-               $form['submit'] = array('#type' => 'submit', '#value' => t('Set default policy'));
-               $form['clear'] = array('#type' => 'submit', '#value' => t('Clear default'));
-               return drupal_get_form('password_policy_view', $form);
+               return drupal_get_form('password_policy_view_form', $summaries);
        }
        return "";
 }
 
+function password_policy_view_form($summaries) {
+  $form = array();
+  //$summaries = _password_policy_load_policy_summaries();
+  if ($summaries) {
+    foreach ($summaries as $summary) {
+      $id = $summary['id'];
+      $name = $summary['name'];
+      $row = array();
+      $options[$id] = '';
+      if ($summary['enabled']) {
+        $default_id = $id;
+      }
+      $form[$name]['id'] = array('#value' => $id);
+      $form[$name]['view'] = array('#value' => l(t('view'), 'admin/settings/password_policy/'. $id));
+      $form[$name]['edit'] = array('#value' => l(t('edit'), 'admin/settings/password_policy/edit/'. $id));
+      $form[$name]['delete'] = array('#value' => l(t('delete'), 'admin/settings/password_policy/delete/'. $id));
+      //$options[$id] = $name;
+      //. l(t('view'), 'admin/password_policy/'. $id) . l(t('edit'), 'admin/password_policy/edit/'. $id) . l(t('delete'), 'admin/password_policy/delete/'. $id);
+    }
+    $form['default'] = array('#type' => 'radios', '#options' => $options, '#default_value' => $default_id);
+    $form['submit'] = array('#type' => 'submit', '#value' => t('Set default policy'));
+    $form['clear'] = array('#type' => 'submit', '#value' => t('Clear default'));
+    
+    return $form;
+  }
+}
 
-
 /**
  * Custom theme for rendering a checkbox list of defined policies.  With Drupal's form
  * API, it can be tricky to get a layout of the form elements which is different from
  * the default.  This layout is based on a similar layout found in the "input formats"
  * module.
  */
-function theme_password_policy_view($form) {
+function theme_password_policy_view_form($form) {
        foreach ($form as $name => $element) {
                if (!empty($element['edit']) && is_array($element['edit'])) {
                        $rows[] = array(
-                       form_render($form['default'][$element['id']['#value']]),
+                       drupal_render($form['default'][$element['id']['#value']]),
                        check_plain($name),
-                       form_render($form[$name]['view']),
-                       form_render($form[$name]['edit']),
-                       form_render($form[$name]['delete'])
+                       drupal_render($form[$name]['view']),
+                       drupal_render($form[$name]['edit']),
+                       drupal_render($form[$name]['delete'])
                        );
                        unset($form[$name]);
                }
        }
        $header = array(t('Default'), t('Name'), array('data' => t('Operations'), 'colspan' => 3));
        $output = theme('table', $header, $rows);
-       $output .= form_render($form);
+       $output .= drupal_render($form);
 
        return $output;
 }
@@ -225,7 +234,7 @@
  * in password_policy_delete_submit().
  */
 function password_policy_delete() {
-       $pid = arg(3);
+       $pid = arg(4);
        if (!$pid) {
                drupal_not_found();
        }
@@ -233,31 +242,35 @@
        $policy = password_policy_load_policy_by_id($pid);
 
        if ($policy) {
-               $form['pid'] = array('#type' => 'hidden', '#value' => $pid);
-               $form['name'] = array('#type' => 'hidden', '#value' => $policy->name);
-
-               $description = count($policy->constraints)
-               ? t('This policy has the following constraints:') . '<br/>' . $policy->getValidationErrorMessage()
-               : t('There are no constraints specified for this policy.');
-
-               return confirm_form(
-       'password_policy_delete', 
-               $form,
-               t('Are you sure you want to delete the policy \'%name\'?', array('%name' => $policy->name)),
-        'admin/password_policy', 
-               $description,
-               t('Delete'),
-               t('Cancel'));
+               return drupal_get_form('password_policy_delete_confirm', $pid, $policy);
        }
        else {
                drupal_not_found();
        }
 }
 
+function password_policy_delete_confirm($pid, $policy) {
+  $form = array();
+  $form['pid'] = array('#type' => 'hidden', '#value' => $pid);
+  $form['name'] = array('#type' => 'hidden', '#value' => $policy->name);
+
+  $description = count($policy->constraints)
+    ? t('This policy has the following constraints:') . '<br/>' . $policy->getValidationErrorMessage()
+    : t('There are no constraints specified for this policy.');
+
+  return confirm_form(
+    $form,
+    t('Are you sure you want to delete the policy \'%name\'?', array('%name' => $policy->name)),
+        'admin/settings/password_policy', 
+    $description,
+    t('Delete')
+  );
+}
+
 /**
  * Submit hook for the delete policy operation.
  */
-function password_policy_delete_submit($form_id, $form_values) {
+function password_policy_delete_confirm_submit($form_id, $form_values) {
        $pid = $form_values['pid'];
        $policy = password_policy_load_policy_by_id($pid);
 
@@ -268,7 +281,7 @@
                watchdog('password_policy', t('Policy \'%name\' was deleted.', array('%name' => $policy->name)), WATCHDOG_NOTICE);
        }
 
-       return 'admin/password_policy';
+       return 'admin/settings/password_policy';
 }
 
 /**
@@ -314,7 +327,13 @@
  */
 function password_policy_form_policy($pid = NULL) {
 
-       if ($pid) {
+       return drupal_get_form('password_policy_form_policy_form', $pid);
+}
+
+function password_policy_form_policy_form($pid) {
+  $form = array();
+
+  if ($pid) {
                $policy = password_policy_load_policy_by_id($pid);
        }
 
@@ -363,16 +382,15 @@
        if ($policy) {
                $form['pid'] = array('#type' => 'hidden', '#value' => $pid);
        }
-
-       return drupal_get_form('password_policy_form_policy', $form);
+
+  return $form;
 }
 
 
 /**
  * Form submission hook for new or edited password policies.
  */
-function password_policy_form_policy_submit($form_id, $form_values) {
-
+function password_policy_form_policy_form_submit($form_id, $form_values) {
        // create the policy
        _password_policy_load_constraint_definitions();
 
@@ -400,14 +418,14 @@
        if ($form_values['pid']) {
                password_policy_update_policy($form_values['pid'], $policy);
                drupal_set_message(t('Policy \'%name\' has been updated.', array('%name' => $policy->name)));
-               watchdog('password_policy', t('Policy \'%name\' updated.', array('%name' => $policy->name)), WATCHDOG_NOTICE, l(t('view'), 'admin/password_policy/' . $form_values['pid']));
+               watchdog('password_policy', t('Policy \'%name\' updated.', array('%name' => $policy->name)), WATCHDOG_NOTICE, l(t('view'), 'admin/settings/password_policy/' . $form_values['pid']));
        }
        else {
                password_policy_save_policy($policy);
-               watchdog('password_policy', t('New policy \'%name\' added.', array('%name' => $policy->name)), WATCHDOG_NOTICE, l(t('view'), 'admin/password_policy'));
+               watchdog('password_policy', t('New policy \'%name\' added.', array('%name' => $policy->name)), WATCHDOG_NOTICE, l(t('view'), 'admin/settings/password_policy'));
        }
 
-       return "admin/password_policy";
+       return "admin/settings/password_policy";
 }
 
 /**
@@ -565,5 +583,3 @@
        $tests = file_scan_directory($dir, '\.test$');
        return array_keys($tests);
 }
-
-?>
Index: password_policy/constraints/constraint_history.php
===================================================================
--- password_policy/constraints/constraint_history.php  (revision 4264)
+++ password_policy/constraints/constraint_history.php  (revision 4371)
@@ -47,7 +47,7 @@
        function getValidationErrorMessage() {
                return t("Password must not match %stmt.", 
                array('%windowSize' => $this->minimumConstraintValue, 
-                         '%stmt' => format_plural($this->minimumConstraintValue, t('the last password used'), t('any of the previous %windowSize passwords', array('%windowSize' => $this->minimumConstraintValue))),
+                         '%stmt' => format_plural($this->minimumConstraintValue, t('the last password used'), t('any of the previous @windowSize passwords', array('@windowSize' => $this->minimumConstraintValue))),
                          '%password' => format_plural($this->minimumConstraintValue, t('password'), t('passwords'))));
        }
 
Index: password_policy/password_policy.info
===================================================================
--- password_policy/password_policy.info        (revision 0)
+++ password_policy/password_policy.info        (revision 4371)
@@ -0,0 +1,4 @@
+; $Id$
+name = "Password Policy"
+description = "Set policy to ensure good password security."
+version = "$Name$"
\ No newline at end of file

hey i dould not see the svn difference or the file attached for porting timeline module to Drupal 5.x

Here is a patch against HEAD that uses jonathan_hunt's code. I haven't tested it yet.

Right, the patch.

This is still a bit buggy. Once you create a profile, the list of profiles doesn't display correctly.

Here's an updated patch which fixes the issues I've found so far with the 5x port and adds password expiration from http://drupal.org/node/150079

Hi All

seanr, I`ve patch the password_policy HEAD version with your patch.
After install module I try configure password policy module. And I ahve next errors
in the tab "list"
user warning: Unknown column 'created' in 'field list' query: SELECT id, name, enabled, description, created FROM password_policy ORDER BY name in *** \www\includes\database.mysql.inc on line 172.
and error in tab "expired accounts" :

user warning: Table 'drupal5.password_policy_expiration' doesn't exist query: SELECT COUNT(*) FROM password_policy_expiration p INNER JOIN users u ON p.uid = u.uid WHERE p.blocked > 0 in *** \www\includes\database.mysql.inc on line 172.
user warning: Table 'drupal5.password_policy_expiration' doesn't exist query: SELECT p.*, u.name FROM password_policy_expiration p INNER JOIN users u ON p.uid = u.uid WHERE p.blocked > 0 ORDER BY blocked DESC LIMIT 0, 20 in *** \www\includes\database.mysql.inc on line 172.

It's unfortunate that 5.x got skipped, but I need to implement password expiration and it'll be a couple months before I'm on 6.x.

I'm attaching improvements to #7 which address #8, as well as coder module notices (which found some bugs like user_mail()). It also moves what was in #7 pointing to admin/settings/password_policy to admin/user/password_policy so the former hook_settings() can be in the admin/settings/password_policy slot.

Since I'm probably not the only one stuck on 5.x for a while, I propose 5.x still get its own branch. That probably mean making a momentary 5.x commit to HEAD, branching, and then re-committing the 6.x code to refresh its newness.

I have coded, installed, run simpletest, and gone through the forms, but haven't actually tested passwords and logging in and out or compared it to what has been done in 6.x, so I'm marking this "needs work" symbolically, but I would like it to be committed anyway. The diff is against a 1/4/2007 20:00 checkout of HEAD. I think it's probably at least worthy of getting its own commit so this 5.x upgrade patch doesn't get any bigger, and address http://drupal.org/node/150079, http://drupal.org/node/266429, and work out the details of a 5.x to 6.x upgrade later.

Thanks. I will install it, test several usecases and if everything goes well, will create a branch for it.

I have created a D5 release based off your patch.

@miglius : When are you planning to release the 5.x version ?

A dev release was released but not explicitly listed to be shown. It is now.