From: Ivan Kohler Date: Mon, 23 Nov 2015 03:44:48 +0000 (-0800) Subject: Merge branch 'master' of git.freeside.biz:/home/git/freeside X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=commitdiff_plain;h=c20f301dd7c437a0d8be414a174dd09721bd8e9a;hp=76f5eb6da76cf9444a5cfee13d6f5d7fd7e0315f Merge branch 'master' of git.freeside.biz:/home/git/freeside --- 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 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) 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) 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. 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 diff --git a/debian/control b/debian/control index 2764f706d..30e9f31dd 100644 --- a/debian/control +++ b/debian/control @@ -27,7 +27,8 @@ Depends: gnupg,ghostscript,gsfonts,gzip,latex-xcolor, libcache-simple-timedexpiry-perl,libchart-perl,libclass-container-perl, libclass-data-inheritable-perl,libclass-returnvalue-perl,libcolor-scheme-perl, libcompress-zlib-perl,libconvert-binhex-perl,libcrypt-passwdmd5-perl, - libcrypt-ssleay-perl,libcss-squish-perl,libdate-manip-perl,libdbd-mysql-perl, + libcrypt-ssleay-perl,libcss-squish-perl, + libdata-password-perl,libdate-manip-perl,libdbd-mysql-perl, libdbd-pg-perl,libdbi-perl,libdbix-dbschema-perl,libdbix-searchbuilder-perl, libdevel-stacktrace-perl,libdevel-symdump-perl,liberror-perl, libexcel-writer-xlsx-perl,libexception-class-perl,libfile-counterfile-perl, diff --git a/debian/freeside-torrus.postinst b/debian/freeside-torrus.postinst index 5cc8accad..d39677ee6 100644 --- a/debian/freeside-torrus.postinst +++ b/debian/freeside-torrus.postinst @@ -2,6 +2,14 @@ chown freeside.freeside /var/log/torrus chown -R freeside.freeside /var/torrus -mkdir /srv/torrus/; mkdir /srv/torrus/collector_rrd + +if [ ! -d /srv/torrus/ ]; then +mkdir /srv/torrus/; +fi + +if [ ! -d /srv/torrus/collector_rrd ]; then +mkdir /srv/torrus/collector_rrd; +fi + chown -R freeside:freeside /srv/torrus/collector_rrd /usr/local/etc/torrus/discovery /usr/local/etc/torrus/xmlconfig/ torrus clearcache diff --git a/httemplate/edit/process/access_user.html b/httemplate/edit/process/access_user.html index 0554bb940..bbe4268be 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/edit/process/svc_dsl.html b/httemplate/edit/process/svc_dsl.html index 627329a00..889366e07 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; +}; + diff --git a/httemplate/edit/svc_acct.cgi b/httemplate/edit/svc_acct.cgi index 31678a991..0cf0c20e1 100755 --- a/httemplate/edit/svc_acct.cgi +++ b/httemplate/edit/svc_acct.cgi @@ -50,7 +50,12 @@ 'required' => $part_svc->part_svc_column('_password')->required ) %> MAXLENGTH=<% $pmax %>> - <& /elements/random_pass.html, 'clear_password' &> + <& /elements/random_pass.html, 'clear_password' &>
+
+ <& '/elements/validate_password.html', + 'fieldid' => 'clear_password', + 'svcnum' => $svcnum + &> %}else{ diff --git a/httemplate/elements/change_password.html b/httemplate/elements/change_password.html index 625ba1fb5..7d8daaeaf 100644 --- a/httemplate/elements/change_password.html +++ b/httemplate/elements/change_password.html @@ -16,6 +16,12 @@ <& /elements/random_pass.html, $pre.'password', 'randomize' &> +
+ <& '/elements/validate_password.html', + 'fieldid' => $pre.'password', + 'svcnum' => $svc_acct->svcnum, + + &> % if ( $error ) {
<% $error |h %> % } diff --git a/httemplate/elements/random_pass.html b/httemplate/elements/random_pass.html index b215b77d9..14bbb581d 100644 --- a/httemplate/elements/random_pass.html +++ b/httemplate/elements/random_pass.html @@ -1,13 +1,23 @@ diff --git a/httemplate/elements/validate_password.html b/httemplate/elements/validate_password.html new file mode 100644 index 000000000..fd2cb6ca0 --- /dev/null +++ b/httemplate/elements/validate_password.html @@ -0,0 +1,58 @@ +<%doc> + +To validate passwords via javascript/xmlhttp: + + +
+ <& '/elements/validate_password.html', + fieldid => 'password_field', + svcnum => $svcnum + &> + +The ID of the input field can be anything; the ID of the DIV in which to display results +should be the input id plus '_result'. + + + +<& '/elements/xmlhttp.html', + 'url' => $p.'misc/xmlhttp-validate_password.html', + 'subs' => [ 'validate_password' ], + 'method' => 'POST', # important not to put passwords in url +&> + + +<%init> +my %opt = @_; + + + diff --git a/httemplate/misc/xmlhttp-validate_password.html b/httemplate/misc/xmlhttp-validate_password.html new file mode 100644 index 000000000..28dbf6460 --- /dev/null +++ b/httemplate/misc/xmlhttp-validate_password.html @@ -0,0 +1,50 @@ +<%doc> +Requires cgi params 'password' (plaintext) and 'sub' ('validate_password' is only +acceptable value.) Also accepts 'svcnum' (for svc_acct, will otherwise create an +empty dummy svc_acct) and 'fieldid' (for html post-processing, passed along in +results for convenience.) + +Returns a json-encoded hashref with keys of 'valid' (set to 1 if object is valid), +'error' (error text if password is invalid) or 'syserror' (error text if password +could not be validated.) Only one of these keys will be set. Will also set +'fieldid' if it was passed. + + +<% encode_json($result) %> + +<%init> + +my $validate_password = sub { + my %arg = $cgi->param('arg'); + my %result; + + $result{'fieldid'} = $arg{'fieldid'} + if $arg{'fieldid'} =~ /^\w+$/; + + $result{'syserror'} = 'Request is not POST' unless $cgi->request_method eq 'POST'; + return \%result if $result{'syserror'}; + + my $password = $arg{'password'}; + $result{'syserror'} = 'Invoked without password' unless $password; + return \%result if $result{'syserror'}; + + my $svcnum = $arg{'svcnum'}; + $result{'syserror'} = 'Invalid svcnum' unless $svcnum =~ /^\d*$/; + return \%result if $result{'syserror'}; + + my $svc_acct = $svcnum + ? qsearchs('svc_acct',{'svcnum' => $svcnum}) + : (new FS::svc_acct {}); + $result{'syserror'} = 'Could not find service' unless $svc_acct; + return \%result if $result{'syserror'}; + + $result{'error'} = $svc_acct->is_password_allowed($password); + $result{'valid'} = 1 unless $result{'error'}; + return \%result; +}; + +my $result = ($cgi->param('sub') eq 'validate_password') + ? &$validate_password() + : { 'syserror' => 'Invalid sub' }; + + diff --git a/httemplate/pref/pref-process.html b/httemplate/pref/pref-process.html index 68f0f6e2f..665bb81c2 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) {