[freeside-commits] branch FREESIDE_4_BRANCH updated. 803f699c6185f061b741d1c7687a482b9ae57520

Mark Wells mark at 420.am
Fri Nov 20 15:09:17 PST 2015


The branch, FREESIDE_4_BRANCH has been updated
       via  803f699c6185f061b741d1c7687a482b9ae57520 (commit)
       via  1ed7b05956d7a2914d10728146fefee41362c867 (commit)
       via  9867909c4592db1331712af78c772c1f1a8d2090 (commit)
      from  5d96e15a61121e8e0c4479363d6a612e702a10da (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 803f699c6185f061b741d1c7687a482b9ae57520
Author: Mark Wells <mark at freeside.biz>
Date:   Fri Nov 20 14:52:24 2015 -0800

    password policy enforcement for svc_dsl, #32456

diff --git a/FS/FS/Password_Mixin.pm b/FS/FS/Password_Mixin.pm
index 9d5421b..393b416 100644
--- a/FS/FS/Password_Mixin.pm
+++ b/FS/FS/Password_Mixin.pm
@@ -128,7 +128,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/svc_dsl.pm b/FS/FS/svc_dsl.pm
index 5704760..dcd6d1d 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/httemplate/edit/process/svc_dsl.html b/httemplate/edit/process/svc_dsl.html
index 627329a..889366e 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>

commit 1ed7b05956d7a2914d10728146fefee41362c867
Author: Mark Wells <mark at freeside.biz>
Date:   Thu Nov 19 16:38:13 2015 -0800

    password policy enforcement for access_users, #32456

diff --git a/FS/FS/Auth/internal.pm b/FS/FS/Auth/internal.pm
index f6d1a00..eea4870 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/access_user.pm b/FS/FS/access_user.pm
index ecab32d..77706b1 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/httemplate/edit/process/access_user.html b/httemplate/edit/process/access_user.html
index 0554bb9..bbe4268 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/pref/pref-process.html b/httemplate/pref/pref-process.html
index 68f0f6e..665bb81 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) {

commit 9867909c4592db1331712af78c772c1f1a8d2090
Author: Mark Wells <mark at freeside.biz>
Date:   Thu Nov 19 14:52:42 2015 -0800

    password policy enforcement for contacts, #32456

diff --git a/FS/FS/ClientAPI/MyAccount.pm b/FS/FS/ClientAPI/MyAccount.pm
index 7e1720d..a6bde82 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 ff29079..c893c10 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/Password_Mixin.pm b/FS/FS/Password_Mixin.pm
index c4549c7..9d5421b 100644
--- a/FS/FS/Password_Mixin.pm
+++ b/FS/FS/Password_Mixin.pm
@@ -105,7 +105,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);
diff --git a/FS/FS/contact.pm b/FS/FS/contact.pm
index 0428d89..e5ddcdc 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;

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/Auth/internal.pm                   |    3 ++
 FS/FS/ClientAPI/MyAccount.pm             |    5 ++-
 FS/FS/ClientAPI/MyAccount/contact.pm     |    2 ++
 FS/FS/Password_Mixin.pm                  |   15 ++++++--
 FS/FS/access_user.pm                     |   16 ++++++++-
 FS/FS/contact.pm                         |   35 +++++++++++++++----
 FS/FS/svc_dsl.pm                         |   55 ++++++++++++++++++++++++++++--
 httemplate/edit/process/access_user.html |    3 +-
 httemplate/edit/process/svc_dsl.html     |   15 ++++++++
 httemplate/pref/pref-process.html        |   20 +++++------
 10 files changed, 144 insertions(+), 25 deletions(-)




More information about the freeside-commits mailing list