Bug: nested $routes->group() with same filter name doesn't work as exepcted

[tangix]

PHP Version

8.1

CodeIgniter4 Version

4.5.2

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

cli-server (PHP built-in webserver)

Database

No response

What happened?

A route definition containing nested group() statements with filters having the same name doesn't execute all the filters before executing the Controller method.

Steps to Reproduce

Please see https://github.com/tangix/ci45-nested-groups for a complete example.
Check the README.md file for instructions.

Expected Output

If the same filter is defined with different arguments (after the : character) the filter should be executed multiple times with the different arguments passed or at least have the multiple arguments passed as an array.

Anything else?

No response

[kenjis]

I have confirmed this behavior.

@codeigniter4/core-team
In the current implementation, we cannot run the same filter more than once with the different arguments.
Because the Filters class can store only one arguments (array) for a filter class as $this->argumentsClass[$className].

https://codeigniter4.github.io/CodeIgniter4/incoming/routing.html#nesting-groups
Logically, I think the behavior in the user guide should be the correct.
But to fix this issue, I think we need to change the properties data structure in the Filters class,
and it will be breaking changes.

[tangix]

@kenjis
I would have expected the $arguments to a single before() call to contain all the passed arguments to the filter. Feels like this would be a non-breaking change, but I may be incorrect.

For the time being, I'll work around this by creating multiple filters with different names.

Your remark about the docs and Nesting Groups; I think the example and its description is misleading. The code works only if the names are different, not like in the example code.

[kenjis]

I would have expected the $arguments to a single before() call to contain all the passed arguments to the filter.

It seems we need to discuss the expected use cases and the expected behavior.

Is there a possible use case where the same filter is executed more than once?
If there is, it cannot be achieved with an implementation that collects all the arguments together and executes it only once.
If we do not allow executing a filter more than once, probably your idea is a way to go.

Your remark about the docs and Nesting Groups; I think the example and its description is misleading. The code works only if the names are different, not like in the example code.

Yes, the current user guide explanation is wrong due to this issue.

[tangix]

Is there a possible use case where the same filter is executed more than once? If there is, it cannot be achieved with an implementation that collects all the arguments together and executes it only once. If we do not allow executing a filter more than once, probably your idea is a way to go.

Maybe my implementation is flawed but I have a BearerAuthFilter checking a JWT token of all incoming requests (it's a pretty busy AJAX backend so I don't use native sessions).
The JWT contains the following authorizations:

{
  "iss": "https://host.xyz.se/admin-api/",
  "iat": 1718782025,
  "jti": "01J0QP8QCH6YM5NX7J23SVFMHT",
  "nbf": 1718782024,
  "exp": 1718782925,
  "access": {
    "comp": 775,
    "empl": 263,
    "user": 16135,
    "gen": 209664,
    "event": 203535,
    "conf": 1515264,
    "rep": 3840
  }
}

Users have different access to different places in the system. In my routes I have multiple filter options such as bearer-auth:comp, bearer-auth:user,write, bearer-auth:config.
I decode and verify the JWT, caching it for multiple filter evaluations, and checking if the user 1) has admin access 2) access to the specific part and 3) read-only or read/write access.

In my process of tidying up Routes.php with nested $routes->group() I ran into this issue.
Currently my code looks like this (I implemented App\Filters\Auth\Tangix and the alias auth-tangix):

$routes->group(
    'config',
    ['except' => ['new', 'edit'], 'filter' => 'bearer-auth:conf'],
    static function ($routes) {

        // ....

        $routes->group('tangix', ['filter' => 'auth-tangix'], static function ($routes) {
            // ...
        });

        // ...

    }
);

Maybe creating App\Filters\Auth\Config and alias to auth-config, replacing bearer-auth:conf is the correct way to go?

[michalsn]

To be clear. Is this a bug or bug+regression (it worked fine before 4.5)?

IMO collecting all the arguments together and executing them only once is not an option since it may lead to security issues. E.g. Shield filters. However, the current behavior can also be a security issue.

The filter should be executed every time it's defined with passed arguments.

Using aliases seems like a nice temporary workaround to make filters work as they should.

[michalsn]

Nope... from what I see in the code using aliases will not work.

[tangix]

To be clear. Is this a bug or bug+regression (it worked fine before 4.5)?

Hehe, I'd say bug since after migrating 4.4 -> 4.5 I started exploring with nested groups based on what I learned from reading through the docs after upgrading. My 4.4 code was different.

[michalsn]

Ok, thanks @tangix. It's a little comforting.

[tangix]

Ok, thanks @tangix. It's a little comforting.

In any case, the docs should be updated regardless because they are misleading/confusing

[kenjis]

In my understanding, this is not a regression. Because $routes->group() did not merge filters in v4.4.* or before.

Note
Prior to v4.5.0, due to a bug, options passed to the outer group() are not merged with the inner group() options.
https://codeigniter4.github.io/CodeIgniter4/incoming/routing.html#nesting-groups

[michalsn]

Yes, merging filters is probably the only change that happened to filters in v4.5 and it has nothing to do with the issue.
The problem is just as you described:

In the current implementation, we cannot run the same filter more than once with the different arguments.
Because the Filters class can store only one arguments (array) for a filter class as $this->argumentsClass[$className].

[tangix]

FWIW - I've replaced my original code with one Filter having $arguments with multiple Filter classes without $arguments and as far as I can see, the code works as expected.

[kenjis]

I sent PR #8977 to 4.6.