From 101264df51f2ae06f9cabf4b394bbee3bc7fedf9 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Thu, 19 Nov 2015 16:38:13 -0800 Subject: [PATCH] password policy enforcement for access_users, #32456 --- FS/FS/Auth/internal.pm | 3 +++ FS/FS/access_user.pm | 16 +++++++++++++++- httemplate/edit/process/access_user.html | 3 ++- httemplate/pref/pref-process.html | 20 ++++++++++---------- 4 files changed, 30 insertions(+), 12 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/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/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/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) { -- 2.11.0