Bug: beforeLoginUrl is not stored in session if a user is logged out while in a protected group route.

[sammyskills]

PHP Version

7.4

CodeIgniter4 Version

4.3.7

Shield Version

develop

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MariaDB 10.4.18

Did you customize Shield?

No.

What happened?

There seems to be a problem with the merged PR #793.

As it stands, the entrance URL will only be filled if the route is protected by the session filter directly, as seen here:

shield/src/Filters/SessionAuth.php

Lines 78 to 81 in 970c67e

	if (! url_is('login')) {
	$session = session();
	$session->setTempdata('beforeLoginUrl', current_url(), 300);
	}

This looks and works fine if the pages of an application are protected using the configuration below in the app/Config/Filters.php file:

public $globals = [
    'before' => [
        // ...
        'session' => ['except' => ['login*', 'register', 'auth/a/*']],
    ],
    // ...
];

A challenge arises when the application contains protected URLs for different user groups. For example, users: users/dashboard, admin: admin/dashboard. To make sure that users cannot access URLs they are not permitted, we apply the group filters: group:user and group:admin respectively. This would not have been a problem, but AFAIK, CI does not yet support arguments in filters via app/Config/Filters.php file, but it can be done via routes, in the app/Config/Routes.php file (see docs), like so:

$routes->get('users/dashboard', 'UsersController::index', ['filter' => 'group:user']);
$routes->get('admin/dashboard', 'UsersController::index', ['filter' => 'group:admin']);

With the configurations in the filter and routes, when we run php spark filter:check get users/dashboard, we get the following:

+--------+-----------------+----------------------------------------------------------+-----------------------------+
| Method | Route           | Before Filters                                           | After Filters               |
+--------+-----------------+----------------------------------------------------------+-----------------------------+
| GET    | users/dashboard | group session                                            | group                       |
+--------+-----------------+----------------------------------------------------------+-----------------------------+

From the output above, it means that if a user's session expired while trying to visit the users/dashboard page, there will be no temporary URL stored in the session, for redirection after logging in. This is because the group filter will run first, before the session filter. The AbstractAuthFilter redirects to the login page if a user is not logged in, before checking for the group:

shield/src/Filters/AbstractAuthFilter.php

Lines 32 to 34 in 970c67e

	if (! auth()->loggedIn()) {
	return redirect()->route('login');
	}

A workaround is to add the filters (session and group) in the routes config file, but from CI docs, it is strongly discouraged as it breaks backward compatibility:

Multiple filters is disabled by default. Because it breaks backward compatibility. If you want to use it, you need to configure. See Multiple Filters for a Route for the details.

Steps to Reproduce

Add this to the GroupFilterTest:

public function testFilterNotAuthorizedStoresRedirectToEntranceUrlIntoSession(): void
{
    $result = $this->call('get', 'protected-route');

    $result->assertRedirectTo('/login');

    $session = session();
    $this->assertNotEmpty($session->get('beforeLoginUrl'));
    $this->assertSame(site_url('protected-route'), $session->get('beforeLoginUrl'));
}

Expected Output

I expected that the entrance URL is stored in the session before logging out an unauthorized user.

Anything else?

There are two possible solutions I am looking at:

1. Advice devs to use route to configure multiple filters and update the docs.
2. Store the entrance URL into session before logging a user out in the AbstractAuthFilter class.

[datamweb]

The explanation was complete. Thanks for the full explanation.

1. Advice devs to use route to configure multiple filters and update the docs.

I've always had trouble with adding documentation for the user to do something.😀
So I like the second solution.

This is because the group filter will run first, before the session filter.

Another thing is that I have seen in the forum that people are looking for prioritizing the apply of filters, I don't know if this feature has been added, but if not, it seems that such a possibility is useful.

[sammyskills]

Yes, if filters can be prioritised, then this wouldn't be an issue.

[mshannaq]

@sammyskills beforeLoginUrl not working at all (tested in codeigniter 4.4.0 after updating ci4)

e.g
while /dashboard is protected route

public array $globals = [
        'before' => [
            // 'honeypot',
            // 'csrf',
            //@TODO remove tests* from before production
            'session' => ['except' => [ '/','tests*', 'lang*', 'account/login*', 'account/register', 'account/auth/a/*']],
            // 'invalidchars',
        ],
        'after' => [
            'toolbar',
            // 'honeypot',
            // 'secureheaders',
        ],
    ];

php spark filter:check get /dashboard

CodeIgniter v4.4.0 Command Line Tool - Server Time: 2023-08-30 15:07:31 UTC+00:00

+--------+------------+----------------+---------------+
| Method | Route      | Before Filters | After Filters |
+--------+------------+----------------+---------------+
| GET    | /dashboard | session        | toolbar       |
+--------+------------+----------------+---------------+

but when visit http://localhost/dashboard

we have the session store value

__ci_last_regenerate | 1693408168
-- | --
beforeLoginUrl | https://localhost/index.php/dashboard
__ci_vars | Array (     [beforeLoginUrl] => 1693408468 )

bit if I login then I will redirected to $redirects['login'] instead of beforeLoginUrl

Iam using Shiled (dev-develop 63891c7)

have you test it?

[sammyskills]

Why did you exclude '/' from the session filter?

[mshannaq]

Why did you exclude '/' from the session filter?

the / is for the homepage which does not need login.
However, even of I remove the / route from global session filter

public array $globals = [
        'before' => [
            // 'honeypot',
            // 'csrf',
            //@TODO remove tests* from before production
            'session' => ['except' => [ 'tests*', 'lang*', 'account/login*', 'account/register', 'account/auth/a/*']],
            // 'invalidchars',
        ],
        'after' => [
            'toolbar',
            // 'honeypot',
            // 'secureheaders',
        ],
    ];

it is still ignoring beforeLoginUrl after login and not redirecting to it.

[sammyskills]

@mshannaq I just updated a sample project from 4.3.7 to 4.4.0, and it works.

Can you confirm that you have the updated loginRedirect() method in your app/Config/Auth.php file, like so?

shield/src/Config/Auth.php

Lines 410 to 416 in b6d327d

	public function loginRedirect(): string
	{
	$session = session();
	$url = $session->getTempdata('beforeLoginUrl') ?? setting('Auth.redirects')['login'];
	return $this->getUrl($url);
	}

[mshannaq]

app/Config/Auth.php

ooops , I forgot to modify app/Config/Auth.php :)

thanks alot working now after updating app/Config/Auth.php

[kenjis]

I sent PR to change filter exec order.
codeigniter4/CodeIgniter4#7955