Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Front end password change throws InvalidArgumentException instead of form error. #3585

Closed
Gnative opened this issue Dec 21, 2018 · 1 comment

Comments

@Gnative
Copy link

Gnative commented Dec 21, 2018

Description

Updating a user email or password from a front end form when the current password is either incorrect or empty throws InvalidArgumentException when it should give a form error of 'Incorrect current password' .

Issue is _verifyExistingPassword method does not return a boolean see below for solution.

Steps to reproduce

  1. Create front-end template with form
{% macro errorList(errors) %}
    {% if errors %}
    <ul class="errors" style="color:red;">
        {% for error in errors %}
        <li>{{ error }}</li>
        {% endfor %}
    </ul>
    {% endif %}
{% endmacro %}

{% from _self import errorList %}

<form method="post" accept-charset="UTF-8">
    {{ csrfInput() }}

    <input type="hidden" name="action" value="users/save-user">
    <input type="hidden" name="userId" value="{{ currentUser.id }}">
    
    <p>Please enter your current password to make any change to your email or create a new password</p>

    <div>
        <label for="password">Current Password:</label>
        <input type="password" id="password" name="password">
        {% if user is defined and user.getFirstError('currentPassword') %}{{ errorList(user.getErrors('currentPassword'))  }}{% endif %}
    </div>

    <hr />

    <div>
        <label for="email">Email:</label>
        <input type="text" id="email" name="email" value="{{ currentUser.email }}">
        {% if user is defined and user.getFirstError('email') %}{{ errorList(user.getErrors('email'))  }}{% endif %}
    </div>
    <div>
        <label for="newPassword">New Password:</label>
        <small>Leave blank if you do no wish to change your password</small>
        <input type="password" id="newPassword" name="newPassword">
        {% if user is defined and user.getFirstError('newPassword') %}{{ errorList(user.getErrors('newPassword'))  }}{% endif %}
    </div>

    <input type="submit" value="Save">

</form>
  1. Login to admin
  2. Visit form and edit email with incorrect or empty current password field input. Should throw the InvalidArgumentException instead of expected behaviour of returning a 'Incorrect current password' form error

Additional info

Possible fix.
Change _verifyExistingPassword on UserController.php Line 1763 to catch Exception


    /**
     * Verifies that the current user's password was submitted with the request.
     *
     * @return bool
     */
    private function _verifyExistingPassword(): bool
    {
        $currentUser = Craft::$app->getUser()->getIdentity();

        if (!$currentUser) {
            return false;
        }

        $currentHashedPassword = $currentUser->password;
        $currentPassword = Craft::$app->getRequest()->getRequiredParam('password');
        
        try {
            return Craft::$app->getSecurity()->validatePassword($currentPassword, $currentHashedPassword);
        } catch (InvalidArgumentException $e) {
            return false;
        }
    }

  • Craft version: 3.0.36
  • PHP version: 7.2
  • Database driver & version: MySQL 10
  • Plugins & versions: -
@Gnative Gnative changed the title Front end password change throws InvalidArgumentException instead of boolean Front end password change throws InvalidArgumentException instead of form error. Dec 21, 2018
@brandonkelly
Copy link
Member

Submitting an incorrect current password worked as expected for me – the page reloaded with a “Incorrect current password.” error. However I was able to reproduce the exception if the current password was left blank. Just fixed that for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants