diff --git a/api/controllers/AccountsController.php b/api/controllers/AccountsController.php index 6d76468..1699e33 100644 --- a/api/controllers/AccountsController.php +++ b/api/controllers/AccountsController.php @@ -1,6 +1,7 @@ AccessControl::class, 'rules' => [ [ - 'actions' => ['current'], + 'actions' => ['current', 'change-password'], 'allow' => true, 'roles' => ['@'], ], @@ -26,6 +27,7 @@ class AccountsController extends Controller { public function verbs() { return [ 'current' => ['GET'], + 'change-password' => ['POST'], ]; } @@ -42,4 +44,21 @@ class AccountsController extends Controller { ]; } + public function actionChangePassword() { + /** @var Account $account */ + $account = Yii::$app->user->identity; + $model = new ChangePasswordForm($account); + $model->load(Yii::$app->request->post()); + if (!$model->changePassword()) { + return [ + 'success' => false, + 'errors' => $this->normalizeModelErrors($model->getErrors()), + ]; + } + + return [ + 'success' => true, + ]; + } + } diff --git a/api/models/ChangePasswordForm.php b/api/models/ChangePasswordForm.php new file mode 100644 index 0000000..33b6c70 --- /dev/null +++ b/api/models/ChangePasswordForm.php @@ -0,0 +1,71 @@ +_account = $account; + parent::__construct($config); + } + + /** + * @inheritdoc + */ + public function rules() { + return [ + ['password', 'required', 'message' => 'error.password_required'], + ['newPassword', 'required', 'message' => 'error.newPassword_required'], + ['newRePassword', 'required', 'message' => 'error.newRePassword_required'], + ['password', 'validatePassword'], + ['newPassword', 'string', 'min' => 8, 'tooShort' => 'error.password_too_short'], + ['newRePassword', 'validatePasswordAndRePasswordMatch'], + ]; + } + + public function validatePassword($attribute) { + if (!$this->hasErrors() && !$this->_account->validatePassword($this->$attribute)) { + $this->addError($attribute, 'error.' . $attribute . '_incorrect'); + } + } + + public function validatePasswordAndRePasswordMatch($attribute) { + if (!$this->hasErrors()) { + if ($this->newPassword !== $this->newRePassword) { + $this->addError($attribute, 'error.newRePassword_does_not_match'); + } + } + } + + /** + * @return boolean if password was changed. + */ + public function changePassword() { + if (!$this->validate()) { + return false; + } + + $account = $this->_account; + $account->setPassword($this->newPassword); + + return $account->save(); + } + +} diff --git a/common/models/Account.php b/common/models/Account.php index b043ab6..fa40f99 100644 --- a/common/models/Account.php +++ b/common/models/Account.php @@ -66,14 +66,6 @@ class Account extends ActiveRecord implements IdentityInterface { return static::findOne(['id' => $id]); } - /** - * @param string $email - * @return static|null - */ - public static function findByEmail($email) { - return static::findOne(['email' => $email]); - } - /** * Finds user by password reset token * @@ -167,20 +159,8 @@ class Account extends ActiveRecord implements IdentityInterface { * @throws InvalidConfigException */ public function setPassword($password) { - switch($this->password_hash_strategy) { - case self::PASS_HASH_STRATEGY_OLD_ELY: - $password = UserPass::make($this->email, $password); - break; - - case self::PASS_HASH_STRATEGY_YII2: - $password = Yii::$app->security->generatePasswordHash($password); - break; - - default: - throw new InvalidConfigException('You must specify password_hash_strategy before you can set password'); - } - - $this->password_hash = $password; + $this->password_hash_strategy = self::PASS_HASH_STRATEGY_YII2; + $this->password_hash = Yii::$app->security->generatePasswordHash($password); } /** diff --git a/tests/codeception/api/_pages/AccountsRoute.php b/tests/codeception/api/_pages/AccountsRoute.php index de128d2..93b065d 100644 --- a/tests/codeception/api/_pages/AccountsRoute.php +++ b/tests/codeception/api/_pages/AccountsRoute.php @@ -9,8 +9,17 @@ use yii\codeception\BasePage; class AccountsRoute extends BasePage { public function current() { - $this->route = ['users/current']; + $this->route = ['accounts/current']; $this->actor->sendGET($this->getUrl()); } + public function changePassword($currentPassword = null, $newPassword = null, $newRePassword = null) { + $this->route = ['accounts/change-password']; + $this->actor->sendPOST($this->getUrl(), [ + 'password' => $currentPassword, + 'newPassword' => $newPassword, + 'newRePassword' => $newRePassword, + ]); + } + } diff --git a/tests/codeception/api/functional/AccountsChangePasswordCest.php b/tests/codeception/api/functional/AccountsChangePasswordCest.php new file mode 100644 index 0000000..3c68095 --- /dev/null +++ b/tests/codeception/api/functional/AccountsChangePasswordCest.php @@ -0,0 +1,44 @@ +route = new AccountsRoute($I); + } + + public function testChangePassword(FunctionalTester $I, Scenario $scenario) { + $I->wantTo('change my password'); + $I = new AccountSteps($scenario); + $I->loggedInAsActiveAccount(); + + $this->route->changePassword('password_0', 'new-password', 'new-password'); + $I->canSeeResponseCodeIs(200); + $I->canSeeResponseIsJson(); + $I->canSeeResponseContainsJson([ + 'success' => true, + ]); + + $I->notLoggedIn(); + + $loginRoute = new LoginRoute($I); + $loginRoute->login('Admin', 'new-password'); + $I->canSeeResponseCodeIs(200); + $I->canSeeResponseContainsJson([ + 'success' => true, + ]); + } + +} diff --git a/tests/codeception/api/functional/_steps/AccountSteps.php b/tests/codeception/api/functional/_steps/AccountSteps.php index fc102b7..ec25886 100644 --- a/tests/codeception/api/functional/_steps/AccountSteps.php +++ b/tests/codeception/api/functional/_steps/AccountSteps.php @@ -16,4 +16,8 @@ class AccountSteps extends FunctionalTester { $I->amBearerAuthenticated($jwt); } + public function notLoggedIn() { + $this->haveHttpHeader('Authorization', null); + } + } diff --git a/tests/codeception/api/unit/models/ChangePasswordFormTest.php b/tests/codeception/api/unit/models/ChangePasswordFormTest.php new file mode 100644 index 0000000..bf777b7 --- /dev/null +++ b/tests/codeception/api/unit/models/ChangePasswordFormTest.php @@ -0,0 +1,108 @@ + [ + 'class' => AccountFixture::class, + 'dataFile' => '@tests/codeception/common/fixtures/data/accounts.php', + ], + ]; + } + + public function testChangePasswordErrors() { + /** @var Account $account */ + $account = Account::findOne($this->accounts['admin']['id']); + $model = new ChangePasswordForm($account); + $this->specify('expected error.{field}_required if we don\'t pass some fields', function() use ($model, $account) { + expect('form should return false', $model->changePassword())->false(); + expect('form should contain errors', $model->getErrors())->equals([ + 'password' => ['error.password_required'], + 'newPassword' => ['error.newPassword_required'], + 'newRePassword' => ['error.newRePassword_required'], + ]); + expect('password not changed', $account->validatePassword('password_0'))->true(); + }); + + $model = new ChangePasswordForm($account, [ + 'password' => 'this-is-wrong-password', + 'newPassword' => 'my-new-password', + 'newRePassword' => 'my-new-password', + ]); + $this->specify('expected error.password_incorrect if we pass invalid current account password', function() use ($model, $account) { + expect('form should return false', $model->changePassword())->false(); + expect('form should contain errors', $model->getErrors())->equals([ + 'password' => ['error.password_incorrect'], + ]); + expect('password not changed', $account->validatePassword('password_0'))->true(); + }); + + $model = new ChangePasswordForm($account, [ + 'password' => 'password_0', + 'newPassword' => 'short', + 'newRePassword' => 'short', + ]); + $this->specify('expected error.password_too_short if we pass short password', function() use ($model, $account) { + expect('form should return false', $model->changePassword())->false(); + expect('form should contain errors', $model->getErrors())->equals([ + 'newPassword' => ['error.password_too_short'], + ]); + expect('password not changed', $account->validatePassword('password_0'))->true(); + }); + + $model = new ChangePasswordForm($account, [ + 'password' => 'password_0', + 'newPassword' => 'first-valid-pass', + 'newRePassword' => 'another-valid-pass', + ]); + $this->specify('expected error.newRePassword_does_not_match if we passwords mismatch', function() use ($model, $account) { + expect('form should return false', $model->changePassword())->false(); + expect('form should contain errors', $model->getErrors())->equals([ + 'newRePassword' => ['error.newRePassword_does_not_match'], + ]); + expect('password not changed', $account->validatePassword('password_0'))->true(); + }); + } + + public function testChangePassword() { + /** @var Account $account */ + $account = Account::findOne($this->accounts['admin']['id']); + $model = new ChangePasswordForm($account, [ + 'password' => 'password_0', + 'newPassword' => 'my-new-password', + 'newRePassword' => 'my-new-password', + ]); + $this->specify('successfully change password with modern hash strategy', function() use ($model, $account) { + expect('form should return true', $model->changePassword())->true(); + expect('new password should be successfully stored into account', $account->validatePassword('my-new-password'))->true(); + expect('always use new strategy', $account->password_hash_strategy)->equals(Account::PASS_HASH_STRATEGY_YII2); + }); + + /** @var Account $account */ + $account = Account::findOne($this->accounts['user-with-old-password-type']['id']); + $model = new ChangePasswordForm($account, [ + 'password' => '12345678', + 'newPassword' => 'my-new-password', + 'newRePassword' => 'my-new-password', + ]); + $this->specify('successfully change password with legacy hash strategy', function() use ($model, $account) { + expect('form should return true', $model->changePassword())->true(); + expect('new password should be successfully stored into account', $account->validatePassword('my-new-password'))->true(); + expect('strategy should be changed to modern', $account->password_hash_strategy)->equals(Account::PASS_HASH_STRATEGY_YII2); + }); + } + +}