Bug: CSRF Token Check Order is Mismatched in the Documentaion and the Implementation

[AmitSonkhiya]

PHP Version

8.1

CodeIgniter4 Version

4.3.2

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

No response

What happened?

As per the documentation

The order of checking the availability of the CSRF token is as follows:

1. $_POST array
2. HTTP header
3. php://input (JSON request)

However, when you notice the implementation, the order of execution is:

1. If HTTP header is present than assign it to the tokenName variable.
2. Else treat it as a JSON request and perform json_decode and assign token to variable if it exists.
3. Now check the same in the POST array and return it otherwise otherwise return from either 1, or 2.

So even the POST array contains the token, the header is still checked. Further, if it is absent then a slower json_decode is performed even through the body is empty. So while the POST array contains the token, if will be still fetched at the last.

Hence, it feels like this implementation doesn't confirm the behavior mentioned in the documentation.

Steps to Reproduce

Kindly review the documentation and the implementation as given in links.

Expected Output

Either documentation should be updated or code execution should be corrected.

Anything else?

No response

[kenjis]

The code is not optimized, but it seems the order in the docs is correct.

return $request->getPost($this->tokenName) ?? $tokenName;

If $_POST has the token, it will be returned first. If not, it will return $tokenName.

[AmitSonkhiya]

I agree that the order is correct that's why I used the term "Check Order". Or perhaps I should say CSRF token check order isn't optimized.

I think a small change can make a significant improvement as CSRF is usually checked for every eligible request method.

I'm very novice in fork, PR, and unit testing. However, I believe the code block is what you would love to place instead of the current method.

Thank you

private function getPostedToken(RequestInterface $request): ?string
    {
        assert($request instanceof IncomingRequest);

        // Does the token exist in POST, HEADER or optionally php:://input - json data.
        if ($tokenName = $request->getPost($this->tokenName)) {
            return $tokenName;
        }

        if ($request->hasHeader($this->headerName) && ! empty($request->header($this->headerName)->getValue())) {
            $tokenName = $request->header($this->headerName)->getValue();
        } else {
            $body = (string) $request->getBody();
            $json = json_decode($body);

            if ($body !== '' && ! empty($json) && json_last_error() === JSON_ERROR_NONE) {
                $tokenName = $json->{$this->tokenName} ?? null;
            } else {
                $tokenName = null;
            }
        }

        return $tokenName;
    }

[kenjis]

I sent a PR #7377

[AmitSonkhiya]

Thank you Kindly close the issue.