Merge branch 'FREESIDE_4_BRANCH' of git.freeside.biz:/home/git/freeside into FREESIDE_4_BRANCH

12 files changed, 164 insertions, 42 deletions

diff --git a/FS/FS/Auth/internal.pm b/FS/FS/Auth/internal.pm
index f6d1a0086..eea4870d7 100644
--- a/FS/FS/Auth/internal.pm
+++ b/FS/FS/Auth/internal.pm
@@ -47,6 +47,9 @@ sub autocreate { 0; }
 sub change_password {
   my($self, $access_user, $new_password) = @_;
 
+  # do nothing if the password is unchanged
+  return if $self->authenticate( $access_user, $new_password );
+
   $self->change_password_fields( $access_user, $new_password );
 
   $access_user->replace;
diff --git a/FS/FS/ClientAPI/MyAccount.pm b/FS/FS/ClientAPI/MyAccount.pm
index 7e1720da5..a6bde824a 100644
--- a/FS/FS/ClientAPI/MyAccount.pm
+++ b/FS/FS/ClientAPI/MyAccount.pm
@@ -3017,6 +3017,8 @@ sub myaccount_passwd {
                            )
   ) {
     #svc_acct was successful but this one returns an error?  "shouldn't happen"
+    #don't recheck is_password_allowed here; if the svc_acct password was
+    #legal, that's good enough
     $error ||= $contact->change_password($p->{'new_password'});
   }
 
