diff options
Diffstat (limited to 'FS')
-rw-r--r-- | FS/FS/Auth/internal.pm | 3 | ||||
-rw-r--r-- | FS/FS/ClientAPI/MyAccount.pm | 11 | ||||
-rw-r--r-- | FS/FS/ClientAPI/MyAccount/contact.pm | 2 | ||||
-rw-r--r-- | FS/FS/Conf.pm | 14 | ||||
-rw-r--r-- | FS/FS/Password_Mixin.pm | 67 | ||||
-rw-r--r-- | FS/FS/access_user.pm | 16 | ||||
-rw-r--r-- | FS/FS/contact.pm | 35 | ||||
-rw-r--r-- | FS/FS/svc_acct.pm | 25 | ||||
-rw-r--r-- | FS/FS/svc_dsl.pm | 55 |
9 files changed, 194 insertions, 34 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..9847e5f90 100644 --- a/FS/FS/ClientAPI/MyAccount.pm +++ b/FS/FS/ClientAPI/MyAccount.pm @@ -2995,12 +2995,6 @@ 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); - $error ||= $svc_acct->is_password_allowed($p->{'new_password'}) || $svc_acct->set_password($p->{'new_password'}) || $svc_acct->replace(); @@ -3017,6 +3011,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 +3294,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..bcad54637 100644 --- a/FS/FS/Password_Mixin.pm +++ b/FS/FS/Password_Mixin.pm @@ -6,13 +6,12 @@ use FS::password_history; use Authen::Passphrase; use Authen::Passphrase::BlowfishCrypt; # https://rt.cpan.org/Ticket/Display.html?id=72743 +use Data::Password qw(:all); -our $DEBUG = 1; +our $DEBUG = 0; our $conf; FS::UID->install_callback( sub { - $conf = FS::Conf->new; - # this is safe - #eval "use Authen::Passphrase::BlowfishCrypt;"; + $conf = FS::Conf->new; }); our $me = '[' . __PACKAGE__ . ']'; @@ -44,11 +43,39 @@ sub is_password_allowed { my $self = shift; my $password = shift; - # check length and complexity here + # basic checks using Data::Password; + # options for Data::Password + $DICTIONARY = 4; # minimum length of disallowed words + $MINLEN = $conf->config('passwordmin') || 6; + $MAXLEN = $conf->config('passwordmax') || 8; + $GROUPS = 4; # must have all 4 'character groups': numbers, symbols, uppercase, lowercase + # other options use the defaults listed below: + # $FOLLOWING = 3; # disallows more than 3 chars in a row, by alphabet or keyboard (ie abcd or asdf) + # $SKIPCHAR = undef; # set to true to skip checking for bad characters + # # lists of disallowed words + # @DICTIONARIES = qw( /usr/share/dict/web2 /usr/share/dict/words /usr/share/dict/linux.words ); + + my $error = IsBadPassword($password); + $error = 'must contain at least one each of numbers, symbols, and lowercase and uppercase letters' + if $error eq 'contains less than 4 character groups'; # avoid confusion + $error = 'Invalid password - ' . $error if $error; + return $error if $error; + + #check against known usernames + my @disallowed_names = $self->password_disallowed_names; + foreach my $noname (@disallowed_names) { + if ($password =~ /$noname/i) { + #keeping message ambiguous to avoid leaking personal info + return 'Password contains a disallowed word'; + } + } + + return '' unless $self->get($self->primary_key); # for validating new passwords pre-insert - 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. @@ -80,6 +107,17 @@ sub is_password_allowed { ''; } +=item password_disallowed_names + +Override to return a list additional words (eg usernames) not +to be used by passwords on this service. + +=cut + +sub password_disallowed_names { + return (); +} + =item password_history_key Returns the name of the field in L<FS::password_history> that's the foreign @@ -105,7 +143,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 +166,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_acct.pm b/FS/FS/svc_acct.pm index 9323976cb..e7ec4a231 100644 --- a/FS/FS/svc_acct.pm +++ b/FS/FS/svc_acct.pm @@ -2676,6 +2676,31 @@ sub virtual_maildir { $self->domain. '/maildirs/'. $self->username. '/'; } +=item password_disallowed_names + +Override, for L<FS::Password_Mixin>. Not really intended for other use. + +=cut + +sub password_disallowed_names { + my $self = shift; + my $dbh = dbh; + my $results = {}; + foreach my $field ( qw( username finger ) ) { + my $sql = 'SELECT DISTINCT '.$field.' FROM svc_acct'; + my $sth = $dbh->prepare($sql) + or die "Error preparing $sql: ". $dbh->errstr; + $sth->execute() + or die "Error executing $sql: ". $sth->errstr; + foreach my $row (@{$sth->fetchall_arrayref}, $self->get($field)) { + foreach my $word (split(/\s+/,$$row[0])) { + $results->{lc($word)} = 1; + } + } + } + return keys %$results; +} + =back =head1 CLASS METHODS 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 |