Skip to content

Commit f92932b

Browse files
Alexander PaliarushOleksii Korshenko
authored andcommitted
MAGETWO-50608: [Github][Security] Able to brute force API token access
1 parent e2c9fc4 commit f92932b

File tree

4 files changed

+323
-36
lines changed

4 files changed

+323
-36
lines changed

dev/tests/api-functional/testsuite/Magento/Integration/Model/AdminTokenServiceTest.php

Lines changed: 144 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Magento\TestFramework\TestCase\WebapiAbstract;
1313
use Magento\User\Model\User as UserModel;
1414
use Magento\Framework\Webapi\Exception as HTTPExceptionCodes;
15+
use Magento\Integration\Model\Oauth\Token\RequestLog\Config as TokenThrottlerConfig;
1516

1617
/**
1718
* api-functional test for \Magento\Integration\Model\AdminTokenService.
@@ -21,7 +22,6 @@ class AdminTokenServiceTest extends WebapiAbstract
2122
const SERVICE_NAME = "integrationAdminTokenServiceV1";
2223
const SERVICE_VERSION = "V1";
2324
const RESOURCE_PATH_ADMIN_TOKEN = "/V1/integration/admin/token";
24-
const RESOURCE_PATH_CUSTOMER_TOKEN = "/V1/integration/customer/token";
2525

2626
/**
2727
* @var \Magento\Integration\Api\AdminTokenServiceInterface
@@ -38,6 +38,11 @@ class AdminTokenServiceTest extends WebapiAbstract
3838
*/
3939
private $userModel;
4040

41+
/**
42+
* @var int
43+
*/
44+
private $attemptsCountToLockAccount;
45+
4146
/**
4247
* Setup AdminTokenService
4348
*/
@@ -47,6 +52,9 @@ public function setUp()
4752
$this->tokenService = Bootstrap::getObjectManager()->get('Magento\Integration\Model\AdminTokenService');
4853
$this->tokenModel = Bootstrap::getObjectManager()->get('Magento\Integration\Model\Oauth\Token');
4954
$this->userModel = Bootstrap::getObjectManager()->get('Magento\User\Model\User');
55+
/** @var TokenThrottlerConfig $tokenThrottlerConfig */
56+
$tokenThrottlerConfig = Bootstrap::getObjectManager()->get(TokenThrottlerConfig::class);
57+
$this->attemptsCountToLockAccount = $tokenThrottlerConfig->getMaxFailuresCount();
5058
}
5159

5260
/**
@@ -67,20 +75,15 @@ public function testCreateAdminAccessToken()
6775
'password' => \Magento\TestFramework\Bootstrap::ADMIN_PASSWORD,
6876
];
6977
$accessToken = $this->_webApiCall($serviceInfo, $requestData);
70-
71-
$adminUserId = $this->userModel->loadByUsername($adminUserNameFromFixture)->getId();
72-
/** @var $token TokenModel */
73-
$token = $this->tokenModel
74-
->loadByAdminId($adminUserId)
75-
->getToken();
76-
$this->assertEquals($accessToken, $token);
78+
$this->assertToken($adminUserNameFromFixture, $accessToken);
7779
}
7880

