Skip to content

Commit 52011ea

Browse files
paulbalandanpaulbalandan
committed
Merge remote-tracking branch 'upstream/develop' into 4.8
2 parents a7c9a2f + a87043b commit 52011ea

File tree

3 files changed

+170
-134
lines changed

3 files changed

+170
-134
lines changed

‎system/Security/Security.php‎

Lines changed: 87 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@
1818
use CodeIgniter\Exceptions\LogicException;
1919
use CodeIgniter\HTTP\IncomingRequest;
2020
use CodeIgniter\HTTP\Method;
21-
use CodeIgniter\HTTP\Request;
2221
use CodeIgniter\HTTP\RequestInterface;
2322
use CodeIgniter\I18n\Time;
2423
use CodeIgniter\Security\Exceptions\SecurityException;
2524
use CodeIgniter\Session\Session;
2625
use Config\Cookie as CookieConfig;
2726
use Config\Security as SecurityConfig;
2827
use ErrorException;
28+
use JsonException;
2929
use SensitiveParameter;
3030

3131
/**
@@ -233,32 +233,27 @@ private function configureCookie(CookieConfig $cookie): void
233233
Cookie::setDefaults($cookie);
234234
}
235235

236-
/**
237-
* CSRF verification.
238-
*
239-
* @return $this
240-
*
241-
* @throws SecurityException
242-
*/
243236
public function verify(RequestInterface $request)
244237
{
245-
// Protects POST, PUT, DELETE, PATCH
246-
$method = $request->getMethod();
247-
$methodsToProtect = [Method::POST, Method::PUT, Method::DELETE, Method::PATCH];
248-
if (! in_array($method, $methodsToProtect, true)) {
238+
$method = $request->getMethod();
239+
240+
// Protect POST, PUT, DELETE, PATCH requests only
241+
if (! in_array($method, [Method::POST, Method::PUT, Method::DELETE, Method::PATCH], true)) {
249242
return $this;
250243
}
251244

245+
assert($request instanceof IncomingRequest);
246+
252247
$postedToken = $this->getPostedToken($request);
253248

254249
try {
255-
$token = ($postedToken !== null && $this->config->tokenRandomize)
256-
? $this->derandomize($postedToken) : $postedToken;
250+
$token = $postedToken !== null && $this->config->tokenRandomize
251+
? $this->derandomize($postedToken)
252+
: $postedToken;
257253
} catch (InvalidArgumentException) {
258254
$token = null;
259255
}
260256

261-
// Do the tokens match?
262257
if (! isset($token, $this->hash) || ! hash_equals($this->hash, $token)) {
263258
throw SecurityException::forDisallowedAction();
264259
}
@@ -277,66 +272,107 @@ public function verify(RequestInterface $request)
277272
/**
278273
* Remove token in POST or JSON request data
279274
*/
280-
private function removeTokenInRequest(RequestInterface $request): void
275+
private function removeTokenInRequest(IncomingRequest $request): void
281276
{
282-
assert($request instanceof Request);
283-
284277
$superglobals = service('superglobals');
285-
if ($superglobals->post($this->config->tokenName) !== null) {
286-
// We kill this since we're done and we don't want to pollute the POST array.
287-
$superglobals->unsetPost($this->config->tokenName);
278+
$tokenName = $this->config->tokenName;
279+
280+
// If the token is found in POST data, we can safely remove it.
281+
if (is_string($superglobals->post($tokenName))) {
282+
$superglobals->unsetPost($tokenName);
288283
$request->setGlobal('post', $superglobals->getPostArray());
289-
} else {
290-
$body = $request->getBody() ?? '';
291-
$json = json_decode($body);
292-
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
293-
// We kill this since we're done and we don't want to pollute the JSON data.
294-
unset($json->{$this->config->tokenName});
295-
$request->setBody(json_encode($json));
296-
} else {
297-
parse_str($body, $parsed);
298-
// We kill this since we're done and we don't want to pollute the BODY data.
299-
unset($parsed[$this->config->tokenName]);
300-
$request->setBody(http_build_query($parsed));
301-
}
284+
285+
return;
286+
}
287+
288+
$body = $request->getBody() ?? '';
289+
290+
if ($body === '') {
291+
return;
292+
}
293+
294+
// If the token is found in JSON data, we can safely remove it.
295+
try {
296+
$json = json_decode($body, flags: JSON_THROW_ON_ERROR);
297+
} catch (JsonException) {
298+
$json = null;
302299
}
300+
301+
if (is_object($json) && property_exists($json, $tokenName)) {
302+
unset($json->{$tokenName});
303+
$request->setBody(json_encode($json));
304+
305+
return;
306+
}
307+
308+
// If the token is found in form-encoded data, we can safely remove it.
309+
parse_str($body, $result);
310+
unset($result[$tokenName]);
311+
$request->setBody(http_build_query($result));
303312
}
304313

305-
private function getPostedToken(RequestInterface $request): ?string
314+
private function getPostedToken(IncomingRequest $request): ?string
306315
{
307-
assert($request instanceof IncomingRequest);
316+
$tokenName = $this->config->tokenName;
317+
$headerName = $this->config->headerName;
308318

309-
// Does the token exist in POST, HEADER or optionally php:://input - json data or PUT, DELETE, PATCH - raw data.
319+
// 1. Check POST data first.
320+
$token = $request->getPost($tokenName);
310321

311-
if ($tokenValue = $request->getPost($this->config->tokenName)) {
312-
return is_string($tokenValue) ? $tokenValue : null;
322+
if ($this->isNonEmptyTokenString($token)) {
323+
return $token;
313324
}
314325

315-
if ($request->hasHeader($this->config->headerName)) {
316-
$tokenValue = $request->header($this->config->headerName)->getValue();
326+
// 2. Check header data next.
327+
if ($request->hasHeader($headerName)) {
328+
$token = $request->header($headerName)->getValue();
317329

318-
return (is_string($tokenValue) && $tokenValue !== '') ? $tokenValue : null;
330+
if ($this->isNonEmptyTokenString($token)) {
331+
return $token;
332+
}
319333
}
320334

321-
$body = (string) $request->getBody();
335+
// 3. Finally, check the raw input data for JSON or form-encoded data.
336+
$body = $request->getBody() ?? '';
322337

323-
if ($body !== '') {
324-
$json = json_decode($body);
325-
if ($json !== null && json_last_error() === JSON_ERROR_NONE) {
326-
$tokenValue = $json->{$this->config->tokenName} ?? null;
338+
if ($body === '') {
339+
return null;
340+
}
327341

328-
return is_string($tokenValue) ? $tokenValue : null;
342+
// 3a. Check if a JSON payload exists and contains the token.
343+
try {
344+
$json = json_decode($body, flags: JSON_THROW_ON_ERROR);
345+
} catch (JsonException) {
346+
$json = null;
347+
}
348+
349+
if (is_object($json) && property_exists($json, $tokenName)) {
350+
$token = $json->{$tokenName};
351+
352+
if ($this->isNonEmptyTokenString($token)) {
353+
return $token;
329354
}
355+
}
330356

331-
parse_str($body, $parsed);
332-
$tokenValue = $parsed[$this->config->tokenName] ?? null;
357+
// 3b. Check if form-encoded data exists and contains the token.
358+
parse_str($body, $result);
359+
$token = $result[$tokenName] ?? null;
333360

334-
return is_string($tokenValue) ? $tokenValue : null;
361+
if ($this->isNonEmptyTokenString($token)) {
362+
return $token;
335363
}
336364

337365
return null;
338366
}
339367

368+
/**
369+
* @phpstan-assert-if-true non-empty-string $token
370+
*/
371+
private function isNonEmptyTokenString(mixed $token): bool
372+
{
373+
return is_string($token) && $token !== '';
374+
}
375+
340376
/**
341377
* Returns the CSRF Token.
342378
*/

‎tests/system/Router/Attributes/CacheTest.php‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use CodeIgniter\Test\Mock\MockAppConfig;
2323
use Config\Services;
2424
use PHPUnit\Framework\Attributes\Group;
25+
use PHPUnit\Framework\MockObject\MockObject;
2526

2627
/**
2728
* @internal
@@ -33,16 +34,17 @@ protected function setUp(): void
3334
{
3435
parent::setUp();
3536

36-
// Clear cache before each test
3737
cache()->clean();
38-
38+
Services::resetSingle('response');
3939
Time::setTestNow('2026-01-10 12:00:00');
4040
}
4141

4242
protected function tearDown(): void
4343
{
4444
parent::tearDown();
4545

46+
cache()->clean();
47+
Services::resetSingle('response');
4648
Time::setTestNow();
4749
}
4850

@@ -215,11 +217,17 @@ private function createMockRequest(string $method, string $path, string $query =
215217

216218
$request = $this->getMockBuilder(IncomingRequest::class)
217219
->setConstructorArgs([$config, $uri, null, $userAgent])
218-
->onlyMethods(['isCLI'])
220+
->onlyMethods(['isCLI', 'withMethod', 'getMethod'])
219221
->getMock();
220222
$request->method('isCLI')->willReturn(false);
221-
$request->setMethod($method);
223+
$request->method('withMethod')->willReturnCallback(
224+
static function (string $method) use ($request): MockObject&IncomingRequest {
225+
$request->method('getMethod')->willReturn($method);
226+
227+
return $request;
228+
},
229+
);
222230

223-
return $request;
231+
return $request->withMethod($method);
224232
}
225233
}

0 commit comments

Comments
 (0)