Password Strengthening patch - submitted

visolveemr wrote on Thursday, February 04, 2010:

Hi

Password strengthening patch is submitted at https://sourceforge.net/tracker/?func=detail&aid=2945875&group_id=60081&atid=1245239

Different features are:
1. Secure password policy (Controlled by the variable ‘secure_password’ in globals.php. Disabled by default)
2. Password expiration after a certain period (Controlled by the variable ‘password_expiration_days’ in globals.php. Disabled by default)
3. Disallowing the last used 3 passwords(Controlled by the variable ‘password_history’ in globals.php. Disabled by default)

Wiki Reference: http://openmedsoftware.org/wiki/1._Password_policy_enhancements

Do share your views.

Thanks
ViCarePlus Team

bradymiller wrote on Friday, February 05, 2010:

hey,
Code looks good and test well. Let’s see if Rod has some input. If he agrees, then you can commit this patch (ensure fix the TYPO in sql/3_2_0-to-3_3_0_upgrade.sql either before or right after the commit). Then other stuff can be worked on directly through SF cvs for this project.

Couple issues:
interface/main/main_screen.php:
-formData on _POST (shouldn’t this be enclosed by the formData() function from formdata.inc.php before it’s used in the sql query??))

interface/usergroup/user_admin.php
-patch line 279: place ‘healthCare@09’ outside the xl function (no reason to translate this)
-Is it ok to place all the password hashes within this page (can see them all by simply looking at source)? I don’t know, which is why I’m asking.

interface/usergroup/usergroup_admin_add.php
-patch line 400: place ‘healthCare@09’ outside the xl function (no reason to translate this)

interface/usergroup/user_info.php
-patch line 538: place ‘healthCare@09’ outside the xl function (no reason to translate this)

library/auth.inc
-for your two sql statements does POST’authuser’ stuff need to be run through formData() function ???

sql/3_2_0-to-3_3_0_upgrade.sql
-LIKELY TYPO: with pwd_expiration_date, should be a DATE, not a VARCHARS

-brady

sunsetsystems wrote on Saturday, February 06, 2010:

It looks pretty good, but a few things need some cleanup.  It’s OK to do that after committing:

This fragment demonstrates some problems that I see in various places (I have changed square brackes to braces here due to screwed-up SF formatting):

// Fetching the password expiration date
if($GLOBALS{'password_expiration_days'} != 0){
$is_expired = false;
$q=$_POST{"authUser"};
$result = sqlStatement("select pwd_expiration_date from users where username = '".$q."'");
$current_date = date("Y-m-d");
$pwd_expires_date = $current_date;
if($row = sqlFetchArray($result)) {
 $pwd_expires_date = $row{'pwd_expiration_date'};
}

… in the use of a raw POST variable in SQL, absence of indenting, and missing white space around some operators.

In interface/main/pwd_expires_alert.php and other places there are a bunch of hard-coded colors.  They should each be defined in one place, probably the stylesheet.

I also see several cases of hard-coded widths.  Again, probably needs styling.

Rod
www.sunsetsystems.com