@@ -3298,7 +3300,8 @@ sub process_reset_passwd {
 
   if ( $contact ) {
 
-    my $error = $contact->change_password($p->{'new_password'});
+    my $error = $contact->is_password_allowed($p->{'new_password'})
+            ||  $contact->change_password($p->{'new_password'});
 
     return { %$info, 'error' => $error }; # if $error;
 
diff --git a/FS/FS/ClientAPI/MyAccount/contact.pm b/FS/FS/ClientAPI/MyAccount/contact.pm
index ff29079c7..c893c105d 100644
--- a/FS/FS/ClientAPI/MyAccount/contact.pm
+++ b/FS/FS/ClientAPI/MyAccount/contact.pm
@@ -33,6 +33,8 @@ sub contact_passwd {
   $error = 'Password too long.'
     if length($p->{'new_password'}) > ($conf->config('passwordmax') || 8);
 
+  $error ||= $contact->is_password_allowed($p->{'new_password'});
+
   $error ||= $contact->change_password($p->{'new_password'});
 
   return { 'error' => $error };
diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index a4cc87199..641f925bc 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -4052,13 +4052,13 @@ and customer address. Include units.',
     'type'        => 'checkbox',
   },
 
-  {
-    'key'         => 'password-no_reuse',
-    'section'     => 'password',
-    'description' => 'Minimum number of password changes before a password can be reused. By default, passwords can be reused without restriction.',
-    'type'        => 'text',
-  },
-
+#  {
+#    'key'         => 'password-no_reuse',
+#    'section'     => 'password',
+#    'description' => 'Minimum number of password changes before a password can be reused. By default, passwords can be reused without restriction.',
+#    'type'        => 'text',
+#  },
+#
   {
     'key'         => 'datavolume-forcemegabytes',
     'section'     => 'UI',
diff --git a/FS/FS/Password_Mixin.pm b/FS/FS/Password_Mixin.pm
index c4549c727..990f73595 100644
--- a/FS/FS/Password_Mixin.pm
+++ b/FS/FS/Password_Mixin.pm
@@ -7,13 +7,7 @@ use Authen::Passphrase;
 use Authen::Passphrase::BlowfishCrypt;
 # https://rt.cpan.org/Ticket/Display.html?id=72743
 
-our $DEBUG = 1;
-our $conf;
-FS::UID->install_callback( sub {
-    $conf = FS::Conf->new;
-    # this is safe
-    #eval "use Authen::Passphrase::BlowfishCrypt;";
-});
+our $DEBUG = 0;
 
 our $me = '[' . __PACKAGE__ . ']';
 
@@ -46,9 +40,10 @@ sub is_password_allowed {
 
   # check length and complexity here
 
-  if ( $conf->config('password-no_reuse') =~ /^(\d+)$/ ) {
+  my $no_reuse = 3;
+  # allow override here if we really must
 
-    my $no_reuse = $1;
+  if ( $no_reuse > 0 ) {
 
     # "the last N" passwords includes the current password and the N-1
     # passwords before that.
@@ -105,7 +100,16 @@ sub insert_password_history {
   my $password = $self->_password;
   my $auth;
 
-  if ( $encoding eq 'bcrypt' or $encoding eq 'crypt' ) {
+  if ( $encoding eq 'bcrypt' ) {
+    # our format, used for contact and access_user passwords
+    my ($cost, $salt, $hash) = split(',', $password);
+    $auth = Authen::Passphrase::BlowfishCrypt->new(
+      cost        => $cost,
+      salt_base64 => $salt,
+      hash_base64 => $hash,
+    );
+
+  } elsif ( $encoding eq 'crypt' ) {
 
     # it's smart enough to figure this out
     $auth = Authen::Passphrase->from_crypt($password);
@@ -119,7 +123,9 @@ sub insert_password_history {
       $auth = $self->_blowfishcrypt( $auth->passphrase );
     }
   
-  } elsif ( $encoding eq 'plain' ) {
+  } else {
+    warn "unrecognized password encoding '$encoding'; treating as plain text"
+      unless $encoding eq 'plain';
 
     $auth = $self->_blowfishcrypt( $password );
 
diff --git a/FS/FS/access_user.pm b/FS/FS/access_user.pm
index ecab32d32..77706b1db 100644
--- a/FS/FS/access_user.pm
+++ b/FS/FS/access_user.pm
@@ -1,5 +1,7 @@
 package FS::access_user;
-use base qw( FS::m2m_Common FS::option_Common ); 
+use base qw( FS::Password_Mixin
+             FS::m2m_Common
+             FS::option_Common ); 
 
 use strict;
 use vars qw( $DEBUG $me );
@@ -125,6 +127,9 @@ sub insert {
   }
 
   $error = $self->SUPER::insert(@_);
+  if ( $self->_password ) {
+    $error ||= $self->insert_password_history;
+  }
 
   if ( $error ) {
     $dbh->rollback or die $dbh->errstr if $oldAutoCommit;
@@ -200,6 +205,9 @@ sub replace {
        );
 
   my $error = $new->SUPER::replace($old, @_);
+  if ( $old->_password ne $new->_password ) {
+    $error ||= $new->insert_password_history;
+  }
 
   if ( $error ) {
     $dbh->rollback or die $dbh->errstr if $oldAutoCommit;
@@ -699,6 +707,12 @@ sub is_system_user {
 
 =item change_password NEW_PASSWORD
 
+Changes the user's password to NEW_PASSWORD. This does not check password
+policy rules (see C<is_password_allowed>) and will return an error only if
+editing the user's record fails for some reason.
+
+If NEW_PASSWORD is the same as the existing password, this does nothing.
+
 =cut
 
 sub change_password {
diff --git a/FS/FS/contact.pm b/FS/FS/contact.pm
index 0428d898b..e5ddcdcb6 100644
--- a/FS/FS/contact.pm
+++ b/FS/FS/contact.pm
@@ -1,5 +1,6 @@
 package FS::contact;
-use base qw( FS::Record );
+use base qw( FS::Password_Mixin
+             FS::Record );
 
 use strict;
 use vars qw( $skip_fuzzyfiles );
@@ -187,22 +188,26 @@ sub insert {
 
   }
 
+  my $error;
   if ( $existing_contact ) {
 
     $self->$_($existing_contact->$_())
       for qw( contactnum _password _password_encoding );
-    $self->SUPER::replace($existing_contact);
+    $error = $self->SUPER::replace($existing_contact);
 
   } else {
 
-    my $error = $self->SUPER::insert;
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit;
-      return $error;
-    }
+    $error = $self->SUPER::insert;
 
   }
 
+  $error ||= $self->insert_password_history;
+
+  if ( $error ) {
+    $dbh->rollback if $oldAutoCommit;
+    return $error;
+  }
+
   my $cust_contact = '';
   if ( $custnum ) {
     my %hash = ( 'contactnum' => $self->contactnum,
@@ -426,6 +431,9 @@ sub replace {
   }
 
   my $error = $self->SUPER::replace($old);
+  if ( $old->_password ne $self->_password ) {
+    $error ||= $self->insert_password_history;
+  }
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
@@ -790,9 +798,22 @@ sub authenticate_password {
 
 }
 
+=item change_password NEW_PASSWORD
+
+Changes the contact's selfservice access password to NEW_PASSWORD. This does
+not check password policy rules (see C<is_password_allowed>) and will return
+an error only if editing the record fails for some reason.
+
+If NEW_PASSWORD is the same as the existing password, this does nothing.
+
+=cut
+
 sub change_password {
   my($self, $new_password) = @_;
 
+  # do nothing if the password is unchanged
+  return if $self->authenticate_password($new_password);
+
   $self->change_password_fields( $new_password );
 
   $self->replace;
diff --git a/FS/FS/svc_dsl.pm b/FS/FS/svc_dsl.pm
index 570476005..dcd6d1dbe 100644
--- a/FS/FS/svc_dsl.pm
+++ b/FS/FS/svc_dsl.pm
@@ -1,10 +1,11 @@
 package FS::svc_dsl;
-use base qw(FS::svc_Common);
+use base qw(FS::Password_Mixin
+            FS::svc_Common);
 
 use strict;
 use vars qw( $conf $DEBUG $me );
 use FS::UID;
-use FS::Record qw( qsearch qsearchs );
+use FS::Record qw( qsearch qsearchs dbh );
 use FS::svc_Common;
 use FS::dsl_note;
 use FS::qual;
@@ -211,7 +212,25 @@ otherwise returns false.
 
 =cut
 
-# the insert method can be inherited from FS::Record
+sub insert {
+  my $self = shift;
+  my $dbh = dbh;
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+
+  my $error = $self->SUPER::insert(@_);
+  if ( length($self->password) ) {
+    $error ||= $self->insert_password_history;
+  }
+
+  if ( $error ) {
+    $dbh->rollback if $oldAutoCommit;
+    return $error;
+  }
+
+  $dbh->commit if $oldAutoCommit;
+  '';
+}
 
 =item delete
 
@@ -228,6 +247,27 @@ returns the error, otherwise returns false.
 
 =cut
 
+sub replace {
+  my $new = shift;
+  my $old = shift || $new->replace_old;
+  my $dbh = dbh;
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+
+  my $error = $new->SUPER::replace($old, @_);
+  if ( $old->password ne $new->password ) {
+    $error ||= $new->insert_password_history;
+  }
+
+  if ( $error ) {
+    $dbh->rollback if $oldAutoCommit;
+    return $error;
+  }
+
+  $dbh->commit if $oldAutoCommit;
+  '';
+}
+
 # the replace method can be inherited from FS::Record
 
 =item check
@@ -317,6 +357,15 @@ sub predelete_hook {
     '';
 }
 
+# password_history compatibility
+
+sub _password {
+  my $self = shift;
+  $self->get('password');
+}
+
+sub _password_encoding { 'plain'; }
+
 =back
 
 =head1 SEE ALSO
diff --git a/debian/freeside-torrus.postinst b/debian/freeside-torrus.postinst
index 5cc8accad..d39677ee6 100644
--- a/debian/freeside-torrus.postinst
+++ b/debian/freeside-torrus.postinst
@@ -2,6 +2,14 @@
 
 chown freeside.freeside /var/log/torrus
 chown -R freeside.freeside /var/torrus
-mkdir /srv/torrus/; mkdir /srv/torrus/collector_rrd
+
+if [ ! -d  /srv/torrus/ ]; then
+mkdir /srv/torrus/;
+fi
+
+if [ ! -d /srv/torrus/collector_rrd ]; then
+mkdir /srv/torrus/collector_rrd;
+fi
+
 chown -R freeside:freeside /srv/torrus/collector_rrd /usr/local/etc/torrus/discovery /usr/local/etc/torrus/xmlconfig/
 torrus clearcache
diff --git a/httemplate/edit/process/access_user.html b/httemplate/edit/process/access_user.html
index 0554bb940..bbe4268be 100644
--- a/httemplate/edit/process/access_user.html
+++ b/httemplate/edit/process/access_user.html
@@ -43,7 +43,8 @@ sub post_new_object_callback {
 
   if ( length($cgi->param('_password')) ) {
     my $password = scalar($cgi->param('_password'));
-    $access_user->change_password_fields($password);
+    my $error = $access_user->is_password_allowed($password)
+             || $access_user->change_password($password);
   }
 
 }
diff --git a/httemplate/edit/process/svc_dsl.html b/httemplate/edit/process/svc_dsl.html
index 627329a00..889366e07 100644
--- a/httemplate/edit/process/svc_dsl.html
+++ b/httemplate/edit/process/svc_dsl.html
@@ -1,5 +1,6 @@
 <% include( 'elements/svc_Common.html',
                'table'    => 'svc_dsl',
+               'precheck_callback' => $precheck_callback,
            )
 %>
 <%init>
@@ -7,4 +8,18 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Provision customer service'); #something else more specific?
 
+my $precheck_callback = sub {
+  my $cgi = shift;
+  my $svcnum = $cgi->param('svcnum');
+  my $error = '';
+  if ( $svcnum ) {
+    my $old = FS::svc_dsl->by_key($svcnum);
+    my $newpass = $cgi->param('password');
+    if ( $old and $newpass ne $old->password ) {
+      $error ||= $old->is_password_allowed($newpass);
+    }
+  }
+  $error;
+};
+
 </%init>
diff --git a/httemplate/pref/pref-process.html b/httemplate/pref/pref-process.html
index 68f0f6e2f..665bb81c2 100644
--- a/httemplate/pref/pref-process.html
+++ b/httemplate/pref/pref-process.html
@@ -7,6 +7,8 @@
 % }
 <%init>
 
+my $access_user = $FS::CurrentUser::CurrentUser;
+
 if ( FS::Conf->new->exists('disable_acl_changes') ) {
   errorpage("Preference changes disabled in public demo");
   die "shouldn't be reached";
@@ -19,29 +21,27 @@ if ( FS::Auth->auth_class->can('change_password')
             qw(_password new_password new_password2)
    ) {
 
-  if ( $cgi->param('new_password') ne $cgi->param('new_password2') ) {
+  my $oldpass = $cgi->param('_password');
+  my $newpass = $cgi->param('new_password');
+
+  if ( $newpass ne $cgi->param('new_password2') ) {
     $error = "New passwords don't match";
 
-  } elsif ( ! length($cgi->param('new_password')) ) {
+  } elsif ( ! length($newpass) ) {
     $error = 'No new password entered';
 
-  } elsif ( ! FS::Auth->authenticate( $FS::CurrentUser::CurrentUser,
-                                      scalar($cgi->param('_password')) )
-          ) {
+  } elsif ( ! FS::Auth->authenticate( $access_user, $oldpass ) ) {
     $error = 'Current password incorrect; password not changed';
 
   } else {
 
-    $error = $FS::CurrentUser::CurrentUser->change_password(
-      scalar($cgi->param('new_password'))
-    );
+    $error = $access_user->is_password_allowed($newpass)
+          || $access_user->change_password($newpass);
 
   }
 
 }
 
-my $access_user = $FS::CurrentUser::CurrentUser;
-
 #well, if you got your password change wrong, you don't get anything else
 #changed right now.  but it should be sticky on the form
 unless ( $error ) { # if ($access_user) {