[freeside-commits] branch master updated. 42ebaebeafc054ca0c3a924efd7a53154fdcf35e

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


The branch, master has been updated
       via  42ebaebeafc054ca0c3a924efd7a53154fdcf35e (commit)
       via  450d0dec955bf6ae8d7acdc13a8bfc19777089d3 (commit)
       via  101264df51f2ae06f9cabf4b394bbee3bc7fedf9 (commit)
       via  a68f2e9239ad5cde3bd25ca7aea6af7e0f2ce75f (commit)
      from  cba31c62f266311cca1af8d80ab8bfb6ea5fff31 (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 42ebaebeafc054ca0c3a924efd7a53154fdcf35e
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 450d0dec955bf6ae8d7acdc13a8bfc19777089d3
Merge: 101264d cba31c6
Author: Mark Wells <mark at freeside.biz>
Date:   Fri Nov 20 14:51:40 2015 -0800

    Merge branch 'master' of git.freeside.biz:/home/git/freeside


commit 101264df51f2ae06f9cabf4b394bbee3bc7fedf9
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 a68f2e9239ad5cde3bd25ca7aea6af7e0f2ce75f
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