Skip to content

Commit

Permalink
Manage the password reset hash as a token
Browse files Browse the repository at this point in the history
Refactor verify.php to be a not-logged-in page (like login_page.php), so
the only action the user can do is change the password, and not navigate
into the site.

If the user does not change the password and quits the page, the
activation token remains valid until the change is effectively done (or
the token times out)

Fixes #20686, #6009, #735

Note: I reworded and reformatted some of the original commit messages.
  • Loading branch information
dregad committed May 14, 2016
2 parents 1b84272 + 777f5e8 commit d7b8d33
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 34 deletions.
4 changes: 2 additions & 2 deletions account_page.php
Expand Up @@ -171,13 +171,13 @@
} else {
?>
<div class="field-container">
<label for="password" <?php echo $t_force_pw_reset_html ?>><span><?php echo lang_get( 'current_password' ) ?></span></label>
<label for="password-current" <?php echo $t_force_pw_reset_html ?>><span><?php echo lang_get( 'current_password' ) ?></span></label>
<span class="input"><input id="password-current" type="password" name="password_current" size="32" maxlength="<?php echo auth_get_password_max_size(); ?>" /></span>
<span class="label-style"></span>
</div>
<?php } ?>
<div class="field-container">
<label for="password" <?php echo $t_force_pw_reset_html ?>><span><?php echo lang_get( 'password' ) ?></span></label>
<label for="password" <?php echo $t_force_pw_reset_html ?>><span><?php echo lang_get( 'new_password' ) ?></span></label>
<span class="input"><input id="password" type="password" name="password" size="32" maxlength="<?php echo auth_get_password_max_size(); ?>" /></span>
<span class="label-style"></span>
</div>
Expand Down
61 changes: 37 additions & 24 deletions account_update.php
Expand Up @@ -57,17 +57,22 @@

form_security_validate( 'account_update' );

$t_user_id = auth_get_current_user_id();

# If token is set, it's a password reset request from verify.php, and if
# not we need to reauthenticate the user
$t_account_verification = token_get_value( TOKEN_ACCOUNT_VERIFY, $t_user_id );
$t_verify_user_id = gpc_get( 'verify_user_id', false );
$t_account_verification = $t_verify_user_id ? token_get_value( TOKEN_ACCOUNT_VERIFY, $t_verify_user_id ) : false;
if( !$t_account_verification ) {
auth_reauthenticate();
$t_user_id = auth_get_current_user_id();
} else {
# set a temporary cookie so the login information is passed between pages.
auth_set_cookies( $t_verify_user_id, false );
# fake login so the user can set their password
auth_attempt_script_login( user_get_field( $t_verify_user_id, 'username' ) );
$t_user_id = $t_verify_user_id;
}

auth_ensure_user_authenticated();

current_user_ensure_unprotected();

$f_email = gpc_get_string( 'email', '' );
Expand All @@ -79,17 +84,24 @@
$t_redirect_url = 'index.php';

# @todo Listing what fields were updated is not standard behaviour of MantisBT - it also complicates the code.
$t_email_updated = false;
$t_password_updated = false;
$t_realname_updated = false;
$t_update_email = null;
$t_update_password = null;
$t_update_realname = null;

# Do not allow blank passwords in account verification/reset
if( $t_account_verification && is_blank( $f_password ) ) {
error_parameters( lang_get( 'password' ) );
trigger_error( ERROR_EMPTY_FIELD, ERROR );
}

$t_ldap = ( LDAP == config_get( 'login_method' ) );

# Update email (but only if LDAP isn't being used)
if( !( $t_ldap && config_get( 'use_ldap_email' ) ) ) {
# Do not update email for a user verification
if( !( $t_ldap && config_get( 'use_ldap_email' ) )
&& !$t_account_verification ) {
if( $f_email != user_get_email( $t_user_id ) ) {
user_set_email( $t_user_id, $f_email );
$t_email_updated = true;
$t_update_email = $f_email;
}
}

Expand All @@ -101,8 +113,7 @@
# checks for problems with realnames
$t_username = user_get_field( $t_user_id, 'username' );
user_ensure_realname_unique( $t_username, $t_realname );
user_set_realname( $t_user_id, $t_realname );
$t_realname_updated = true;
$t_update_realname = $t_realname;
}
}

Expand All @@ -116,37 +127,39 @@
}

if( !auth_does_password_match( $t_user_id, $f_password ) ) {
user_set_password( $t_user_id, $f_password );
$t_password_updated = true;
$t_update_password = $f_password;
}
}
}

form_security_purge( 'account_update' );

# Clear the verification token
if( $t_account_verification ) {
token_delete( TOKEN_ACCOUNT_VERIFY, $t_user_id );
}