7981
/**
8082
* @dataProvider validationDataProvider
8183
*/
8284
public function testCreateAdminAccessTokenEmptyOrNullCredentials()
8385
{
86+
$noExceptionOccurred = false;
8487
try {
8588
$serviceInfo = [
8689
'rest' => [
@@ -90,30 +93,36 @@ public function testCreateAdminAccessTokenEmptyOrNullCredentials()
9093
];
9194
$requestData = ['username' => '', 'password' => ''];
9295
$this->_webApiCall($serviceInfo, $requestData);
96+
$noExceptionOccurred = true;
9397
} catch (\Exception $e) {
9498
$this->assertInputExceptionMessages($e);
9599
}
100+
if ($noExceptionOccurred) {
101+
$this->fail("Exception was expected to be thrown when provided credentials are invalid.");
102+
}
96103
}
97104

98-
public function testCreateAdminAccessTokenInvalidCustomer()
105+
public function testCreateAdminAccessTokenInvalidCredentials()
99106
{
100107
$customerUserName = 'invalid';
101108
$password = 'invalid';
109+
$noExceptionOccurred = false;
102110
try {
103111
$serviceInfo = [
104112
'rest' => [
105-
'resourcePath' => self::RESOURCE_PATH_CUSTOMER_TOKEN,
113+
'resourcePath' => self::RESOURCE_PATH_ADMIN_TOKEN,
106114
'httpMethod' => \Magento\Framework\Webapi\Rest\Request::HTTP_METHOD_POST,
107115
],
108116
];
109117
$requestData = ['username' => $customerUserName, 'password' => $password];
110118
$this->_webApiCall($serviceInfo, $requestData);
119+
$noExceptionOccurred = true;
111120
} catch (\Exception $e) {
112-
$this->assertEquals(HTTPExceptionCodes::HTTP_UNAUTHORIZED, $e->getCode());
113-
$exceptionData = $this->processRestExceptionResult($e);
114-
$expectedExceptionData = ['message' => 'Invalid login or password.'];
121+
$this->assertInvalidCredentialsException($e);
122+
}
123+
if ($noExceptionOccurred) {
124+
$this->fail("Exception was expected to be thrown when provided credentials are invalid.");
115125
}
116-
$this->assertEquals($expectedExceptionData, $exceptionData);
117126
}
118127

119128
/**
@@ -129,6 +138,96 @@ public function validationDataProvider()
129138
];
130139
}
131140

141+
/**
142+
* @magentoApiDataFixture Magento/User/_files/user_with_role.php
143+
*/
144+
public function testThrottlingMaxAttempts()
145+
{
146+
$adminUserNameFromFixture = 'adminUser';
147+
148+
$serviceInfo = [
149+
'rest' => [
150+
'resourcePath' => self::RESOURCE_PATH_ADMIN_TOKEN,
151+
'httpMethod' => \Magento\Framework\Webapi\Rest\Request::HTTP_METHOD_POST,
152+
],
153+
];
154+
$invalidCredentials = [
155+
'username' => $adminUserNameFromFixture,
156+
'password' => 'invalid',
157+
];
158+
$validCredentials = [
159+
'username' => $adminUserNameFromFixture,
160+
'password' => \Magento\TestFramework\Bootstrap::ADMIN_PASSWORD,
161+
];
162+
163+
/* Try to get token using invalid credentials for 5 times (account is locked after 6 attempts) */
164+
$noExceptionOccurred = false;
165+
for ($i = 0; $i < ($this->attemptsCountToLockAccount - 1); $i++) {
166+
try {
167+
$this->_webApiCall($serviceInfo, $invalidCredentials);
168+
$noExceptionOccurred = true;
169+
} catch (\Exception $e) {
170+
}
171+
}
172+
if ($noExceptionOccurred) {
173+
$this->fail(
174+
"Precondition failed: exception should have occurred when token was requested with invalid credentials."
175+
);
176+
}
177+
178+
/** On 6th attempt it still should be possible to get token if valid credentials are specified */
179+
$accessToken = $this->_webApiCall($serviceInfo, $validCredentials);
180+
$this->assertToken($adminUserNameFromFixture, $accessToken);
181+
}
182+
183+
/**
184+
* @magentoApiDataFixture Magento/User/_files/user_with_role.php
185+
*/
186+
public function testThrottlingAccountLockout()
187+
{
188+
$adminUserNameFromFixture = 'adminUser';
189+
190+
$serviceInfo = [
191+
'rest' => [
192+
'resourcePath' => self::RESOURCE_PATH_ADMIN_TOKEN,
193+
'httpMethod' => \Magento\Framework\Webapi\Rest\Request::HTTP_METHOD_POST,
194+
],
195+
];
196+
$invalidCredentials = [
197+
'username' => $adminUserNameFromFixture,
198+
'password' => 'invalid',
199+
];
200+
$validCredentials = [
201+
'username' => $adminUserNameFromFixture,
202+
'password' => \Magento\TestFramework\Bootstrap::ADMIN_PASSWORD,
203+
];
204+
205+
/* Try to get token using invalid credentials for 5 times (account would be locked after 6 attempts) */
206+
$noExceptionOccurred = false;
207+
for ($i = 0; $i < $this->attemptsCountToLockAccount; $i++) {
208+
try {
209+
$this->_webApiCall($serviceInfo, $invalidCredentials);
210+
$noExceptionOccurred = true;
211+
} catch (\Exception $e) {
212+
$this->assertInvalidCredentialsException($e);
213+
}
214+
if ($noExceptionOccurred) {
215+
$this->fail("Exception was expected to be thrown when provided credentials are invalid.");
216+
}
217+
}
218+
219+
$noExceptionOccurred = false;
220+
try {
221+
$this->_webApiCall($serviceInfo, $validCredentials);
222+
$noExceptionOccurred = true;
223+
} catch (\Exception $e) {
224+
$this->assertInvalidCredentialsException($e);
225+
}
226+
if ($noExceptionOccurred) {
227+
$this->fail("Exception was expected to be thrown because account should have been locked at this point.");
228+
}
229+
}
230+
132231
/**
133232
* Assert for presence of Input exception messages
134233
*
@@ -157,4 +256,35 @@ private function assertInputExceptionMessages($e)
157256
];
158257
$this->assertEquals($expectedExceptionData, $exceptionData);
159258
}
259+
260+
/**
261+
* Make sure that status code and message are correct in case of authentication failure.
262+
*
263+
* @param \Exception $e
264+
*/
265+
private function assertInvalidCredentialsException($e)
266+
{
267+
$this->assertEquals(HTTPExceptionCodes::HTTP_UNAUTHORIZED, $e->getCode(), "Response HTTP code is invalid.");
268+
$exceptionData = $this->processRestExceptionResult($e);
269+
$expectedExceptionData = [
270+
'message' => 'You did not sign in correctly or your account is temporarily disabled.'
271+
];
272+
$this->assertEquals($expectedExceptionData, $exceptionData, "Exception message is invalid.");
273+
}
274+
275+
/**
276+
* Make sure provided token is valid and belongs to the specified user.
277+
*
278+
* @param string $username
279+
* @param string $accessToken
280+
*/
281+
private function assertToken($username, $accessToken)
282+
{
283+
$adminUserId = $this->userModel->loadByUsername($username)->getId();
284+
/** @var $token TokenModel */
285+
$token = $this->tokenModel
286+
->loadByAdminId($adminUserId)
287+
->getToken();
288+
$this->assertEquals($accessToken, $token);
289+
}
160290
}

0 commit comments

Comments
 (0)