diff --git a/depfile.yaml b/depfile.yaml index 2c87969f4fc4..301f17076f82 100644 --- a/depfile.yaml +++ b/depfile.yaml @@ -172,6 +172,7 @@ ruleset: HTTP: - Cookie - Files + - Security - URI Images: - Files diff --git a/system/Cookie/CookieStore.php b/system/Cookie/CookieStore.php index 87dc472d1fe5..af7dd2f0e437 100644 --- a/system/Cookie/CookieStore.php +++ b/system/Cookie/CookieStore.php @@ -158,6 +158,8 @@ public function remove(string $name, string $prefix = '') /** * Dispatches all cookies in store. + * + * @deprecated Response should dispatch cookies. */ public function dispatch(): void { @@ -232,6 +234,8 @@ protected function validateCookies(array $cookies): void * Extracted call to `setrawcookie()` in order to run unit tests on it. * * @codeCoverageIgnore + * + * @deprecated */ protected function setRawCookie(string $name, string $value, array $options): void { @@ -242,6 +246,8 @@ protected function setRawCookie(string $name, string $value, array $options): vo * Extracted call to `setcookie()` in order to run unit tests on it. * * @codeCoverageIgnore + * + * @deprecated */ protected function setCookie(string $name, string $value, array $options): void { diff --git a/system/HTTP/ResponseTrait.php b/system/HTTP/ResponseTrait.php index 7ec5f1e69e6b..278143fd3c3a 100644 --- a/system/HTTP/ResponseTrait.php +++ b/system/HTTP/ResponseTrait.php @@ -16,13 +16,14 @@ use CodeIgniter\Cookie\Exceptions\CookieException; use CodeIgniter\HTTP\Exceptions\HTTPException; use CodeIgniter\Pager\PagerInterface; +use CodeIgniter\Security\Exceptions\SecurityException; use Config\Services; use DateTime; use DateTimeZone; use InvalidArgumentException; /** - * Request Trait + * Response Trait * * Additional methods to make a PSR-7 Response class * compliant with the framework's own ResponseInterface. @@ -446,7 +447,7 @@ public function send() } /** - * Sends the headers of this HTTP request to the browser. + * Sends the headers of this HTTP response to the browser. * * @return Response */ @@ -535,15 +536,15 @@ public function redirect(string $uri, string $method = 'auto', ?int $code = null * Accepts an arbitrary number of binds (up to 7) or an associative * array in the first parameter containing all the values. * - * @param array|string $name Cookie name or array containing binds - * @param string $value Cookie value - * @param string $expire Cookie expiration time in seconds - * @param string $domain Cookie domain (e.g.: '.yourdomain.com') - * @param string $path Cookie path (default: '/') - * @param string $prefix Cookie name prefix - * @param bool $secure Whether to only transfer cookies via SSL - * @param bool $httponly Whether only make the cookie accessible via HTTP (no javascript) - * @param string|null $samesite + * @param array|Cookie|string $name Cookie name / array containing binds / Cookie object + * @param string $value Cookie value + * @param string $expire Cookie expiration time in seconds + * @param string $domain Cookie domain (e.g.: '.yourdomain.com') + * @param string $path Cookie path (default: '/') + * @param string $prefix Cookie name prefix + * @param bool $secure Whether to only transfer cookies via SSL + * @param bool $httponly Whether only make the cookie accessible via HTTP (no javascript) + * @param string|null $samesite * * @return $this */ @@ -558,6 +559,12 @@ public function setCookie( $httponly = false, $samesite = null ) { + if ($name instanceof Cookie) { + $this->cookieStore = $this->cookieStore->put($name); + + return $this; + } + if (is_array($name)) { // always leave 'name' in last place, as the loop will break otherwise, due to $$item foreach (['samesite', 'value', 'expire', 'domain', 'path', 'prefix', 'secure', 'httponly', 'name'] as $item) { @@ -689,7 +696,51 @@ protected function sendCookies() return; } - $this->cookieStore->dispatch(); + $this->dispatchCookies(); + } + + private function dispatchCookies(): void + { + /** @var IncomingRequest $request */ + $request = Services::request(); + + foreach ($this->cookieStore->display() as $cookie) { + if ($cookie->isSecure() && ! $request->isSecure()) { + throw SecurityException::forDisallowedAction(); + } + + $name = $cookie->getPrefixedName(); + $value = $cookie->getValue(); + $options = $cookie->getOptions(); + + if ($cookie->isRaw()) { + $this->doSetRawCookie($name, $value, $options); + } else { + $this->doSetCookie($name, $value, $options); + } + } + + $this->cookieStore->clear(); + } + + /** + * Extracted call to `setrawcookie()` in order to run unit tests on it. + * + * @codeCoverageIgnore + */ + private function doSetRawCookie(string $name, string $value, array $options): void + { + setrawcookie($name, $value, $options); + } + + /** + * Extracted call to `setcookie()` in order to run unit tests on it. + * + * @codeCoverageIgnore + */ + private function doSetCookie(string $name, string $value, array $options): void + { + setcookie($name, $value, $options); } /** diff --git a/system/Security/Security.php b/system/Security/Security.php index 35e889a85d69..008e94aeb612 100644 --- a/system/Security/Security.php +++ b/system/Security/Security.php @@ -13,6 +13,7 @@ use CodeIgniter\Cookie\Cookie; use CodeIgniter\HTTP\RequestInterface; +use CodeIgniter\HTTP\Response; use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Session\Session; use Config\App; @@ -528,13 +529,18 @@ private function saveHashInCookie(): void 'expires' => $this->expires === 0 ? 0 : time() + $this->expires, ] ); - $this->sendCookie($this->request); + + /** @var Response $response */ + $response = Services::response(); + $response->setCookie($this->cookie); } /** * CSRF Send Cookie * * @return false|Security + * + * @deprecated Set cookies to Response object instead. */ protected function sendCookie(RequestInterface $request) { @@ -553,6 +559,8 @@ protected function sendCookie(RequestInterface $request) * Extracted for this to be unit tested. * * @codeCoverageIgnore + * + * @deprecated Set cookies to Response object instead. */ protected function doSendCookie(): void { diff --git a/tests/system/CommonFunctionsTest.php b/tests/system/CommonFunctionsTest.php index ba4c1e7a4e07..c6f1d6b9b2e1 100644 --- a/tests/system/CommonFunctionsTest.php +++ b/tests/system/CommonFunctionsTest.php @@ -41,10 +41,10 @@ final class CommonFunctionsTest extends CIUnitTestCase { protected function setUp(): void { - parent::setUp(); - $renderer = Services::renderer(); - $renderer->resetData(); unset($_ENV['foo'], $_SERVER['foo']); + Services::reset(); + + parent::setUp(); } public function testStringifyAttributes() diff --git a/tests/system/HTTP/ResponseCookieTest.php b/tests/system/HTTP/ResponseCookieTest.php index 721a2fbd47b7..678928463cc4 100644 --- a/tests/system/HTTP/ResponseCookieTest.php +++ b/tests/system/HTTP/ResponseCookieTest.php @@ -65,6 +65,17 @@ public function testCookiesAll() $this->assertTrue($response->hasCookie('bee')); } + public function testSetCookieObject() + { + $cookie = new Cookie('foo', 'bar'); + $response = new Response(new App()); + + $response->setCookie($cookie); + + $this->assertCount(1, $response->getCookies()); + $this->assertTrue($response->hasCookie('foo')); + } + public function testCookieGet() { $response = new Response(new App()); diff --git a/tests/system/HTTP/ResponseSendTest.php b/tests/system/HTTP/ResponseSendTest.php index 5fedc69da7b1..6610321040a2 100644 --- a/tests/system/HTTP/ResponseSendTest.php +++ b/tests/system/HTTP/ResponseSendTest.php @@ -11,8 +11,10 @@ namespace CodeIgniter\HTTP; +use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Test\CIUnitTestCase; use Config\App; +use Config\Services; /** * This test suite has been created separately from @@ -135,4 +137,38 @@ public function testRedirectResponseCookies() $this->assertHeaderEmitted('Set-Cookie: foo=bar;'); $this->assertHeaderEmitted('Set-Cookie: login_time'); } + + /** + * Make sure secure cookies are not sent with HTTP request + * + * @ runInSeparateProcess + * @ preserveGlobalState disabled + */ + public function testDoNotSendUnSecureCookie(): void + { + $this->expectException(SecurityException::class); + $this->expectExceptionMessage('The action you requested is not allowed'); + + $request = $this->createMock(IncomingRequest::class); + $request->method('isSecure')->willReturn(false); + Services::injectMock('request', $request); + + $response = new Response(new App()); + $response->pretend(false); + $body = 'Hello'; + $response->setBody($body); + + $response->setCookie( + 'foo', + 'bar', + '', + '', + '/', + '', + true + ); + + // send it + $response->send(); + } } diff --git a/tests/system/HTTP/ResponseTest.php b/tests/system/HTTP/ResponseTest.php index b4eba4c547c9..a94cd8e8b2b5 100644 --- a/tests/system/HTTP/ResponseTest.php +++ b/tests/system/HTTP/ResponseTest.php @@ -30,14 +30,18 @@ final class ResponseTest extends CIUnitTestCase protected function setUp(): void { - parent::setUp(); $this->server = $_SERVER; + + Services::reset(); + + parent::setUp(); } protected function tearDown(): void { - $_SERVER = $this->server; Factories::reset('config'); + + $_SERVER = $this->server; } public function testCanSetStatusCode() diff --git a/tests/system/Security/SecurityTest.php b/tests/system/Security/SecurityTest.php index cc9fcf753853..160e35b17cb6 100644 --- a/tests/system/Security/SecurityTest.php +++ b/tests/system/Security/SecurityTest.php @@ -20,8 +20,8 @@ use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockAppConfig; use CodeIgniter\Test\Mock\MockSecurity; -use Config\Cookie as CookieConfig; use Config\Security as SecurityConfig; +use Config\Services; /** * @backupGlobals enabled @@ -41,11 +41,7 @@ protected function setUp(): void public function testBasicConfigIsSaved() { - $config = new MockAppConfig(); - $security = $this->getMockBuilder(Security::class) - ->setConstructorArgs([$config]) - ->onlyMethods(['doSendCookie']) - ->getMock(); + $security = new MockSecurity(new MockAppConfig()); $hash = $security->getHash(); @@ -57,13 +53,12 @@ public function testHashIsReadFromCookie() { $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005a'; - $config = new MockAppConfig(); - $security = $this->getMockBuilder(Security::class) - ->setConstructorArgs([$config]) - ->onlyMethods(['doSendCookie']) - ->getMock(); + $security = new MockSecurity(new MockAppConfig()); - $this->assertSame('8b9218a55906f9dcc1dc263dce7f005a', $security->getHash()); + $this->assertSame( + '8b9218a55906f9dcc1dc263dce7f005a', + $security->getHash() + ); } public function testGetHashSetsCookieWhenGETWithoutCSRFCookie() @@ -74,7 +69,8 @@ public function testGetHashSetsCookieWhenGETWithoutCSRFCookie() $security->verify(new Request(new MockAppConfig())); - $this->assertSame($_COOKIE['csrf_cookie_name'], $security->getHash()); + $cookie = Services::response()->getCookie('csrf_cookie_name'); + $this->assertSame($security->getHash(), $cookie->getValue()); } public function testGetHashReturnsCSRFCookieWhenGETWithCSRFCookie() @@ -96,7 +92,12 @@ public function testCSRFVerifyPostThrowsExceptionOnNoMatch() $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005b'; $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); $this->expectException(SecurityException::class); $security->verify($request); @@ -110,7 +111,12 @@ public function testCSRFVerifyPostReturnsSelfOnMatch() $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005a'; $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); $this->assertInstanceOf(Security::class, $security->verify($request)); $this->assertLogged('info', 'CSRF token verified.'); @@ -124,7 +130,12 @@ public function testCSRFVerifyHeaderThrowsExceptionOnNoMatch() $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005b'; $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); $request->setHeader('X-CSRF-TOKEN', '8b9218a55906f9dcc1dc263dce7f005a'); @@ -139,7 +150,12 @@ public function testCSRFVerifyHeaderReturnsSelfOnMatch() $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005a'; $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); $request->setHeader('X-CSRF-TOKEN', '8b9218a55906f9dcc1dc263dce7f005a'); @@ -155,9 +171,16 @@ public function testCSRFVerifyJsonThrowsExceptionOnNoMatch() $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005b'; $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); - $request->setBody('{"csrf_test_name":"8b9218a55906f9dcc1dc263dce7f005a"}'); + $request->setBody( + '{"csrf_test_name":"8b9218a55906f9dcc1dc263dce7f005a"}' + ); $this->expectException(SecurityException::class); $security->verify($request); @@ -169,9 +192,16 @@ public function testCSRFVerifyJsonReturnsSelfOnMatch() $_COOKIE['csrf_cookie_name'] = '8b9218a55906f9dcc1dc263dce7f005a'; $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); - $request->setBody('{"csrf_test_name":"8b9218a55906f9dcc1dc263dce7f005a","foo":"bar"}'); + $request->setBody( + '{"csrf_test_name":"8b9218a55906f9dcc1dc263dce7f005a","foo":"bar"}' + ); $this->assertInstanceOf(Security::class, $security->verify($request)); $this->assertLogged('info', 'CSRF token verified.'); @@ -199,7 +229,12 @@ public function testRegenerateWithFalseSecurityRegenerateProperty() Factories::injectMock('config', 'Security', $config); $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); $oldHash = $security->getHash(); $security->verify($request); @@ -219,7 +254,12 @@ public function testRegenerateWithTrueSecurityRegenerateProperty() Factories::injectMock('config', 'Security', $config); $security = new MockSecurity(new MockAppConfig()); - $request = new IncomingRequest(new MockAppConfig(), new URI('http://badurl.com'), null, new UserAgent()); + $request = new IncomingRequest( + new MockAppConfig(), + new URI('http://badurl.com'), + null, + new UserAgent() + ); $oldHash = $security->getHash(); $security->verify($request); @@ -238,45 +278,4 @@ public function testGetters(): void $this->assertIsString($security->getCookieName()); $this->assertIsBool($security->shouldRedirect()); } - - public function testSendingCookiesFalse(): void - { - $request = $this->createMock(IncomingRequest::class); - $request->method('isSecure')->willReturn(false); - - $config = new CookieConfig(); - - $config->secure = true; - Factories::injectMock('config', CookieConfig::class, $config); - - $security = $this->getMockBuilder(Security::class) - ->setConstructorArgs([new MockAppConfig()]) - ->onlyMethods(['doSendCookie']) - ->getMock(); - - $sendCookie = $this->getPrivateMethodInvoker($security, 'sendCookie'); - - $security->expects($this->never())->method('doSendCookie'); - $this->assertFalse($sendCookie($request)); - } - - public function testSendingGoodCookies(): void - { - $request = $this->createMock(IncomingRequest::class); - $request->method('isSecure')->willReturn(true); - - $config = new MockAppConfig(); - - $config->cookieSecure = true; - - $security = $this->getMockBuilder(Security::class) - ->setConstructorArgs([$config]) - ->onlyMethods(['doSendCookie']) - ->getMock(); - - $sendCookie = $this->getPrivateMethodInvoker($security, 'sendCookie'); - - $security->expects($this->once())->method('doSendCookie'); - $this->assertInstanceOf(Security::class, $sendCookie($request)); - } } diff --git a/user_guide_src/source/changelogs/v4.1.6.rst b/user_guide_src/source/changelogs/v4.1.6.rst index 4cddcc21f953..82b03127cda2 100644 --- a/user_guide_src/source/changelogs/v4.1.6.rst +++ b/user_guide_src/source/changelogs/v4.1.6.rst @@ -10,13 +10,15 @@ Release Date: Not released :depth: 2 BREAKING -======== +******** + - Multiple table names will no longer be stored in ``BaseBuilder::$tableName`` - an empty string will be used instead. .. _changelog-v416-validation-changes: Validation changes ------------------- +================== + - The previous version of the Validation can't handle an array item. Because of the bug fix, the validation results may be different, or raise a ``TypeError``. @@ -28,14 +30,29 @@ Validation changes On the other hand, the current version passes the array to the validation rule as a whole. Enhancements -============ +************ + - Database pane on debug toolbar now displays location where Query was called from. Also displays full backtrace. Changes -======= +******* + +- The process of sending cookies has been moved to the ``Response`` class. Now the ``Security`` and ``CookieStore`` class don't send cookies, set them to the Response. Deprecations -============ +************ + +Sending Cookies +=============== + +The process of sending cookies has been moved to the ``Response`` class. +And the following methods are deprecated: + +- ``CookieStore::dispatch()`` +- ``CookieStore::setRawCookie()`` +- ``CookieStore::setCookie()`` +- ``Security::sendCookie()`` +- ``Security::doSendCookie()`` Bugs Fixed -========== +********** diff --git a/user_guide_src/source/outgoing/response.rst b/user_guide_src/source/outgoing/response.rst index 060e2cd28291..2cd619dbd390 100644 --- a/user_guide_src/source/outgoing/response.rst +++ b/user_guide_src/source/outgoing/response.rst @@ -396,7 +396,7 @@ The methods provided by the parent class that are available are: .. php:method:: setCookie($name = ''[, $value = ''[, $expire = ''[, $domain = ''[, $path = '/'[, $prefix = ''[, $secure = false[, $httponly = false[, $samesite = null]]]]]]]]) - :param mixed $name: Cookie name or an array of parameters + :param array|Cookie|string $name: Cookie name or an array of parameters or an instance of ``CodeIgniter\Cookie\Cookie`` :param string $value: Cookie value :param int $expire: Cookie expiration time in seconds :param string $domain: Cookie domain