Bug: cannot login after logging out from url

[bgeneto]

PHP Version

8.2.12

CodeIgniter4 Version

4.4.1

Shield Version

dev-develop

Which operating systems have you tested for this bug?

Windows, Linux

Which server did you use?

apache

Database

MariaDB

Did you customize Shield?

No

What happened?

Shield now attempts to redirect users to the page they were trying to access before logging in. However, if an unauthenticated user tries to directly access the logout URL (/logout route), it creates a login/logout loop, preventing them from logging in.

Steps to Reproduce

• While not logged in, access /logout route.
• Now try to logging, you will be redirected to logout page

Expected Output

I think Shield should ignore the logout route, i.e., not redirect the user to this route after login (to avoid this login/logout loop issue).

Anything else?

No response

[datamweb]

@bgeneto Thank you for your report, I confirm this issue.

Can you try with the following settings?

    public array $globals = [
        'before' => [
            // 'honeypot',
            // 'csrf',
            // 'invalidchars',
            'session' => ['except' => ['login*', 'register', 'auth/a/*', 'logout']],
        ],
        'after' => [
            'toolbar',
            // 'honeypot',
            // 'secureheaders',
        ],
    ];

[bgeneto]

@bgeneto Thank you for your report, I confirm this issue.

Can you try with the following settings?

    public array $globals = [
        'before' => [
            // 'honeypot',
            // 'csrf',
            // 'invalidchars',
            'session' => ['except' => ['login*', 'register', 'auth/a/*', 'logout']],
        ],
        'after' => [
            'toolbar',
            // 'honeypot',
            // 'secureheaders',
        ],
    ];

'Logout' was already in my 'force-reset' exceptions. However, not in session exceptions—that was the solution! I believe this should be included in the documentation. Thanks for pointing out.

[paulbalandan]

My 2 cents. Should logout route be accessed not by GET but by POST?

[kenjis]

See https://security.stackexchange.com/questions/62769/should-login-and-logout-action-have-csrf-protection

[datamweb]

Should logout route be accessed not by GET but by POST?

@paulbalandan I don't understand what you mean, we don't have route /logout as POST.

CodeIgniter v4.4.1 Command Line Tool - Server Time: 2023-10-20 00:35:22 UTC+00:00

+--------+-------------------------+--------------------+--------------------------------------------------------------------+----------------+---------------+
| Method | Route                   | Name               | Handler                                                            | Before Filters | After Filters |
+--------+-------------------------+--------------------+--------------------------------------------------------------------+----------------+---------------+
| GET    | /                       | »                  | \App\Controllers\Home::index                                       | session        | toolbar       |
| GET    | register                | »                  | \CodeIgniter\Shield\Controllers\RegisterController::registerView   |                | toolbar       |
| GET    | login                   | »                  | \CodeIgniter\Shield\Controllers\LoginController::loginView         |                | toolbar       |
| GET    | login/magic-link        | magic-link         | \CodeIgniter\Shield\Controllers\MagicLinkController::loginView     |                | toolbar       |
| GET    | login/verify-magic-link | verify-magic-link  | \CodeIgniter\Shield\Controllers\MagicLinkController::verify        |                | toolbar       |
| GET    | logout                  | »                  | \CodeIgniter\Shield\Controllers\LoginController::logoutAction      |                | toolbar       |
| GET    | auth/a/show             | auth-action-show   | \CodeIgniter\Shield\Controllers\ActionController::show             |                | toolbar       |
| POST   | register                | »                  | \CodeIgniter\Shield\Controllers\RegisterController::registerAction |                | toolbar       |
| POST   | login                   | »                  | \CodeIgniter\Shield\Controllers\LoginController::loginAction       |                | toolbar       |
| POST   | login/magic-link        | »                  | \CodeIgniter\Shield\Controllers\MagicLinkController::loginAction   |                | toolbar       |
| POST   | auth/a/handle           | auth-action-handle | \CodeIgniter\Shield\Controllers\ActionController::handle           |                | toolbar       |
| POST   | auth/a/verify           | auth-action-verify | \CodeIgniter\Shield\Controllers\ActionController::verify           |                | toolbar       |
+--------+-------------------------+--------------------+--------------------------------------------------------------------+----------------+---------------+

[paulbalandan]

What I mean is currently logout is setup in routes using GET. Should that be POST instead?

[datamweb]

I didn't see an example POST used for logout.

[kenjis]

The following flow seems good to me.

GET /logout → shows a form to logout (confirmation page) POST /logout → does logout

[kenjis]

It seems Laravel uses (used?) the following code:

<ul class="dropdown-menu" role="menu">
  <li>
    <a
      href="{{ url('/logout') }}"
      onclick="event.preventDefault();document.getElementById('logout-form').submit();">
      Logout
    </a>

    <form id="logout-form" action="{{ url('/logout') }}" method="POST" style="display: none;">
      {{ csrf_field() }}
    </form>
  </li>
</ul>

https://stackoverflow.com/questions/38789965/why-does-laravel-by-default-logout-via-post-as-opposed-to-get

[paulbalandan]

From the same SO article.

GET requests are supposed to be "safe" and shouldn't have any significant side effects. It shouldn't matter, for example, if a precaching feature of a browser followed the link. That should just get some data.

Logging the user out would be a significant side effect, so GET would be inappropriate.

I believe also logout should not be accessed regularly by GET.