Cleanup code, improve typings

This commit is contained in:
ErickSkrauch
2019-12-13 22:27:13 +03:00
parent 830a17612b
commit d9ef27b745
28 changed files with 189 additions and 225 deletions

View File

@@ -107,7 +107,7 @@ class AuthenticationController extends Controller {
],
];
if ($model->getLoginAttribute() !== 'email') {
if (strpos($model->login, '@') === false) {
$response['data']['emailMask'] = StringHelper::getEmailMask($model->getAccount()->email);
}

View File

@@ -1,10 +1,11 @@
<?php
declare(strict_types=1);
namespace api\models\authentication;
use api\aop\annotations\CollectModelMetrics;
use api\components\ReCaptcha\Validator as ReCaptchaValidator;
use api\models\base\ApiForm;
use api\traits\AccountFinder;
use common\components\UserFriendlyRandomKey;
use common\helpers\Error as E;
use common\models\Account;
@@ -15,13 +16,12 @@ use Yii;
use yii\base\ErrorException;
class ForgotPasswordForm extends ApiForm {
use AccountFinder;
public $captcha;
public $login;
public function rules() {
public function rules(): array {
return [
['captcha', ReCaptchaValidator::class],
['login', 'required', 'message' => E::LOGIN_REQUIRED],
@@ -31,7 +31,7 @@ class ForgotPasswordForm extends ApiForm {
];
}
public function validateLogin($attribute) {
public function validateLogin(string $attribute): void {
if (!$this->hasErrors()) {
if ($this->getAccount() === null) {
$this->addError($attribute, E::LOGIN_NOT_EXIST);
@@ -39,7 +39,7 @@ class ForgotPasswordForm extends ApiForm {
}
}
public function validateActivity($attribute) {
public function validateActivity(string $attribute): void {
if (!$this->hasErrors()) {
$account = $this->getAccount();
if ($account->status !== Account::STATUS_ACTIVE) {
@@ -48,7 +48,7 @@ class ForgotPasswordForm extends ApiForm {
}
}
public function validateFrequency($attribute) {
public function validateFrequency(string $attribute): void {
if (!$this->hasErrors()) {
$emailConfirmation = $this->getEmailActivation();
if ($emailConfirmation !== null && !$emailConfirmation->canRepeat()) {
@@ -57,12 +57,15 @@ class ForgotPasswordForm extends ApiForm {
}
}
public function getAccount(): ?Account {
return Account::find()->andWhereLogin($this->login)->one();
}
/**
* @CollectModelMetrics(prefix="authentication.forgotPassword")
* @return bool
* @throws ErrorException
*/
public function forgotPassword() {
public function forgotPassword(): bool {
if (!$this->validate()) {
return false;
}
@@ -86,23 +89,13 @@ class ForgotPasswordForm extends ApiForm {
return true;
}
public function getLogin(): string {
return $this->login;
}
/**
* @return EmailActivation|null
* @throws ErrorException
*/
public function getEmailActivation() {
public function getEmailActivation(): ?EmailActivation {
$account = $this->getAccount();
if ($account === null) {
throw new ErrorException('Account not founded');
return null;
}
return $account->getEmailActivations()
->andWhere(['type' => EmailActivation::TYPE_FORGOT_PASSWORD_KEY])
->one();
return $account->getEmailActivations()->withType(EmailActivation::TYPE_FORGOT_PASSWORD_KEY)->one();
}
}

View File

@@ -5,7 +5,6 @@ namespace api\models\authentication;
use api\aop\annotations\CollectModelMetrics;
use api\models\base\ApiForm;
use api\traits\AccountFinder;
use api\validators\TotpValidator;
use common\helpers\Error as E;
use common\models\Account;
@@ -14,14 +13,25 @@ use Webmozart\Assert\Assert;
use Yii;
class LoginForm extends ApiForm {
use AccountFinder;
/**
* @var string
*/
public $login;
/**
* @var string
*/
public $password;
/**
* @var string|null
*/
public $totp;
/**
* @var bool
*/
public $rememberMe = false;
public function rules(): array {
@@ -90,8 +100,8 @@ class LoginForm extends ApiForm {
}
}
public function getLogin(): string {
return $this->login;
public function getAccount(): ?Account {
return Account::find()->andWhereLogin($this->login)->one();
}
/**

View File

@@ -1,4 +1,6 @@
<?php
declare(strict_types=1);
namespace api\models\authentication;
use api\aop\annotations\CollectModelMetrics;
@@ -21,7 +23,7 @@ class RepeatAccountActivationForm extends ApiForm {
private $emailActivation;
public function rules() {
public function rules(): array {
return [
['captcha', ReCaptchaValidator::class],
['email', 'filter', 'filter' => 'trim'],
@@ -31,7 +33,7 @@ class RepeatAccountActivationForm extends ApiForm {
];
}
public function validateEmailForAccount($attribute) {
public function validateEmailForAccount(string $attribute): void {
if (!$this->hasErrors()) {
$account = $this->getAccount();
if ($account === null) {
@@ -45,7 +47,7 @@ class RepeatAccountActivationForm extends ApiForm {
}
}
public function validateExistsActivation($attribute) {
public function validateExistsActivation(string $attribute): void {
if (!$this->hasErrors()) {
$activation = $this->getActivation();
if ($activation !== null && !$activation->canRepeat()) {
@@ -58,11 +60,12 @@ class RepeatAccountActivationForm extends ApiForm {
* @CollectModelMetrics(prefix="signup.repeatEmail")
* @return bool
*/
public function sendRepeatMessage() {
public function sendRepeatMessage(): bool {
if (!$this->validate()) {
return false;
}
/** @var Account $account */
$account = $this->getAccount();
$transaction = Yii::$app->db->beginTransaction();
@@ -85,27 +88,17 @@ class RepeatAccountActivationForm extends ApiForm {
return true;
}
/**
* @return Account|null
*/
public function getAccount() {
public function getAccount(): ?Account {
return Account::find()
->andWhere(['email' => $this->email])
->one();
}
/**
* @return EmailActivation|null
*/
public function getActivation() {
if ($this->emailActivation === null) {
$this->emailActivation = $this->getAccount()
->getEmailActivations()
->andWhere(['type' => EmailActivation::TYPE_REGISTRATION_EMAIL_CONFIRMATION])
->one();
}
return $this->emailActivation;
public function getActivation(): ?EmailActivation {
return $this->getAccount()
->getEmailActivations()
->withType(EmailActivation::TYPE_REGISTRATION_EMAIL_CONFIRMATION)
->one();
}
}

View File

@@ -1,4 +1,6 @@
<?php
declare(strict_types=1);
namespace api\modules\accounts\models;
use api\aop\annotations\CollectModelMetrics;
@@ -26,7 +28,7 @@ class SendEmailVerificationForm extends AccountActionForm {
];
}
public function validateFrequency($attribute): void {
public function validateFrequency(string $attribute): void {
if (!$this->hasErrors()) {
$emailConfirmation = $this->getEmailActivation();
if ($emailConfirmation !== null && !$emailConfirmation->canRepeat()) {
@@ -82,12 +84,10 @@ class SendEmailVerificationForm extends AccountActionForm {
public function getEmailActivation(): ?EmailActivation {
return $this->getAccount()
->getEmailActivations()
->andWhere([
'type' => [
EmailActivation::TYPE_CURRENT_EMAIL_CONFIRMATION,
EmailActivation::TYPE_NEW_EMAIL_CONFIRMATION,
],
])
->withType(
EmailActivation::TYPE_CURRENT_EMAIL_CONFIRMATION,
EmailActivation::TYPE_NEW_EMAIL_CONFIRMATION
)
->one();
}

View File

@@ -75,10 +75,7 @@ class AuthenticationForm extends ApiForm {
// The previous authorization server implementation used the nickname field instead of username,
// so we keep such behavior
$attribute = $loginForm->getLoginAttribute();
if ($attribute === 'username') {
$attribute = 'nickname';
}
$attribute = strpos($this->username, '@') === false ? 'nickname' : 'email';
// TODO: эта логика дублируется с логикой в SignoutForm

View File

@@ -47,10 +47,7 @@ class SignoutForm extends ApiForm {
// The previous authorization server implementation used the nickname field instead of username,
// so we keep such behavior
$attribute = $loginForm->getLoginAttribute();
if ($attribute === 'username') {
$attribute = 'nickname';
}
$attribute = strpos($this->username, '@') === false ? 'nickname' : 'email';
throw new ForbiddenOperationException("Invalid credentials. Invalid {$attribute} or password.");
}

View File

@@ -1,4 +1,6 @@
<?php
declare(strict_types=1);
namespace api\request;
use Yii;
@@ -18,7 +20,14 @@ use yii\web\RequestParserInterface;
*/
class RequestParser implements RequestParserInterface {
public function parse($rawBody, $contentType) {
/**
* @param string $rawBody
* @param string $contentType
*
* @return array
* @throws \yii\web\BadRequestHttpException
*/
public function parse($rawBody, $contentType): array {
if (!empty($_POST)) {
return $_POST;
}

View File

@@ -126,8 +126,8 @@ class ForgotPasswordFormTest extends TestCase {
return new class($params) extends ForgotPasswordForm {
public $key;
public function getEmailActivation() {
return EmailActivation::findOne($this->key);
public function getEmailActivation(): ?EmailActivation {
return EmailActivation::findOne(['key' => $this->key]);
}
};
}

View File

@@ -100,7 +100,7 @@ class RepeatAccountActivationFormTest extends TestCase {
return new class($params) extends RepeatAccountActivationForm {
public $emailKey;
public function getActivation() {
public function getActivation(): ?EmailActivation {
return EmailActivation::findOne($this->emailKey);
}
};

View File

@@ -29,7 +29,6 @@ class ChangeEmailFormTest extends TestCase {
'account_id' => $account->id,
'type' => EmailActivation::TYPE_NEW_EMAIL_CONFIRMATION,
]));
/** @noinspection UnserializeExploitsInspection */
$data = unserialize($newEmailConfirmationFixture['_data']);
$this->assertSame($data['newEmail'], $account->email);
}

View File

@@ -1,52 +0,0 @@
<?php
namespace api\tests\_support\traits;
use api\tests\unit\TestCase;
use api\traits\AccountFinder;
use common\models\Account;
use common\tests\fixtures\AccountFixture;
class AccountFinderTest extends TestCase {
public function _fixtures(): array {
return [
'accounts' => AccountFixture::class,
];
}
public function testGetAccount() {
$model = new AccountFinderTestTestClass();
/** @var Account $account */
$accountFixture = $this->tester->grabFixture('accounts', 'admin');
$model->login = $accountFixture->email;
$account = $model->getAccount();
$this->assertInstanceOf(Account::class, $account);
$this->assertSame($accountFixture->id, $account->id, 'founded account for passed login data');
$model = new AccountFinderTestTestClass();
$model->login = 'unexpected';
$this->assertNull($account = $model->getAccount(), 'null, if account can\'t be found');
}
public function testGetLoginAttribute() {
$model = new AccountFinderTestTestClass();
$model->login = 'erickskrauch@ely.by';
$this->assertSame('email', $model->getLoginAttribute(), 'if login look like email value, then \'email\'');
$model = new AccountFinderTestTestClass();
$model->login = 'erickskrauch';
$this->assertSame('username', $model->getLoginAttribute(), 'username in any other case');
}
}
class AccountFinderTestTestClass {
use AccountFinder;
public $login;
public function getLogin(): string {
return $this->login;
}
}

View File

@@ -1,24 +0,0 @@
<?php
namespace api\traits;
use common\models\Account;
trait AccountFinder {
private $account;
abstract public function getLogin(): string;
public function getAccount(): ?Account {
if ($this->account === null) {
$this->account = Account::findOne([$this->getLoginAttribute() => $this->getLogin()]);
}
return $this->account;
}
public function getLoginAttribute(): string {
return strpos($this->getLogin(), '@') ? 'email' : 'username';
}
}

View File

@@ -24,7 +24,7 @@ class EmailActivationKeyValidator extends Validator {
public $skipOnEmpty = false;
public function validateAttribute($model, $attribute) {
public function validateAttribute($model, $attribute): void {
$value = $model->$attribute;
if (empty($value)) {
$this->addError($model, $attribute, $this->keyRequired);
@@ -49,7 +49,7 @@ class EmailActivationKeyValidator extends Validator {
$query = EmailActivation::find();
$query->andWhere(['key' => $key]);
if ($type !== null) {
$query->andWhere(['type' => $type]);
$query->withType($type);
}
return $query->one();