diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 28308826e57..5c3601f39fe 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -5,6 +5,7 @@ Yii Framework 2 Change Log -------------------------- - Enh #13963: Added tests for yii\behaviors\TimestampBehavior (vladis84) +- Enh #13994: Refactored `yii\filters\RateLimiter`. Added tests (vladis84) - Enh #13820: Add new HTTP status code 451 (yyxx9988) - Bug #13671: Fixed error handler trace to work correctly with XDebug (samdark) - Bug #13657: Fixed `yii\helpers\StringHelper::truncateHtml()` skip extra tags at the end (sam002) diff --git a/framework/filters/RateLimiter.php b/framework/filters/RateLimiter.php index 798706171e4..4feaff20b49 100644 --- a/framework/filters/RateLimiter.php +++ b/framework/filters/RateLimiter.php @@ -62,25 +62,37 @@ class RateLimiter extends ActionFilter public $response; + /** + * @inheritdoc + */ + public function init() + { + if ($this->request === null) { + $this->request = Yii::$app->getRequest(); + } + if ($this->response === null) { + $this->response = Yii::$app->getResponse(); + } + } + /** * @inheritdoc */ public function beforeAction($action) { - $user = $this->user ? : (Yii::$app->getUser() ? Yii::$app->getUser()->getIdentity(false) : null); - if ($user instanceof RateLimitInterface) { + if ($this->user === null && Yii::$app->getUser()) { + $this->user = Yii::$app->getUser()->getIdentity(false); + } + + if ($this->user instanceof RateLimitInterface) { Yii::trace('Check rate limit', __METHOD__); - $this->checkRateLimit( - $user, - $this->request ? : Yii::$app->getRequest(), - $this->response ? : Yii::$app->getResponse(), - $action - ); - } elseif ($user) { + $this->checkRateLimit($this->user, $this->request, $this->response, $action); + } elseif ($this->user) { Yii::info('Rate limit skipped: "user" does not implement RateLimitInterface.', __METHOD__); } else { Yii::info('Rate limit skipped: user not logged in.', __METHOD__); } + return true; } diff --git a/tests/TestCase.php b/tests/TestCase.php index 829be9f9943..13aa1b83a61 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -148,7 +148,7 @@ protected function setInaccessibleProperty($object, $propertyName, $value, $revo } $property = $class->getProperty($propertyName); $property->setAccessible(true); - $property->setValue($value); + $property->setValue($object, $value); if ($revoke) { $property->setAccessible(false); } diff --git a/tests/framework/filters/RateLimiterTest.php b/tests/framework/filters/RateLimiterTest.php new file mode 100644 index 00000000000..e4ca51574dc --- /dev/null +++ b/tests/framework/filters/RateLimiterTest.php @@ -0,0 +1,155 @@ +prophesize(Logger::className()); + $logger + ->log(Argument::any(), Argument::any(), Argument::any()) + ->will(function ($parameters, $logger) { + $logger->messages = $parameters; + }); + + Yii::setLogger($logger->reveal()); + + $this->mockWebApplication(); + } + protected function tearDown() + { + parent::tearDown(); + Yii::setLogger(null); + } + + public function testInitFilledRequest() + { + $rateLimiter = new RateLimiter(['request' => 'Request']); + + $this->assertEquals('Request', $rateLimiter->request); + } + + public function testInitNotFilledRequest() + { + $rateLimiter = new RateLimiter(); + + $this->assertInstanceOf(Request::className(), $rateLimiter->request); + } + + public function testInitFilledResponse() + { + $rateLimiter = new RateLimiter(['response' => 'Response']); + + $this->assertEquals('Response', $rateLimiter->response); + } + + public function testInitNotFilledResponse() + { + $rateLimiter = new RateLimiter(); + + $this->assertInstanceOf(Response::className(), $rateLimiter->response); + } + + public function testBeforeActionUserInstanceOfRateLimitInterface() + { + $rateLimiter = new RateLimiter(); + $rateLimit = new RateLimit(); + $rateLimit->setAllowance([1, time()]) + ->setRateLimit([1, 1]); + $rateLimiter->user = $rateLimit; + + $result = $rateLimiter->beforeAction('test'); + + $this->assertContains('Check rate limit', Yii::getLogger()->messages); + $this->assertTrue($result); + } + + public function testBeforeActionUserNotInstanceOfRateLimitInterface() + { + $rateLimiter = new RateLimiter(['user' => 'User']); + + $result = $rateLimiter->beforeAction('test'); + + $this->assertContains('Rate limit skipped: "user" does not implement RateLimitInterface.', Yii::getLogger()->messages); + $this->assertTrue($result); + } + + public function testBeforeActionEmptyUser() + { + $user = new User(['identityClass' => RateLimit::className()]); + Yii::$app->set('user', $user); + $rateLimiter = new RateLimiter(); + + $result = $rateLimiter->beforeAction('test'); + + $this->assertContains('Rate limit skipped: user not logged in.', Yii::getLogger()->messages); + $this->assertTrue($result); + } + + public function testCheckRateLimitTooManyRequests() + { + /* @var $rateLimit UserIdentity|\Prophecy\ObjectProphecy */ + $rateLimit = new RateLimit; + $rateLimit + ->setRateLimit([1, 1]) + ->setAllowance([1, time() + 2]); + $rateLimiter = new RateLimiter(); + + $this->setExpectedException('yii\web\TooManyRequestsHttpException'); + $rateLimiter->checkRateLimit($rateLimit, Yii::$app->request, Yii::$app->response, 'testAction'); + } + + public function testCheckRateaddRateLimitHeaders() + { + /* @var $user UserIdentity|\Prophecy\ObjectProphecy */ + $rateLimit = new RateLimit; + $rateLimit + ->setRateLimit([1, 1]) + ->setAllowance([1, time()]); + $rateLimiter = $this->getMockBuilder(RateLimiter::className()) + ->setMethods(['addRateLimitHeaders']) + ->getMock(); + $rateLimiter->expects(self::at(0)) + ->method('addRateLimitHeaders') + ->willReturn(null); + + $rateLimiter->checkRateLimit($rateLimit, Yii::$app->request, Yii::$app->response, 'testAction'); + } + + public function testAddRateLimitHeadersDisabledRateLimitHeaders() + { + $rateLimiter = new RateLimiter(); + $rateLimiter->enableRateLimitHeaders = false; + $response = Yii::$app->response; + + $rateLimiter->addRateLimitHeaders($response, 1, 0, 0); + $this->assertCount(0, $response->getHeaders()); + } + + public function testAddRateLimitHeadersEnabledRateLimitHeaders() + { + $rateLimiter = new RateLimiter(); + $rateLimiter->enableRateLimitHeaders = true; + $response = Yii::$app->response; + + $rateLimiter->addRateLimitHeaders($response, 1, 0, 0); + $this->assertCount(3, $response->getHeaders()); + } +} diff --git a/tests/framework/filters/stubs/RateLimit.php b/tests/framework/filters/stubs/RateLimit.php new file mode 100644 index 00000000000..040fb83ae51 --- /dev/null +++ b/tests/framework/filters/stubs/RateLimit.php @@ -0,0 +1,44 @@ +_rateLimit; + } + + public function setRateLimit($rateLimit) + { + $this->_rateLimit = $rateLimit; + + return $this; + } + + public function loadAllowance($request, $action) + { + return $this->_allowance; + } + + public function setAllowance($allowance) + { + $this->_allowance = $allowance; + + return $this; + } + + + public function saveAllowance($request, $action, $allowance, $timestamp) + { + return [$action, $allowance, $timestamp]; + } + +}