html_page_top( null, $t_redirect_url );

$t_message = '';

if( $t_email_updated ) {
if( $t_update_email ) {
user_set_email( $t_user_id, $f_email );
$t_message .= lang_get( 'email_updated' );
}

if( $t_password_updated ) {
if( $t_update_password ) {
user_set_password( $t_user_id, $f_password );
$t_message = is_blank( $t_message ) ? '' : $t_message . '<br />';
$t_message .= lang_get( 'password_updated' );

# Clear the verification token
if( $t_account_verification ) {
token_delete( TOKEN_ACCOUNT_VERIFY, $t_user_id );
}
}

if( $t_realname_updated ) {
if( $t_update_realname ) {
user_set_realname( $t_user_id, $t_realname );
$t_message = is_blank( $t_message ) ? '' : $t_message . '<br />';
$t_message .= lang_get( 'realname_updated' );
}

form_security_purge( 'account_update' );

html_operation_successful( $t_redirect_url, $t_message );

html_page_bottom();
2 changes: 2 additions & 0 deletions core/constant_inc.php
Expand Up @@ -496,6 +496,7 @@
define( 'TOKEN_AUTHENTICATED', 4 );
define( 'TOKEN_COLLAPSE', 5 );
define( 'TOKEN_ACCOUNT_VERIFY', 6 );
define( 'TOKEN_ACCOUNT_ACTIVATION', 7 );
define( 'TOKEN_USER', 1000 );

# token expirations
Expand All @@ -505,6 +506,7 @@
define( 'TOKEN_EXPIRY_LAST_VISITED', 24 * 60 * 60 );
define( 'TOKEN_EXPIRY_AUTHENTICATED', 5 * 60 );
define( 'TOKEN_EXPIRY_COLLAPSE', 365 * 24 * 60 * 60 );
define( 'TOKEN_EXPIRY_ACCOUNT_ACTIVATION', 24 * 60 * 60 );

# config types
define( 'CONFIG_TYPE_DEFAULT', 0 );
Expand Down
4 changes: 4 additions & 0 deletions core/user_api.php
Expand Up @@ -602,6 +602,7 @@ function user_create( $p_username, $p_password, $p_email = '',
# Send notification email
if( !is_blank( $p_email ) ) {
$t_confirm_hash = auth_generate_confirm_hash( $t_user_id );
token_set( TOKEN_ACCOUNT_ACTIVATION, $t_confirm_hash, TOKEN_EXPIRY_ACCOUNT_ACTIVATION, $t_user_id );
email_signup( $t_user_id, $t_confirm_hash, $p_admin_name );
}

Expand Down Expand Up @@ -1566,6 +1567,8 @@ function user_set_password( $p_user_id, $p_password, $p_allow_protected = false
# When the password is changed, invalidate the cookie to expire sessions that
# may be active on all browsers.
$c_cookie_string = auth_generate_unique_cookie_string();
# Delete token for password activation if there is any
token_delete( TOKEN_ACCOUNT_ACTIVATION, $p_user_id );

$c_password = auth_process_plain_password( $p_password );

Expand Down Expand Up @@ -1661,6 +1664,7 @@ function user_reset_password( $p_user_id, $p_send_email = true ) {
# Send notification email
if( $p_send_email ) {
$t_confirm_hash = auth_generate_confirm_hash( $p_user_id );
token_set( TOKEN_ACCOUNT_ACTIVATION, $t_confirm_hash, TOKEN_EXPIRY_ACCOUNT_ACTIVATION, $p_user_id );
email_send_confirm_hash_url( $p_user_id, $t_confirm_hash );
}
} else {
Expand Down
1 change: 1 addition & 0 deletions css/default.css
Expand Up @@ -480,6 +480,7 @@ div.form-container table th,
div.table-container table th {
text-align: left;
}
div#verify-div,
div#reauth-div,
div#login-div,
div#lost-password-div,
Expand Down
3 changes: 2 additions & 1 deletion lang/strings_english.txt
Expand Up @@ -357,13 +357,14 @@ $s_realname_label = 'Real Name';
$s_email = 'E-mail';
$s_email_label = 'E-mail';
$s_password = 'Password';
$s_new_password = 'New password';
$s_no_password_change = 'The password is controlled by another system, hence cannot be edited here.';
$s_confirm_password = 'Confirm Password';
$s_current_password = 'Current Password';
$s_access_level = 'Access Level';
$s_access_level_label = 'Access Level';
$s_update_user_button = 'Update User';
$s_verify_warning = 'Your account information has been verified. The account confirmation message you have received is now invalid.';
$s_verify_warning = 'Your account information has been verified.';
$s_verify_change_password = 'You must set a password here to allow you to log in again.';

# API tokens page
Expand Down
1 change: 1 addition & 0 deletions lost_pwd.php
Expand Up @@ -95,6 +95,7 @@
}

$t_confirm_hash = auth_generate_confirm_hash( $t_user_id );
token_set( TOKEN_ACCOUNT_ACTIVATION, $t_confirm_hash, TOKEN_EXPIRY_ACCOUNT_ACTIVATION, $t_user_id );
email_send_confirm_hash_url( $t_user_id, $t_confirm_hash );

user_increment_lost_password_in_progress_count( $t_user_id );
Expand Down
80 changes: 73 additions & 7 deletions verify.php
Expand Up @@ -61,15 +61,12 @@
print_header_redirect( 'verify.php?id=' . $f_user_id . '&confirm_hash=' . $f_confirm_hash );
}

$t_calculated_confirm_hash = auth_generate_confirm_hash( $f_user_id );
$t_token_confirm_hash = token_get_value( TOKEN_ACCOUNT_ACTIVATION, $f_user_id );

if( $f_confirm_hash != $t_calculated_confirm_hash ) {
if( $f_confirm_hash != $t_token_confirm_hash ) {
trigger_error( ERROR_LOST_PASSWORD_CONFIRM_HASH_INVALID, ERROR );
}

# set a temporary cookie so the login information is passed between pages.
auth_set_cookies( $f_user_id, false );

user_reset_failed_login_count_to_zero( $f_user_id );
user_reset_lost_password_in_progress_count_to_zero( $f_user_id );

Expand All @@ -79,5 +76,74 @@
user_increment_login_count( $f_user_id );


define( 'ACCOUNT_VERIFICATION_INC', true );
include ( dirname( __FILE__ ) . '/account_page.php' );
# extracts the user information
# and prefixes it with u_
$t_row = user_get_row( $f_user_id );

extract( $t_row, EXTR_PREFIX_ALL, 'u' );

$t_can_change_password = helper_call_custom_function( 'auth_can_change_password', array() );

html_page_top1();
html_page_top2a();

?>

<div id="reset-passwd-msg" class="important-msg">
<ul>
<?php
if( $t_can_change_password ) {
echo '<li>' . lang_get( 'verify_warning' ) . '</li>';
echo '<li>' . lang_get( 'verify_change_password' ) . '</li>';
} else {
echo '<li>' . lang_get( 'no_password_change' ) . '</li>';
}
?>
</ul>
</div>

<?php
if( $t_can_change_password ) {
?>

<div id="verify-div" class="form-container">
<form id="account-update-form" method="post" action="account_update.php">
<fieldset class="required">
<legend><span><?php echo lang_get( 'edit_account_title' ); ?></span></legend>
<div class="field-container">
<span class="display-label"><span><?php echo lang_get( 'username' ) ?></span></span>
<span class="input"><span class="field-value"><?php echo string_display_line( $u_username ) ?></span></span>
<span class="label-style"></span>
</div>
<input type="hidden" name="verify_user_id" value="<?php echo $u_id ?>">
<?php
echo form_security_field( 'account_update' );
# When verifying account, set a token and don't display current password
token_set( TOKEN_ACCOUNT_VERIFY, true, TOKEN_EXPIRY_AUTHENTICATED, $u_id );
?>
<div class="field-container">
<label for="realname"><span><?php echo lang_get( 'realname' ) ?></span></label>
<span class="input">
<input id="realname" type="text" size="32" maxlength="<?php echo DB_FIELD_SIZE_REALNAME ?>" name="realname" value="<?php echo string_attribute( $u_realname ) ?>" />
</span>
<span class="label-style"></span>
</div>
<div class="field-container">
<label for="password" class="required"><span><?php echo lang_get( 'new_password' ) ?></span></label>
<span class="input"><input id="password" type="password" name="password" size="32" maxlength="<?php echo auth_get_password_max_size(); ?>" /></span>
<span class="label-style"></span>
</div>
<div class="field-container">
<label for="password-confirm" class="required"><span><?php echo lang_get( 'confirm_password' ) ?></span></label>
<span class="input"><input id="password-confirm" type="password" name="password_confirm" size="32" maxlength="<?php echo auth_get_password_max_size(); ?>" /></span>
<span class="label-style"></span>
</div>
<span class="submit-button"><input type="submit" class="button" value="<?php echo lang_get( 'update_user_button' ) ?>" /></span>
</fieldset>
</form>
</div>

<?php
}

html_page_bottom1a( __FILE__ );

0 comments on commit d7b8d33

Please sign in to comment.