Skip to content

Commit

Permalink
Item14506: Clean up the password change process
Browse files Browse the repository at this point in the history 
 - Make sure admin doesn't try to change the admin password
 - Allow admins to change anyone's password without knowing the old
   password - but verify the old password if entered.
 - Other misc. cleanup and restructuring.
  • Loading branch information
@gac410
gac410 committed Oct 12, 2017
1 parent 699918a commit 21409f4
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 48 deletions.
41 changes: 27 additions & 14 deletions PasswordManagementPlugin/data/System/ChangePassword.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,43 +37,56 @@ Once your password has been changed, the restriction will be removed."}%

<form name="changepasswd" action="%SCRIPTURLPATH{"rest"}%/PasswordManagementPlugin/changePassword" method="post">
<div id="changePassword" class="foswikiFormSteps">
%IF{"context isadmin" then="
<div class='foswikiFormStep'>
<h3>User ID (WikiName or Login name) <span class='required'>Required</span></h3>
<div class='col1'>
<p>
<input type='text' name='username' value='%WIKINAME%' size='40' class='foswikiInputField' />
</p>
</div><!--//col1-->
<div class='foswikiClear'></div>
</div><!--//foswikiFormStep-->
"}%
%IF{"'%SESSION_VARIABLE{"FOSWIKI_TOPICRESTRICTION"}%' = '%WEB%.%TOPIC%'" else="
<div class='foswikiFormStep'>
<h3>%MAKETEXT{"Current password:"}%</h3>
<h3>%MAKETEXT{"Current password:"}%%IF{"context isadmin" else=" <span class='required'>Required</span>"}%</h3>
<div class='col1'>
<p>
<input type='password' name='oldpassword' size='40' class='foswikiInputField' />
</p>
</div><!--//col1-->
<div class='col2'>
%IF{"context isadmin" then="
<p>
%MAKETEXT{"Old password is optional for administrators. If entered, it will be validated."}%
<p>
"}%
</div><!--//col2-->
<div class='foswikiClear'></div>
</div><!--//foswikiFormStep-->
"}%
<div class="foswikiFormStep">
<div class="col1">
<h3>%MAKETEXT{"New password:"}%</h3>
<h3>%MAKETEXT{"New password:"}% <span class='required'>Required</span></h3>
<p>
<input type="password" name="password" size="40" class="foswikiInputField" />
</p>
</div><!--//col1-->
<div class='col2'>
<h3>%MAKETEXT{"Retype new password:"}%</h3>
<h3>%MAKETEXT{"Retype new password:"}% <span class='required'>Required</span></h3>
<p>
<input type="password" name="passwordA" size="40" class="foswikiInputField" />
</p>
</div><!--//col2-->
<div class="foswikiClear"></div>
</div><!--//foswikiFormStep-->
%IF{"context isadmin" then="
<div class='foswikiFormStep'>
<h3>User ID (WikiName or Login name) <span class='required'>Optional</span></h3>
<div class='col1'>
<p>
<input type='text' name='username' size='40' class='foswikiInputField' />
</p>
</div><!--//col1-->
<div class='col2'>
<p>
%MAKETEXT{"Administrators can change passwords for any user, without
entering the old password. Enter an optional WikiName or Login
name here to change that user\'s password."}%
</div><!--//col2-->
<div class='foswikiClear'></div>
</div><!--//foswikiFormStep-->
"}%
<div class="foswikiFormStep">
<input type="submit" class="foswikiSubmit" value="%MAKETEXT{"Change Password"}%" />
</div><!--//foswikiFormStep-->
Expand Down
58 changes: 24 additions & 34 deletions PasswordManagementPlugin/lib/Foswiki/Plugins/PasswordManagementPlugin/Core.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,22 @@ sub _RESTchangePassword {
my $loginManager = $session->getLoginManager();

my $oldpassword = $query->param('oldpassword');
my $login = $query->param('username');
my $login = $query->param('username') || $requestUser;
my $passwordA = $query->param('password');
my $passwordB = $query->param('passwordA');

# check if required fields are filled in
unless ($login) {
if ( $login eq $Foswiki::cfg{AdminUserLogin}
|| $login eq $Foswiki::cfg{AdminUserWikiName} )
{
throw Foswiki::OopsException(
'attention',
web => $webName,
topic => $topic,
def => 'missing_fields',
params => ['username']
'password',
web => $webName,
topic => $topic,
def => 'not_admin',
);
}

my $users = $session->{users};
my $users = $session->{users}; # Get the Foswiki::Users object

my $user = Foswiki::Func::getCanonicalUserID($login);
unless ( $user && $session->{users}->userExists($user) ) {
Expand All @@ -205,7 +205,7 @@ sub _RESTchangePassword {

unless ( defined $passwordA ) {
throw Foswiki::OopsException(
'attention',
'password',
web => $webName,
topic => $topic,
def => 'missing_fields',
Expand All @@ -216,7 +216,7 @@ sub _RESTchangePassword {
# check if passwords are identical
if ( $passwordA ne $passwordB ) {
throw Foswiki::OopsException(
'register',
'password',
web => $webName,
topic => $topic,
def => 'password_mismatch'
Expand All @@ -226,48 +226,38 @@ sub _RESTchangePassword {
my $resetActive = $loginManager->getSessionValue('FOSWIKI_PASSWORDRESET');

if ($resetActive) {
$oldpassword = 1;
$oldpassword = 1; # Allow password change without oldpassword.
}
elsif ( $users->isAdmin($requestUser)
&& ! length($oldpassword) ) {
$oldpassword = 1; # Allow an admin to omit the oldpassword
}
else {
# check if required fields are filled in
unless ( defined $oldpassword || $users->isAdmin($requestUser) ) {
unless ( defined $oldpassword ) {
throw Foswiki::OopsException(
'attention',
'password',
web => $webName,
topic => $topic,
def => 'missing_fields',
params => ['oldpassword']
);
}

unless ( $users->isAdmin($requestUser)
|| $users->checkPassword( $login, $oldpassword ) )
unless ( $users->checkPassword( $login, $oldpassword ) )
{
throw Foswiki::OopsException(
'register',
'password',
web => $webName,
topic => $topic,
def => 'wrong_password'
);
}
}

my $cUID = $users->getCanonicalUserID($login);

# Determine that the cUID exists.
unless ( defined $cUID ) {
throw Foswiki::OopsException(
'register',
web => $webName,
topic => $topic,
def => 'not_a_user',
params => [$login]
);
}

if ( length($passwordA) < $Foswiki::cfg{MinPasswordLength} ) {
throw Foswiki::OopsException(
'register',
'password',
web => $webName,
topic => $topic,
def => 'bad_password',
Expand All @@ -276,9 +266,9 @@ sub _RESTchangePassword {
}

# OK - password may be changed
unless ( $users->setPassword( $cUID, $passwordA, $oldpassword ) ) {
unless ( $users->setPassword( $user, $passwordA, $oldpassword ) ) {
throw Foswiki::OopsException(
'register',
'password',
web => $webName,
topic => $topic,
def => 'password_not_changed'
Expand All @@ -299,7 +289,7 @@ sub _RESTchangePassword {

# OK - password changed
throw Foswiki::OopsException(
'register',
'password',
status => 200,
web => $webName,
topic => $topic,
Expand Down
8 changes: 8 additions & 0 deletions PasswordManagementPlugin/templates/passwordmessages.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@
%MAKETEXT{"Please go back in your browser and try again."}%
%TMPL:END%
%{==============================================================================}%
%TMPL:DEF{"not_admin"}%
---+++ %MAKETEXT{"Invalid user [_1]" args="%PARAM1%"}%
%MAKETEXT{"The super admin username may not be changed here. See [[[_1]][configure =Security and Authentication -> Passwords= tab]]" args="%SCRIPTURL{"configure"}%"}%

%MAKETEXT{"For more information, see [[[_1]][The Installation Guide]]." args="%SCRIPTURLPATH{"view" topic="System.InstallationGuide" #="Establishing_an_internal_admin_login_40optional_41"}%"}%

%MAKETEXT{"See [_1] for a list of existing users or register as new user in [_2]." args="%USERSWEB%.%WIKIUSERSTOPIC%, %SYSTEMWEB%.UserRegistration"}% %TMPL:END%
%{==============================================================================}%
%TMPL:DEF{"not_a_user"}%
---+++ %MAKETEXT{"Invalid user [_1]" args="%PARAM1%"}%
%MAKETEXT{"The username you entered may not exist, or may not be eligible for password reset. Check the spelling of [_1] and try again. If you get stuck, please contact [_2]." args="%PARAM1%, %WIKIWEBMASTER%"}%
Expand Down

0 comments on commit 21409f4

Please sign in to comment.