codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.38k stars 1.9k forks source link

Bug: CI 4.2.0 - running CLI command with csrf enabled causes error #6086

Closed sclubricants closed 2 years ago

sclubricants commented 2 years ago

PHP Version

8.0

CodeIgniter4 Version

4.2

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

cli

Database

No response

What happened?

After updating to version 4.2 I'm having errors running cli. If I disable csrf filter in App/Config/Filters then I don't get the error.

Steps to Reproduce

Create a controller App\cron\test\run().

Ensure csrf filter enabled.

diff --git a/app/Config/Cookie.php b/app/Config/Cookie.php
index 8ee01c71b..37a24299c 100644
--- a/app/Config/Cookie.php
+++ b/app/Config/Cookie.php
@@ -62,7 +62,7 @@ class Cookie extends BaseConfig
      *
      * @var bool
      */
-    public $secure = false;
+    public $secure = true;

     /**
      * --------------------------------------------------------------------------
diff --git a/app/Config/Filters.php b/app/Config/Filters.php
index d0a97238b..960ae2358 100644
--- a/app/Config/Filters.php
+++ b/app/Config/Filters.php
@@ -34,7 +34,7 @@ class Filters extends BaseConfig
     public $globals = [
         'before' => [
             // 'honeypot',
-            // 'csrf',
+            'csrf',
             // 'invalidchars',
         ],
         'after' => [

Run command: php8.0 /var/www/portal/public/index.php cron/test/run

Response: [CodeIgniter\Security\Exceptions\SecurityException]

The action you requested is not allowed.

at SYSTEMPATH/HTTP/ResponseTrait.php:711

And in the logs: CRITICAL - 2022-06-06 09:54:01 --> The action you requested is not allowed. in SYSTEMPATH/HTTP/ResponseTrait.php on line 711 1 SYSTEMPATH/HTTP/ResponseTrait.php(711): CodeIgniter\Security\Exceptions\SecurityException::forDisallowedAction() 2 SYSTEMPATH/HTTP/ResponseTrait.php(701): CodeIgniter\HTTP\Response->dispatchCookies() 3 SYSTEMPATH/HTTP/ResponseTrait.php(445): CodeIgniter\HTTP\Response->sendCookies() 4 SYSTEMPATH/CodeIgniter.php(1078): CodeIgniter\HTTP\Response->send() 5 SYSTEMPATH/CodeIgniter.php(507): CodeIgniter\CodeIgniter->sendResponse() 6 SYSTEMPATH/CodeIgniter.php(351): CodeIgniter\CodeIgniter->handleRequest() 7 FCPATH/index.php(103): CodeIgniter\CodeIgniter->run()

The error is thrown here:

    private function dispatchCookies(): void
    {
        /** @var IncomingRequest $request */
        $request = Services::request();

        foreach ($this->cookieStore->display() as $cookie) {
            if ($cookie->isSecure() && ! $request->isSecure()) {
                throw SecurityException::forDisallowedAction();
            }

Expected Output

Expected to return without error.

Anything else?

I'm not exactly sure of everything going on here but I've found a way to get rid of the error.

In vendor/codeigniter4/framework/system/HTTP/IncommingRequest Line 348:

    public function isCLI(): bool
    {
        return false;
    }

Replace with the method in version 4.1.8

    public function isCLI(): bool
    {
        return is_cli();
    }

Then everything runs fine.

I'm still trying to find the issue.

Any ideas?

sclubricants commented 2 years ago

The isCLI() method is called from vendor/codeigniter4/framework/system/Filters/CSRF.php

    public function before(RequestInterface $request, $arguments = null)
    {
        if ($request->isCLI()) {
            return;
        }

        $security = Services::security();

        try {
            $security->verify($request);
        } catch (SecurityException $e) {
            if ($security->shouldRedirect() && ! $request->isAJAX()) {
                return redirect()->back()->with('error', $e->getMessage());
            }

            throw $e;
        }
    }
kenjis commented 2 years ago

I've confirmed this bug. Thank you for reporting.

kenjis commented 2 years ago

@sclubricants Yes, as a workaround, the following change solves the error.

    public function isCLI(): bool
    {
        return is_cli();
    }
iRedds commented 2 years ago

For console call, use spark commands instead of controllers. https://codeigniter.com/user_guide/cli/cli_commands.html

Using the controller you are loading the Request, Response, Filter classes, which don't make sense for the CLI.

marios88 commented 2 years ago

@kenjis @MGatner

https://github.com/codeigniter4/CodeIgniter4/issues/6086#issuecomment-1148006449 makes it work again but it looks like you made that change here https://github.com/codeigniter4/CodeIgniter4/pull/5653 a while ago

Running $this->request->isCLI() from a controller always returns false now. Maybe consider reverting https://github.com/codeigniter4/CodeIgniter4/pull/5653 until your other pull (https://github.com/codeigniter4/CodeIgniter4/pull/6089) goes through?

edit: Quick Fix for anyone interested file: app/Config/Services.php

   /**
     * The Request class models an HTTP request.
     *
     * @return \App\Libraries\IncomingRequest
     */
    public static function request(App $config = NULL, $getShared = TRUE) {
        if ($getShared) {
            return static::getSharedInstance('request', $config);
        }

        $config ??= config('App');

        return new \App\Libraries\IncomingRequest(
            $config,
            AppServices::uri(),
            'php://input',
            new UserAgent()
        );
    }

file:/app/Libraries/IncomingRequest.php

<?php

namespace App\Libraries;

class IncomingRequest extends \CodeIgniter\HTTP\IncomingRequest{    
    public function isCLI() {
        return is_cli();
    }   
}
kenjis commented 2 years ago

I reverted https://github.com/codeigniter4/CodeIgniter4/pull/5653 for now.

kenjis commented 2 years ago

v4.2.1 has been released with revert #5653, which temporarily fixes this bug.

But the root cause is still there, and we need to fix it. See #6089