From 549db30b2be0d9dfc664af82d734cdfc57b97b9c Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Tue, 8 Aug 2017 20:18:44 +0300 Subject: [PATCH] =?UTF-8?q?=D0=9E=D0=B1=D0=BD=D0=BE=D0=B2=D0=BB=D1=91?= =?UTF-8?q?=D0=BD=20Spomky-Labs/otphp=20=D0=B4=D0=BE=209.0.2=20=D0=B2?= =?UTF-8?q?=D0=B5=D1=80=D1=81=D0=B8=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/models/profile/TwoFactorAuthForm.php | 8 +++++--- api/validators/TotpValidator.php | 9 +++++++-- composer.json | 5 +++-- tests/codeception/api/functional/ForgotPasswordCest.php | 2 +- tests/codeception/api/functional/LoginCest.php | 2 +- .../api/functional/TwoFactorAuthDisableCest.php | 2 +- .../api/functional/TwoFactorAuthEnableCest.php | 2 +- .../models/authentication/ForgotPasswordFormTest.php | 2 +- .../api/unit/models/authentication/LoginFormTest.php | 4 ++-- .../api/unit/models/profile/TwoFactorAuthFormTest.php | 2 ++ .../api/unit/validators/TotpValidatorTest.php | 4 ++-- tests/codeception/common/fixtures/data/accounts.php | 4 ++-- 12 files changed, 28 insertions(+), 18 deletions(-) diff --git a/api/models/profile/TwoFactorAuthForm.php b/api/models/profile/TwoFactorAuthForm.php index df3dcc6..8f6c31f 100644 --- a/api/models/profile/TwoFactorAuthForm.php +++ b/api/models/profile/TwoFactorAuthForm.php @@ -9,11 +9,11 @@ use BaconQrCode\Encoder\Encoder; use BaconQrCode\Renderer\Color\Rgb; use BaconQrCode\Renderer\Image\Svg; use BaconQrCode\Writer; -use Base32\Base32; use common\components\Qr\ElyDecorator; use common\helpers\Error as E; use common\models\Account; use OTPHP\TOTP; +use ParagonIE\ConstantTime\Encoding; use Yii; use yii\base\ErrorException; @@ -124,7 +124,8 @@ class TwoFactorAuthForm extends ApiForm { * @return TOTP */ public function getTotp(): TOTP { - $totp = new TOTP($this->account->email, $this->account->otp_secret); + $totp = TOTP::create($this->account->otp_secret); + $totp->setLabel($this->account->email); $totp->setIssuer('Ely.by'); return $totp; @@ -154,7 +155,8 @@ class TwoFactorAuthForm extends ApiForm { */ protected function setOtpSecret(int $length = 24): void { $randomBytesLength = ceil($length / 1.6); - $this->account->otp_secret = substr(trim(Base32::encode(random_bytes($randomBytesLength)), '='), 0, $length); + $randomBase32 = trim(Encoding::base32EncodeUpper(random_bytes($randomBytesLength)), '='); + $this->account->otp_secret = substr($randomBase32, 0, $length); if (!$this->account->save()) { throw new ErrorException('Cannot set account otp_secret'); } diff --git a/api/validators/TotpValidator.php b/api/validators/TotpValidator.php index e436d0c..68bbc5e 100644 --- a/api/validators/TotpValidator.php +++ b/api/validators/TotpValidator.php @@ -4,6 +4,7 @@ namespace api\validators; use common\helpers\Error as E; use common\models\Account; use OTPHP\TOTP; +use RangeException; use Yii; use yii\base\InvalidConfigException; use yii\validators\Validator; @@ -48,8 +49,12 @@ class TotpValidator extends Validator { } protected function validateValue($value) { - $totp = new TOTP(null, $this->account->otp_secret); - if (!$totp->verify((string)$value, $this->getTimestamp(), $this->window)) { + try { + $totp = TOTP::create($this->account->otp_secret); + if (!$totp->verify((string)$value, $this->getTimestamp(), $this->window)) { + return [E::OTP_TOKEN_INCORRECT, []]; + } + } catch (RangeException $e) { return [E::OTP_TOKEN_INCORRECT, []]; } diff --git a/composer.json b/composer.json index a67e4de..6dc66a0 100644 --- a/composer.json +++ b/composer.json @@ -20,9 +20,10 @@ "predis/predis": "^1.0", "mito/yii2-sentry": "^1.0", "minime/annotations": "~3.0", - "spomky-labs/otphp": "8.3.1", + "spomky-labs/otphp": "^9.0.2", "bacon/bacon-qr-code": "^1.0", - "roave/security-advisories": "dev-master" + "roave/security-advisories": "dev-master", + "paragonie/constant_time_encoding": "^2.0" }, "require-dev": { "yiisoft/yii2-codeception": "*", diff --git a/tests/codeception/api/functional/ForgotPasswordCest.php b/tests/codeception/api/functional/ForgotPasswordCest.php index 0444838..36023bd 100644 --- a/tests/codeception/api/functional/ForgotPasswordCest.php +++ b/tests/codeception/api/functional/ForgotPasswordCest.php @@ -74,7 +74,7 @@ class ForgotPasswordCest { public function testForgotPasswordByAccountWithOtp(FunctionalTester $I) { $I->wantTo('create new password recover request by passing username and otp token'); - $totp = new TOTP(null, 'secret-secret-secret'); + $totp = TOTP::create('BBBB'); $this->route->forgotPassword('AccountWithEnabledOtp', $totp->now()); $this->assertSuccessResponse($I, true); } diff --git a/tests/codeception/api/functional/LoginCest.php b/tests/codeception/api/functional/LoginCest.php index ab11f50..6f2adaf 100644 --- a/tests/codeception/api/functional/LoginCest.php +++ b/tests/codeception/api/functional/LoginCest.php @@ -206,7 +206,7 @@ class LoginCest { $route = new AuthenticationRoute($I); $I->wantTo('login into account with enabled otp'); - $route->login('AccountWithEnabledOtp', 'password_0', (new TOTP(null, 'secret-secret-secret'))->now()); + $route->login('AccountWithEnabledOtp', 'password_0', (TOTP::create('BBBB'))->now()); $I->canSeeResponseContainsJson([ 'success' => true, ]); diff --git a/tests/codeception/api/functional/TwoFactorAuthDisableCest.php b/tests/codeception/api/functional/TwoFactorAuthDisableCest.php index 5e41f83..1e288d1 100644 --- a/tests/codeception/api/functional/TwoFactorAuthDisableCest.php +++ b/tests/codeception/api/functional/TwoFactorAuthDisableCest.php @@ -49,7 +49,7 @@ class TwoFactorAuthDisableCest { public function testSuccessEnable(FunctionalTester $I) { $I->amAuthenticated('AccountWithEnabledOtp'); - $totp = new TOTP(null, 'secret-secret-secret'); + $totp = TOTP::create('BBBB'); $this->route->disable($totp->now(), 'password_0'); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); diff --git a/tests/codeception/api/functional/TwoFactorAuthEnableCest.php b/tests/codeception/api/functional/TwoFactorAuthEnableCest.php index aee002e..5fea216 100644 --- a/tests/codeception/api/functional/TwoFactorAuthEnableCest.php +++ b/tests/codeception/api/functional/TwoFactorAuthEnableCest.php @@ -49,7 +49,7 @@ class TwoFactorAuthEnableCest { public function testSuccessEnable(FunctionalTester $I) { $I->amAuthenticated('AccountWithOtpSecret'); - $totp = new TOTP(null, 'some otp secret value'); + $totp = TOTP::create('AAAA'); $this->route->enable($totp->now(), 'password_0'); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); diff --git a/tests/codeception/api/unit/models/authentication/ForgotPasswordFormTest.php b/tests/codeception/api/unit/models/authentication/ForgotPasswordFormTest.php index 4ae4cc9..e6d82c9 100644 --- a/tests/codeception/api/unit/models/authentication/ForgotPasswordFormTest.php +++ b/tests/codeception/api/unit/models/authentication/ForgotPasswordFormTest.php @@ -48,7 +48,7 @@ class ForgotPasswordFormTest extends TestCase { $model->validateTotpToken('token'); $this->assertEquals(['error.token_incorrect'], $model->getErrors('token')); - $totp = new TOTP(null, 'secret-secret-secret'); + $totp = TOTP::create('BBBB'); $model = new ForgotPasswordForm(); $model->login = 'AccountWithEnabledOtp'; $model->token = $totp->now(); diff --git a/tests/codeception/api/unit/models/authentication/LoginFormTest.php b/tests/codeception/api/unit/models/authentication/LoginFormTest.php index 0cf524e..1307eca 100644 --- a/tests/codeception/api/unit/models/authentication/LoginFormTest.php +++ b/tests/codeception/api/unit/models/authentication/LoginFormTest.php @@ -76,7 +76,7 @@ class LoginFormTest extends TestCase { $account = new AccountIdentity(['password' => '12345678']); $account->password = '12345678'; $account->is_otp_enabled = true; - $account->otp_secret = 'mock secret'; + $account->otp_secret = 'AAAA'; $this->specify('error.token_incorrect if totp invalid', function() use ($account) { $model = $this->createModel([ @@ -88,7 +88,7 @@ class LoginFormTest extends TestCase { $this->assertEquals(['error.token_incorrect'], $model->getErrors('token')); }); - $totp = new TOTP(null, 'mock secret'); + $totp = TOTP::create($account->otp_secret); $this->specify('no errors if password valid', function() use ($account, $totp) { $model = $this->createModel([ 'password' => '12345678', diff --git a/tests/codeception/api/unit/models/profile/TwoFactorAuthFormTest.php b/tests/codeception/api/unit/models/profile/TwoFactorAuthFormTest.php index 6e8837d..5157b05 100644 --- a/tests/codeception/api/unit/models/profile/TwoFactorAuthFormTest.php +++ b/tests/codeception/api/unit/models/profile/TwoFactorAuthFormTest.php @@ -197,10 +197,12 @@ class TwoFactorAuthFormTest extends TestCase { $model = new TwoFactorAuthForm($account); $this->callProtected($model, 'setOtpSecret'); $this->assertEquals(24, strlen($model->getAccount()->otp_secret)); + $this->assertSame(strtoupper($model->getAccount()->otp_secret), $model->getAccount()->otp_secret); $model = new TwoFactorAuthForm($account); $this->callProtected($model, 'setOtpSecret', 25); $this->assertEquals(25, strlen($model->getAccount()->otp_secret)); + $this->assertSame(strtoupper($model->getAccount()->otp_secret), $model->getAccount()->otp_secret); } } diff --git a/tests/codeception/api/unit/validators/TotpValidatorTest.php b/tests/codeception/api/unit/validators/TotpValidatorTest.php index dfc0bd3..1954009 100644 --- a/tests/codeception/api/unit/validators/TotpValidatorTest.php +++ b/tests/codeception/api/unit/validators/TotpValidatorTest.php @@ -13,8 +13,8 @@ class TotpValidatorTest extends TestCase { public function testValidateValue() { $account = new Account(); - $account->otp_secret = 'some secret'; - $controlTotp = new TOTP(null, $account->otp_secret); + $account->otp_secret = 'AAAA'; + $controlTotp = TOTP::create($account->otp_secret); $validator = new TotpValidator(['account' => $account]); diff --git a/tests/codeception/common/fixtures/data/accounts.php b/tests/codeception/common/fixtures/data/accounts.php index c8c79d3..fe15830 100644 --- a/tests/codeception/common/fixtures/data/accounts.php +++ b/tests/codeception/common/fixtures/data/accounts.php @@ -156,7 +156,7 @@ return [ 'lang' => 'ru', 'status' => \common\models\Account::STATUS_ACTIVE, 'rules_agreement_version' => \common\LATEST_RULES_VERSION, - 'otp_secret' => 'some otp secret value', + 'otp_secret' => 'AAAA', 'is_otp_enabled' => false, 'created_at' => 1485124615, 'updated_at' => 1485124615, @@ -171,7 +171,7 @@ return [ 'lang' => 'ru', 'status' => \common\models\Account::STATUS_ACTIVE, 'rules_agreement_version' => \common\LATEST_RULES_VERSION, - 'otp_secret' => 'secret-secret-secret', + 'otp_secret' => 'BBBB', 'is_otp_enabled' => true, 'created_at' => 1485124685, 'updated_at' => 1485124685,