Skip to content

Commit 27d9984

Browse files
committed
fix: move secure cookie check from Security to CookieStore
1 parent 90edc48 commit 27d9984

3 files changed

Lines changed: 44 additions & 3 deletions

File tree

system/Cookie/CookieStore.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
use ArrayIterator;
1515
use CodeIgniter\Cookie\Exceptions\CookieException;
16+
use CodeIgniter\Security\Exceptions\SecurityException;
17+
use Config\Services;
1618
use Countable;
1719
use IteratorAggregate;
1820
use Traversable;
@@ -161,7 +163,13 @@ public function remove(string $name, string $prefix = '')
161163
*/
162164
public function dispatch(): void
163165
{
166+
$request = Services::request();
167+
164168
foreach ($this->cookies as $cookie) {
169+
if ($cookie->isSecure() && ! $request->isSecure()) {
170+
throw SecurityException::forDisallowedAction();
171+
}
172+
165173
$name = $cookie->getPrefixedName();
166174
$value = $cookie->getValue();
167175
$options = $cookie->getOptions();

system/Security/Security.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -538,9 +538,6 @@ private function saveHashInCookie(): void
538538
*/
539539
protected function sendCookie(RequestInterface $request)
540540
{
541-
if ($this->cookie->isSecure() && ! $request->isSecure()) {
542-
return false;
543-
}
544541

545542
$this->doSendCookie();
546543
log_message('info', 'CSRF cookie sent.');

tests/system/HTTP/ResponseSendTest.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111

1212
namespace CodeIgniter\HTTP;
1313

14+
use CodeIgniter\Security\Exceptions\SecurityException;
1415
use CodeIgniter\Test\CIUnitTestCase;
1516
use Config\App;
17+
use Config\Services;
1618

1719
/**
1820
* This test suite has been created separately from
@@ -135,4 +137,38 @@ public function testRedirectResponseCookies()
135137
$this->assertHeaderEmitted('Set-Cookie: foo=bar;');
136138
$this->assertHeaderEmitted('Set-Cookie: login_time');
137139
}
140+
141+
/**
142+
* Make sure secure cookies are not sent with HTTP request
143+
*
144+
* @ runInSeparateProcess
145+
* @ preserveGlobalState disabled
146+
*/
147+
public function testDoNotSendUnSecureCookie(): void
148+
{
149+
$this->expectException(SecurityException::class);
150+
$this->expectExceptionMessage('The action you requested is not allowed');
151+
152+
$request = $this->createMock(IncomingRequest::class);
153+
$request->method('isSecure')->willReturn(false);
154+
Services::injectMock('request', $request);
155+
156+
$response = new Response(new App());
157+
$response->pretend(false);
158+
$body = 'Hello';
159+
$response->setBody($body);
160+
161+
$response->setCookie(
162+
'foo',
163+
'bar',
164+
'',
165+
'',
166+
'/',
167+
'',
168+
true
169+
);
170+
171+
// send it
172+
$response->send();
173+
}
138174
}

0 commit comments

Comments
 (0)