basic password hashing, still need to rework recovery

diff --git a/schema/bse.sql b/schema/bse.sql
index 5dcdebf005305bda70af2e3b1c5223b3b8f2ae99..a8fc4740e30f25a7cefcf1922338941a61fc77b0 100644 (file)
--- a/schema/bse.sql
+++ b/schema/bse.sql
@@ -549,7 +549,7 @@ create table site_users (
   id integer not null auto_increment,
 
   userId varchar(40) not null,
-  password varchar(40) not null,
+  password varchar(255) not null,
   email varchar(255) not null,
 
   keepAddress integer not null default 1,
@@ -637,6 +637,8 @@ create table site_users (
   customInt1 integer,
   customInt2 integer,
 
+  password_type varchar(20) not null default 'plain',
+
   primary key (id),
   unique (userId),
   index (affiliate_name)

diff --git a/site/cgi-bin/modules/BSE/AdminSiteUsers.pm b/site/cgi-bin/modules/BSE/AdminSiteUsers.pm
index dcd21f413cb22993ce5251790fb778c61ec23d2b..8b209e6b02e7125cc78c3b1a2e0d93497dd9c147 100644 (file)
--- a/site/cgi-bin/modules/BSE/AdminSiteUsers.pm
+++ b/site/cgi-bin/modules/BSE/AdminSiteUsers.pm
@@ -13,7 +13,7 @@ use constant SITEUSER_GROUP_SECT => 'BSE Siteuser groups validation';
 use BSE::Template;
 use DevHelp::Date qw(dh_parse_date_sql dh_parse_time_sql);
 
-our $VERSION = "1.003";
+our $VERSION = "1.004";
 
 my %actions =
   (
@@ -443,7 +443,9 @@ sub req_save {
     $user->{userId} = $email if $nopassword;
     ++$newemail;
   }
-  $user->{password} = $newpass if !$nopassword && $newpass;
+  if (!$nopassword && $newpass) {
+    $user->changepw($newpass, $req->user || "U");
+  }
   
   $user->{affiliate_name} = $aff_name if defined $aff_name;
   
@@ -634,9 +636,6 @@ sub req_add {
   my %user;
   my @cols = SiteUser->columns;
   shift @cols;
-  for my $field (@cols) {
-    $user{$field} = '';
-  }
 
   my $custom = custom_class($cfg);
   my @required = $custom->siteuser_add_required($req);
@@ -752,7 +751,7 @@ sub req_add {
 
   my $user;
   eval {
-    $user = SiteUsers->add(@user{@cols});
+    $user = SiteUsers->make(%user);
   };
   if ($user) {
     my $subs = $class->save_subs($req, $user);

diff --git a/site/cgi-bin/modules/BSE/DB/Mysql.pm b/site/cgi-bin/modules/BSE/DB/Mysql.pm
index 6a4db65f22c5260f2d84d241cdb408cc760ec08a..6b5f174457f09ddd62f6f24263b8c8c68068fe38 100644 (file)
--- a/site/cgi-bin/modules/BSE/DB/Mysql.pm
+++ b/site/cgi-bin/modules/BSE/DB/Mysql.pm
@@ -5,7 +5,7 @@ use vars qw/@ISA/;
 use Carp 'confess';
 @ISA = qw(BSE::DB);
 
-our $VERSION = "1.002";
+our $VERSION = "1.003";
 
 use vars qw($VERSION $MAX_CONNECTION_AGE);
 
@@ -15,8 +15,6 @@ use Carp;
 
 my $self;
 
-$VERSION = 1.01;
-
 $MAX_CONNECTION_AGE = 1200;
 
 my %statements =
@@ -194,14 +192,14 @@ where af.articleId = oi.productId and oi.orderId = ?
 order by oi.id, af.displayOrder desc
 SQL
    
-   getSiteUserByUserId =>
-   'select * from site_users where userId = ?',
-   getSiteUserByPkey =>
-   'select * from site_users where id = ?',
-   getSiteUserByAffiliate_name =>
-   'select * from site_users where affiliate_name = ?',
-   addSiteUser => 'insert site_users values(null,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
-   replaceSiteUser => 'replace site_users values(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
+   # getSiteUserByUserId =>
+   # 'select * from site_users where userId = ?',
+   # getSiteUserByPkey =>
+   # 'select * from site_users where id = ?',
+   # getSiteUserByAffiliate_name =>
+   # 'select * from site_users where affiliate_name = ?',
+   # addSiteUser => 'insert site_users values(null,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
+   # replaceSiteUser => 'replace site_users values(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
    'SiteUsers.removeSubscriptions'=>
    'delete from subscribed_users where userId = ?',
    'SiteUsers.removeSub'=>

diff --git a/site/cgi-bin/modules/BSE/TB/AdminUser.pm b/site/cgi-bin/modules/BSE/TB/AdminUser.pm
index 17d50a3fed154dbb520b7e913e99038ce0babc7b..5197aaddfaf8b15d0876c106901584f69b751aac 100644 (file)
--- a/site/cgi-bin/modules/BSE/TB/AdminUser.pm
+++ b/site/cgi-bin/modules/BSE/TB/AdminUser.pm
@@ -2,7 +2,7 @@ package BSE::TB::AdminUser;
 use strict;
 use base qw(BSE::TB::AdminBase);
 
-our $VERSION = "1.001";
+our $VERSION = "1.002";
 
 sub columns {
   return ($_[0]->SUPER::columns,
@@ -46,7 +46,7 @@ sub check_password {
   my ($self, $password, $error) = @_;
 
   require BSE::Passwords;
-  return BSE::Passwords->check_password_hash($self->password, $self->password_type, $password, \$error);
+  return BSE::Passwords->check_password_hash($self->password, $self->password_type, $password, $error);
 }
 
 sub link {

diff --git a/site/cgi-bin/modules/BSE/UserReg.pm b/site/cgi-bin/modules/BSE/UserReg.pm
index 8e3de5f016b51bd2ba31222eb69abc04e3f2996c..d54811444f71dd3409c525a4fb3abc5f2ddcef08 100644 (file)
--- a/site/cgi-bin/modules/BSE/UserReg.pm
+++ b/site/cgi-bin/modules/BSE/UserReg.pm
@@ -18,7 +18,7 @@ use BSE::Util::Iterate;
 use base 'BSE::UI::UserCommon';
 use Carp qw(confess);
 
-our $VERSION = "1.013";
+our $VERSION = "1.014";
 
 use constant MAX_UNACKED_CONF_MSGS => 3;
 use constant MIN_UNACKED_CONF_GAP => 2 * 24 * 60 * 60;
@@ -162,8 +162,14 @@ sub req_logon {
   my $password = $cgi->param("password");
   unless (keys %errors) {
     $user = SiteUsers->getBy(userId => $userid);
-    unless ($user && $user->{password} eq $password) {
-      $errors{_} = $msgs->(baduserpass=>"Invalid username or password");
+    my $error = "INVALID";
+    unless ($user && $user->check_password($password, \$error)) {
+      if ($error eq "INVALID") {
+       $errors{_} = $msgs->(baduserpass=>"Invalid username or password");
+      }
+      else {
+       $errors{_} = $msgs->(passwordload => "Error loading password module");
+      }
     }
   }
   if (!keys %errors && $user->{disabled}) {
@@ -660,7 +666,8 @@ sub req_saveopts {
     
     if (defined $newpass && length $newpass) {
       if ($oldpass) {
-       if ($oldpass ne $user->{password}) {
+       my $error;
+       if (!$user->check_password($oldpass, \$error)) {
          sleep 5; # yeah, it's ugly
          $errors{old_password} = $msgs->(optsbadold=>"You need to enter your old password to change your password")
        }
@@ -732,7 +739,9 @@ sub req_saveopts {
     $user->{userId} = $email if $nopassword;
     ++$newemail;
   }
-  $user->{password} = $newpass if !$nopassword && $newpass;
+  if (!$nopassword && $newpass) {
+    $user->changepw($newpass, $user);
+  }
 
   $user->{affiliate_name} = $aff_name if defined $aff_name;
   
@@ -990,7 +999,7 @@ sub req_register {
 
   my $user;
   eval {
-    $user = SiteUsers->add(@user{@cols});
+    $user = SiteUsers->make(%user);
   };
   if ($user) {
     my $custom = custom_class($cfg);

diff --git a/site/cgi-bin/modules/SiteUser.pm b/site/cgi-bin/modules/SiteUser.pm
index c288f6f3696d61e49b5cd16f6bc06df127ab9cbc..69c0fe2f2e86808810b8be5fdac1cc10a2d71d94 100644 (file)
--- a/site/cgi-bin/modules/SiteUser.pm
+++ b/site/cgi-bin/modules/SiteUser.pm
@@ -8,7 +8,7 @@ use Constants qw($SHOP_FROM);
 use Carp qw(confess);
 use BSE::Util::SQL qw/now_datetime now_sqldate sql_normal_date sql_add_date_days/;
 
-our $VERSION = "1.004";
+our $VERSION = "1.005";
 
 use constant MAX_UNACKED_CONF_MSGS => 3;
 use constant MIN_UNACKED_CONF_GAP => 2 * 24 * 60 * 60;
@@ -28,13 +28,75 @@ sub columns {
             affiliate_name delivMobile billMobile
             delivStreet2 billStreet2
             billOrganization
-            customInt1 customInt2/;
+            customInt1 customInt2 password_type/;
 }
 
 sub table {
   return "site_users";
 }
 
+sub defaults {
+  require BSE::Util::SQL;
+  return
+    (
+     keepAddress => 1, # what am I for - appears unused
+     whenRegistered => BSE::Util::SQL::now_datetime(),
+     lastLogon => BSE::Util::SQL::now_datetime(),
+     name1 => "",
+     name2 => "",
+     address => "",
+     city => "",
+     state => "",
+     postcode => "",
+     telephone => "",
+     facsimile => "",
+     country => "",
+     wantLetter => 0, # also unused
+     confirmed => 0,
+     confirmSecret => "",
+     waitingForConfirmation => 0,
+     textOnlyMail => 0,
+     title => "",
+     organization => "",
+     referral => 0,
+     otherReferral => "",
+     prompt => 0,
+     otherPrompt => "",
+     profession => 0,
+     otherProfession => "",
+     previousLogon => BSE::Util::SQL::now_datetime(),
+     billFirstName => "",
+     billLastName => "",
+     billStreet => "",
+     billSuburb => "",
+     billState => "", 
+     billPostCode => "",
+     billCountry => "",
+     instructions => "",
+     billTelephone => "",
+     billFacsimile => "", 
+     billEmail => "",
+     adminNotes => "",
+     disabled => 0,
+     flags => "",
+     customText1 => undef,
+     customText2 => undef,
+     customText3 => undef,
+     customStr1 => undef,
+     customStr2 => undef,
+     customStr3 => undef,
+     affiliate_name => "",
+     delivMobile => "",
+     billMobile => "",
+     delivStreet2 => "",
+     billStreet2 => "",
+     billOrganization => "",
+     customInt1 => "",
+     customInt2 => "",
+     #password_type
+    );
+}
+
 sub valid_fields {
   my ($class, $cfg, $admin) = @_;
 
@@ -728,4 +790,34 @@ sub send_registration_notify {
       );
 }
 
+sub changepw {
+  my ($self, $password, $who) = @_;
+
+  require BSE::Passwords;
+
+  my ($hash, $type) = BSE::Passwords->new_password_hash($password);
+
+  $self->set_password($hash);
+  $self->set_password_type($type);
+
+  require BSE::TB::AuditLog;
+  BSE::TB::AuditLog->log
+      (
+       component => "siteusers::changepw",
+       object => $self,
+       actor => $who,
+       level => "info",
+       msg => "Change password",
+      );
+
+  1;
+}
+
+sub check_password {
+  my ($self, $password, $error) = @_;
+
+  require BSE::Passwords;
+  return BSE::Passwords->check_password_hash($self->password, $self->password_type, $password, $error);
+}
+
 1;

diff --git a/site/cgi-bin/modules/SiteUsers.pm b/site/cgi-bin/modules/SiteUsers.pm
index 3a237b29110a28aaa15464c469517f238ae1ecee..5bc65455f303fed67defc63c62934389de5a90ae 100644 (file)
--- a/site/cgi-bin/modules/SiteUsers.pm
+++ b/site/cgi-bin/modules/SiteUsers.pm
@@ -5,7 +5,7 @@ use vars qw(@ISA $VERSION);
 @ISA = qw(Squirrel::Table);
 use SiteUser;
 
-our $VERSION = "1.000";
+our $VERSION = "1.001";
 
 sub rowClass {
   return 'SiteUser';
@@ -23,4 +23,17 @@ sub all_ids {
   map $_->{id}, BSE::DB->query('siteuserAllIds');
 }
 
+sub make {
+  my ($self, %opts) = @_;
+
+  require BSE::Passwords;
+  my $password = delete $opts{password};
+  my ($hash, $type) = BSE::Passwords->new_password_hash($password);
+
+  $opts{password} = $hash;
+  $opts{password_type} = $type;
+
+  return $self->SUPER::make(%opts);
+}
+
 1;

diff --git a/site/util/mysql.str b/site/util/mysql.str
index 7022f85f86b5986bd38ec84d628ca4055f812ec8..d32d37bd4946a7247137c7421d7c2d2b3e8b041d 100644 (file)
--- a/site/util/mysql.str
+++ b/site/util/mysql.str
@@ -599,7 +599,7 @@ Table site_users
 Engine MyISAM
 Column id;int(11);NO;NULL;auto_increment
 Column userId;varchar(40);NO;NULL;
-Column password;varchar(40);NO;NULL;
+Column password;varchar(255);NO;NULL;
 Column email;varchar(255);NO;NULL;
 Column keepAddress;int(11);NO;1;
 Column whenRegistered;datetime;NO;NULL;
@@ -655,6 +655,7 @@ Column billStreet2;varchar(127);NO;;
 Column billOrganization;varchar(127);NO;;
 Column customInt1;int(11);YES;NULL;
 Column customInt2;int(11);YES;NULL;
+Column password_type;varchar(20);NO;plain;
 Index PRIMARY;1;[id]
 Index affiliate_name;0;[affiliate_name]
 Index userId;1;[userId]