[freeside-commits] branch FREESIDE_4_BRANCH updated. d3f33fa4dbabb61cd94dac9f4f63cd8f249313da

Mark Wells mark at 420.am
Fri Nov 13 10:13:00 PST 2015


The branch, FREESIDE_4_BRANCH has been updated
       via  d3f33fa4dbabb61cd94dac9f4f63cd8f249313da (commit)
       via  75a3ac488dc908290e75edd09471e01dba2199cb (commit)
      from  a93d85b710311eeaeb5cb6f5e987ee3f8e0a1c81 (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 d3f33fa4dbabb61cd94dac9f4f63cd8f249313da
Author: Mark Wells <mark at freeside.biz>
Date:   Thu Nov 12 16:49:39 2015 -0800

    limit password reuse, core and svc_acct, #29354

diff --git a/FS/FS/ClientAPI/MyAccount.pm b/FS/FS/ClientAPI/MyAccount.pm
index 6e76e1d..53deaaa 100644
--- a/FS/FS/ClientAPI/MyAccount.pm
+++ b/FS/FS/ClientAPI/MyAccount.pm
@@ -2982,13 +2982,15 @@ sub myaccount_passwd {
         )
     && ! $svc_acct->check_password($p->{'old_password'});
 
+    # should move password length checks into is_password_allowed
   $error = 'Password too short.'
     if length($p->{'new_password'}) < ($conf->config('passwordmin') || 6);
   $error = 'Password too long.'
     if length($p->{'new_password'}) > ($conf->config('passwordmax') || 8);
 
-  $svc_acct->set_password($p->{'new_password'});
-  $error ||= $svc_acct->replace();
+  $error ||= $svc_acct->is_password_allowed($p->{'new_password'})
+         ||  $svc_acct->set_password($p->{'new_password'})
+         ||  $svc_acct->replace();
 
   #regular pw change in self-service should change contact pw too, otherwise its
   #way too confusing.  hell its confusing they're separate at all, but alas.
@@ -3267,8 +3269,9 @@ sub process_reset_passwd {
 
   if ( $svc_acct ) {
 
-    $svc_acct->set_password($p->{'new_password'});
-    my $error = $svc_acct->replace();
+    my $error ||= $svc_acct->is_password_allowed($p->{'new_password'})
+              ||  $svc_acct->set_password($p->{'new_password'})
+              ||  $svc_acct->replace();
 
     return { %$info, 'error' => $error } if $error;
 
diff --git a/FS/FS/ClientAPI/Signup.pm b/FS/FS/ClientAPI/Signup.pm
index 5d719c4..a178bec 100644
--- a/FS/FS/ClientAPI/Signup.pm
+++ b/FS/FS/ClientAPI/Signup.pm
@@ -694,6 +694,9 @@ sub new_customer {
         map { $_ => $packet->{$_} }
           qw( username _password sec_phrase popnum domsvc ),
       };
+      
+      my $error = $svc->is_password_allowed($packet->{_password});
+      return { error => $error } if $error;
 
       my @acct_snarf;
       my $snarfnum = 1;
diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index ffe5302..a4cc871 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -4053,6 +4053,13 @@ and customer address. Include units.',
   },
 
   {
+    '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',
     'description' => 'All data volumes are expressed in megabytes',
diff --git a/FS/FS/Mason.pm b/FS/FS/Mason.pm
index 98a75c8..58b3da7 100644
--- a/FS/FS/Mason.pm
+++ b/FS/FS/Mason.pm
@@ -409,6 +409,7 @@ if ( -e $addl_handler_use_file ) {
   use FS::report_batch;
   use FS::report_batch;
   use FS::report_batch;
+  use FS::password_history;
   # Sammath Naur
 
   if ( $FS::Mason::addl_handler_use ) {
diff --git a/FS/FS/Password_Mixin.pm b/FS/FS/Password_Mixin.pm
new file mode 100644
index 0000000..af4c5e2
--- /dev/null
+++ b/FS/FS/Password_Mixin.pm
@@ -0,0 +1,165 @@
+package FS::Password_Mixin;
+
+use FS::Record qw(qsearch);
+use FS::Conf;
+use FS::password_history;
+use Authen::Passphrase;
+# use Authen::Passphrase::BlowfishCrypt; # ha ha, no.
+# 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 $me = '[' . __PACKAGE__ . ']';
+
+our $BLOWFISH_COST = 10;
+
+=head1 NAME
+
+FS::Password_Mixin - Object methods for accounts that have passwords governed
+by the password policy.
+
+=head1 METHODS
+
+=over 4
+
+=item is_password_allowed PASSWORD
+
+Checks the password against the system password policy. Returns an error
+message on failure, an empty string on success.
+
+This MUST NOT be called from check(). It should be called by the office UI,
+self-service ClientAPI, or other I<user-interactive> code that processes a
+password change, and only if the user has taken some action with the intent
+of changing the password.
+
+=cut
+
+sub is_password_allowed {
+  my $self = shift;
+  my $password = shift;
+
+  # check length and complexity here
+
+  if ( $conf->config('password-no_reuse') =~ /^(\d+)$/ ) {
+
+    my $no_reuse = $1;
+
+    # "the last N" passwords includes the current password and the N-1
+    # passwords before that.
+    warn "$me checking password reuse limit of $no_reuse\n" if $DEBUG;
+    my @latest = qsearch({
+        'table'     => 'password_history',
+        'hashref'   => { $self->password_history_key => $self->get($self->primary_key) },
+        'order_by'  => " ORDER BY created DESC LIMIT $no_reuse",
+    });
+
+    # don't check the first one; reusing the current password is allowed.
+    shift @latest;
+
+    foreach my $history (@latest) {
+      warn "$me previous password created ".$history->created."\n" if $DEBUG;
+      if ( $history->password_equals($password) ) {
+        my $message;
+        if ( $no_reuse == 1 ) {
+          $message = "This password is the same as your previous password.";
+        } else {
+          $message = "This password was one of the last $no_reuse passwords on this account.";
+        }
+        return $message;
+      }
+    } #foreach $history
+
+  } # end of no_reuse checking
+
+  '';
+}
+
+=item password_history_key
+
+Returns the name of the field in L<FS::password_history> that's the foreign
+key to this table.
+
+=cut
+
+sub password_history_key {
+  my $self = shift;
+  $self->table . '__' . $self->primary_key;
+}
+
+=item insert_password_history
+
+Creates a L<FS::password_history> record linked to this object, with its
+current password.
+
+=cut
+
+sub insert_password_history {
+  my $self = shift;
+  my $encoding = $self->_password_encoding;
+  my $password = $self->_password;
+  my $auth;
+
+  if ( $encoding eq 'bcrypt' or $encoding eq 'crypt' ) {
+
+    # it's smart enough to figure this out
+    $auth = Authen::Passphrase->from_crypt($password);
+
+  } elsif ( $encoding eq 'ldap' ) {
+
+    $password =~ s/^{PLAIN}/{CLEARTEXT}/i; # normalize
+    $auth = Authen::Passphrase->from_rfc2307($password);
+    if ( $auth->isa('Authen::Passphrase::Clear') ) {
+      # then we've been given the password in cleartext
+      $auth = $self->_blowfishcrypt( $auth->passphrase );
+    }
+  
+  } elsif ( $encoding eq 'plain' ) {
+
+    $auth = $self->_blowfishcrypt( $password );
+
+  }
+
+  my $password_history = FS::password_history->new({
+      _password => $auth->as_rfc2307,
+      created   => time,
+      $self->password_history_key => $self->get($self->primary_key),
+  });
+
+  my $error = $password_history->insert;
+  return "recording password history: $error" if $error;
+  '';
+
+}
+
+=item _blowfishcrypt PASSWORD
+
+For internal use: takes PASSWORD and returns a new
+L<Authen::Passphrase::BlowfishCrypt> object representing it.
+
+=cut
+
+sub _blowfishcrypt {
+  my $class = shift;
+  my $passphrase = shift;
+  return Authen::Passphrase::BlowfishCrypt->new(
+    cost => $BLOWFISH_COST,
+    salt_random => 1,
+    passphrase => $passphrase,
+  );
+}
+
+=back
+
+=head1 SEE ALSO
+
+L<FS::password_history>
+
+=cut
+
+1;
diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 1c6e993..9f2cb76 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -7187,6 +7187,52 @@ sub tables_hashref {
                         ],
     },
 
+    'password_history' => {
+      'columns' => [
+        'passwordnum',        'serial',  '',          '', '', '',
+        '_password',          'varchar', 'NULL', $char_d, '', '',
+        'encryption_method',  'varchar', 'NULL', $char_d, '', '',
+        'created',   @date_type,   '', '',
+        # each table that needs password history gets a column here, and
+        # an entry in foreign_keys.
+        'svc_acct__svcnum',     'int', 'NULL', '', '', '',
+        'svc_dsl__svcnum',      'int', 'NULL', '', '', '',
+        'svc_alarm__svcnum',    'int', 'NULL', '', '', '',
+        'agent__agentnum',      'int', 'NULL', '', '', '',
+        'contact__contactnum',  'int', 'NULL', '', '', '',
+        'access_user__usernum', 'int', 'NULL', '', '', '',
+      ],
+      'primary_key' => 'passwordnum',
+      'unique' => [],
+      'index'  => [],
+      'foreign_keys' => [
+                          { columns     => [ 'svc_acct__svcnum' ],
+                            table       => 'svc_acct',
+                            references  => [ 'svcnum' ],
+                          },
+                          { columns     => [ 'svc_dsl__svcnum' ],
+                            table       => 'svc_dsl',
+                            references  => [ 'svcnum' ],
+                          },
+                          { columns     => [ 'svc_alarm__svcnum' ],
+                            table       => 'svc_alarm',
+                            references  => [ 'svcnum' ],
+                          },
+                          { columns    => [ 'agent__agentnum' ],
+                            table      => 'agent',
+                            references => [ 'agentnum' ],
+                          },
+                          { columns    => [ 'contact__contactnum' ],
+                            table      => 'contact',
+                            references => [ 'contactnum' ],
+                          },
+                          { columns    => [ 'access_user__usernum' ],
+                            table      => 'access_user',
+                            references => [ 'usernum' ],
+                          },
+                        ],
+    },
+
     # name type nullability length default local
 
     #'new_table' => {
diff --git a/FS/FS/password_history.pm b/FS/FS/password_history.pm
new file mode 100644
index 0000000..dd527b9
--- /dev/null
+++ b/FS/FS/password_history.pm
@@ -0,0 +1,174 @@
+package FS::password_history;
+use base qw( FS::Record );
+
+use strict;
+use FS::Record qw( qsearch qsearchs );
+use Authen::Passphrase;
+
+# the only bit of autogenerated magic in here
+our @foreign_keys;
+FS::UID->install_callback(sub {
+    @foreign_keys = grep /__/, __PACKAGE__->dbdef_table->columns;
+});
+
+=head1 NAME
+
+FS::password_history - Object methods for password_history records
+
+=head1 SYNOPSIS
+
+  use FS::password_history;
+
+  $record = new FS::password_history \%hash;
+  $record = new FS::password_history { 'column' => 'value' };
+
+  $error = $record->insert;
+
+  $error = $new_record->replace($old_record);
+
+  $error = $record->delete;
+
+  $error = $record->check;
+
+=head1 DESCRIPTION
+
+An FS::password_history object represents a current or past password used
+by a login account, employee, or other account managed within Freeside.  
+FS::password_history inherits from FS::Record.  The following fields are
+currently supported:
+
+=over 4
+
+=item passwordnum - primary key
+
+=item _password - the encrypted password, as an RFC2307-style string
+("{CRYPT}$2a$08$..." or "{MD5}1ab201f..." or similar). This is a serialized
+L<Authen::Passphrase> object.
+
+=item created - the date the password was set to this value. The record with
+the most recent created time is the current password.
+
+=back
+
+Plus one of the following foreign keys:
+
+=over 4
+
+=item svc_acct__svcnum
+
+=item svc_dsl__svcnum
+
+=item svc_alarm__svcnum
+
+=item agent__agentnum
+
+=item contact__contactnum
+
+=item access_user__usernum
+
+=back
+
+=head1 METHODS
+
+=over 4
+
+=item new HASHREF
+
+Creates a new password history record.  To add the record to the database,
+see L<"insert">.
+
+=cut
+
+sub table { 'password_history'; }
+
+=item insert
+
+=item delete
+
+=item replace OLD_RECORD
+
+=item check
+
+Checks all fields to make sure this is a valid password history record.  If
+there is an error, returns the error, otherwise returns false.  Called by the
+insert and replace methods.
+
+=cut
+
+sub check {
+  my $self = shift;
+
+  my $error = 
+    $self->ut_numbern('passwordnum')
+    || $self->ut_anything('_password')
+    || $self->ut_numbern('create')
+    || $self->ut_numbern('create')
+  ;
+  return $error if $error;
+
+  # FKs are mutually exclusive
+  my $fk_in_use;
+  foreach my $fk ( @foreign_keys ) {
+    if ( $self->get($fk) ) {
+      $self->ut_numbern($fk);
+      return "multiple records linked to this password_history" if $fk_in_use;
+      $fk_in_use = $fk;
+    }
+  }
+
+  $self->SUPER::check;
+}
+
+=item linked_acct
+
+Returns the object that's using this password.
+
+=cut
+
+sub linked_acct {
+  my $self = shift;
+
+  foreach my $fk ( @foreign_keys ) {
+    if ( my $val = $self->get($fk) ) {
+      my ($table, $key) = split(/__/, $fk);
+      return qsearchs($table, { $key => $val });
+    }
+  }
+}
+
+=item password_equals PASSWORD
+
+Returns true if PASSWORD (plaintext) is the same as the one stored in the 
+history record, false if not.
+
+=cut
+
+sub password_equals {
+
+  my ($self, $check_password) = @_;
+
+  # _password here is always LDAP-style.
+  try {
+    my $auth = Authen::Passphrase->from_rfc2307($self->_password);
+    return $auth->match($check_password);
+  } catch {
+    # if there's somehow bad data in the _password field, then it doesn't
+    # match anything. much better than having it match _everything_.
+    warn "password_history #" . $self->passwordnum . ": $_";
+    return '';
+  }
+
+}
+
+=back
+
+=head1 BUGS
+
+=head1 SEE ALSO
+
+L<FS::Record>
+
+=cut
+
+1;
+
diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index f307033..d3e23f2 100644
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -4,6 +4,7 @@ use base qw( FS::svc_Domain_Mixin FS::svc_PBX_Mixin
              FS::svc_Radius_Mixin
              FS::svc_Tower_Mixin
              FS::svc_IP_Mixin
+             FS::Password_Mixin
              FS::svc_Common
            );
 
@@ -684,6 +685,9 @@ sub insert {
     'child_objects' => $self->child_objects,
     %options,
   );
+
+  $error ||= $self->insert_password_history;
+
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
@@ -893,6 +897,12 @@ sub replace {
   my $dbh = dbh;
 
   $error = $new->SUPER::replace($old, @_); # usergroup here
+
+  # don't need to record this unless the password was changed
+  if ( $old->_password ne $new->_password ) {
+    $error ||= $new->insert_password_history;
+  }
+
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error if $error;
diff --git a/FS/MANIFEST b/FS/MANIFEST
index 5041ccd..f1195ac 100644
--- a/FS/MANIFEST
+++ b/FS/MANIFEST
@@ -858,3 +858,5 @@ FS/report_batch.pm
 t/report_batch.t
 FS/report_batch.pm
 t/report_batch.t
+FS/password_history.pm
+t/password_history.t
diff --git a/FS/t/password_history.t b/FS/t/password_history.t
new file mode 100644
index 0000000..b7a05fd
--- /dev/null
+++ b/FS/t/password_history.t
@@ -0,0 +1,5 @@
+BEGIN { $| = 1; print "1..1\n" }
+END {print "not ok 1\n" unless $loaded;}
+use FS::password_history;
+$loaded=1;
+print "ok 1\n";
diff --git a/httemplate/edit/process/svc_acct.cgi b/httemplate/edit/process/svc_acct.cgi
index 9cac2c5..d75ff92 100755
--- a/httemplate/edit/process/svc_acct.cgi
+++ b/httemplate/edit/process/svc_acct.cgi
@@ -81,7 +81,12 @@ if (     $cgi->param('clear_password') eq '*HIDDEN*'
       || $cgi->param('clear_password') =~ /^\(.* encrypted\)$/ ) {
   die "fatal: no previous account to recall hidden password from!" unless $old;
 } else {
-  $error ||= $new->set_password($cgi->param('clear_password'));
+  my $newpass = $cgi->param('clear_password');
+  if ( ! $old->check_password($newpass) ) {
+    # then the password is being changed
+    $error ||= $new->is_password_allowed($newpass)
+           ||  $new->set_password($newpass);
+  }
 }
 
 if ( ! $error ) {
diff --git a/httemplate/misc/process/change-password.html b/httemplate/misc/process/change-password.html
index 7cab9c4..d58ce54 100644
--- a/httemplate/misc/process/change-password.html
+++ b/httemplate/misc/process/change-password.html
@@ -11,7 +11,9 @@ die "access denied" unless (
   ( $curuser->access_right('Edit password') and 
     ! $part_svc->restrict_edit_password )
   );
-my $error = $svc_acct->set_password($cgi->param('password'))
+my $newpass = $cgi->param('password');
+my $error = $svc_acct->is_password_allowed($newpass)
+        ||  $svc_acct->set_password($newpass)
         ||  $svc_acct->replace;
 
 # annoyingly specific to view/svc_acct.cgi, for now...

commit 75a3ac488dc908290e75edd09471e01dba2199cb
Author: Mark Wells <mark at freeside.biz>
Date:   Thu Nov 12 15:34:55 2015 -0800

    fix selfservice display when there are term discounts defined, looks like fallout from #15539

diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index eee0958..d3c618d 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -1183,7 +1183,8 @@ sub _make_lines {
       } else {
       # the normal case, not a supplemental package
       $next_bill = $part_pkg->add_freq($sdate, $options{freq_override} || 0);
-      return "unparsable frequency: ". $part_pkg->freq
+      return "unparsable frequency: ".
+        ($options{freq_override} || $part_pkg->freq)
         if $next_bill == -1;
       }  
   
diff --git a/FS/FS/cust_main/Billing_Discount.pm b/FS/FS/cust_main/Billing_Discount.pm
index 117bf31..ec2bf07 100644
--- a/FS/FS/cust_main/Billing_Discount.pm
+++ b/FS/FS/cust_main/Billing_Discount.pm
@@ -92,8 +92,11 @@ sub discount_terms {
 
   my @discount_pkgs = $self->_discount_pkgs_and_bill;
   shift @discount_pkgs; #discard bill;
-  
-  map { $terms{$_->months} = 1 }
+
+  # convert @discount_pkgs (the list of packages that have available discounts)
+  # to a list of distinct term lengths in months, and strip any decimal places
+  # from the number of months, not that it should have any 
+  map { $terms{sprintf('%.0f', $_->months)} = 1 }
     grep { $_->months && $_->months > 1 }
     map { $_->discount }
     map { $_->part_pkg->part_pkg_discount }

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

Summary of changes:
 FS/FS/ClientAPI/MyAccount.pm                 |   11 +-
 FS/FS/ClientAPI/Signup.pm                    |    3 +
 FS/FS/Conf.pm                                |    7 ++
 FS/FS/Mason.pm                               |    1 +
 FS/FS/Password_Mixin.pm                      |  165 ++++++++++++++++++++++++
 FS/FS/Schema.pm                              |   46 +++++++
 FS/FS/cust_main/Billing.pm                   |    3 +-
 FS/FS/cust_main/Billing_Discount.pm          |    7 +-
 FS/FS/password_history.pm                    |  174 ++++++++++++++++++++++++++
 FS/FS/svc_acct.pm                            |   10 ++
 FS/MANIFEST                                  |    2 +
 FS/t/{AccessRight.t => password_history.t}   |    2 +-
 httemplate/edit/process/svc_acct.cgi         |    7 +-
 httemplate/misc/process/change-password.html |    4 +-
 14 files changed, 432 insertions(+), 10 deletions(-)
 create mode 100644 FS/FS/Password_Mixin.pm
 create mode 100644 FS/FS/password_history.pm
 copy FS/t/{AccessRight.t => password_history.t} (79%)




More information about the freeside-commits mailing list