Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Only verify current password if credential field exists. #129

Closed
wants to merge 2 commits into from

4 participants

@rhutchison

Current service does not allow you to remove credential from the form.

This fix allows you to remove credential, but still forces the most secure option. To completely remove credential, you also need to use a custom filter to replace ZfcUser\Form\ChangePasswordFilter

@bjyoungblood

I don't like that the check is implicit, and is only based on the existence of a field. Someone who is not using a form validator (obviously not a good practice) would then unwittingly make themselves vulnerable to CSRF.

Perhaps instead of only verifying the password if the credential field exists, add an optional flag to the method that will determine whether the check is performed. The signature would become changePassword(array $data, $checkCredential = true) or something similar. This way, the check is done (or not) explicitly and doesn't rely on people properly using input filters.

@rhutchison

I think the strategy needs to be thought through with a new PR. The current implementation prevents you from removing the credential field and this PR is only intended to address that.

@akrabat
Owner

I agree with @bjyoundblood. There needs to be an explicit developer action to enable changing password without providing the old password

@EvanDotPro
Owner

Trying to clean up some of the old PR's/issues that have no activity. This looks like it needs more discussion and possibly refactored. Please feel free to re-open when it's ready.

@EvanDotPro EvanDotPro closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 8 additions and 5 deletions.
  1. +8 −5 src/ZfcUser/Service/User.php
View
13 src/ZfcUser/Service/User.php
@@ -97,15 +97,18 @@ public function register(array $data)
public function changePassword(array $data)
{
$currentUser = $this->getAuthService()->getIdentity();
+ $bcrypt = new Bcrypt;
- $oldPass = $data['credential'];
$newPass = $data['newCredential'];
- $bcrypt = new Bcrypt;
- $bcrypt->setCost($this->getOptions()->getPasswordCost());
+ if (array_key_exists('credential', $data)) {
+ $oldPass = $data['credential'];
- if (!$bcrypt->verify($oldPass, $currentUser->getPassword())) {
- return false;
+ $bcrypt->setCost($this->getOptions()->getPasswordCost());
+
+ if (!$bcrypt->verify($oldPass, $currentUser->getPassword())) {
+ return false;
+ }
}
$pass = $bcrypt->create($newPass);
Something went wrong with that request. Please try again.