[freeside-commits] branch master updated. c44432a5f0f1c1841ff8b50e734a30bd9aeef945
Mark Wells
mark at 420.am
Thu Nov 12 16:50:03 PST 2015
The branch, master has been updated
via c44432a5f0f1c1841ff8b50e734a30bd9aeef945 (commit)
via bc4c63e61b2113088d164dc86ebca429e219fc0b (commit)
from 03c5dc73291c514d7373d3a15d671018edeb9568 (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 c44432a5f0f1c1841ff8b50e734a30bd9aeef945
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 60c8e9a..5a2a9be 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -7206,6 +7206,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 bc4c63e61b2113088d164dc86ebca429e219fc0b
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