Bug: LogicException thrown when registered user attempts to login multiple times via API call

[AaronMk44]

PHP Version

8.0.8

CodeIgniter4 Version

4.2.10

Shield Version

1.0.0-beta.3

Which operating systems have you tested for this bug?

macOS

Which server did you use?

cli-server (PHP built-in webserver)

Database

MySQL 5.7.34

Did you customize Shield?

No

What happened?

LogicException thrown when user attempts to login multiple times via API call.

Steps to Reproduce

1. Perform initial setup of a codeigniter 4 project with shield
2. Using the guide: Mobile Authentication with Access Tokens create a loginController for mobile login
3. Provision a route to map the request in (2)
4. Create a user ahead of the following step
5. Using the credentials in (4), attempt to login via Postman or any other client app
6. Using the credentials in (4), attempt to login again via Postman or any other client app

Expected Output

The expected output is a LogicException:

{
...,
"code": 500,
"message": "The user has User Info in Session, so already logged in or in pending login state. 
If a logged in user logs in again with other account, the session data of the previous user 
will be used as the new user. Fix your code to prevent users from logging in without logging 
out or delete the session data. user_id: 6",
...
}

Anything else?

Nothing more.

[AaronMk44]

I'd suggest that the guide: Mobile Authentication with Access Tokens, uses the snippet below for authentication:

// Check login credentials
$loginCheck = auth()->check($credentials);

if (!$loginCheck->isOK()) {
    return $this->response
        ->setJSON(['error' => $loginCheck->reason()])
        ->setStatusCode(401);
}

$user = (new UserModel())->where('username', $input['username'])->first(); // or by email

// Generate token and return to client
$token = $user->generateAccessToken($input['deviceName']);

The reason behind this suggestion is that, the auth()->attempt() in a way does not comply with the principle that RESTful APIs must be stateless, and operate in complete isolation. Keeping session data about the currently logged in user, defeats the purpose of REST.

If the constraint a device should only be allowed to have a single user logged in must hold, then I think (opinion here) that the consumers of the framework (developers) should implement that check themselves and not the framework in this case (RESTful APIs).

Please refer to Advantages and disadvantages of statelessness - Hands-On RESTful API Design Patterns and Best Practices by Harihara Subramanian, Pethuru Raj and RESTful Statelessness

[kenjis]

Thank you for reporting.

As you pointed out, the sample code logs a user in with the Session auth. If it is needed, it is better to check if the user is logged in already.

Or it is not needed, we should change the sample code as you say.

In this use case, it does not seem that a Session auth login is required. But I'm not sure.

[lonnieezell]

If the route is just being protected by the tokens filter, then it shouldn't use the session at all. See the code at: https://github.com/codeigniter4/shield/blob/develop/src/Authentication/Authenticators/AccessTokens.php#L43

What you're describing makes it sound like the session filter is also being used?

[AaronMk44]

I see what you mean. So based on what the configuration that is set, if AccessTokens is set, then the sessions will be ignored either way, right?

[lonnieezell]

Correct.

But if you have the route protected by the chain filter, it will check both session and token auths as defined in the config file.

It looks like it is backwards from what it should be by default, though. It tries the session first. I think that needs to be reversed to check the tokens first.

[AaronMk44]

I agree, cause my fear was that the configuration could be overlooked by accident and cause a security vulnerability.

[kenjis]

@AaronMk44 You can check your routes and filters: https://codeigniter4.github.io/CodeIgniter4/incoming/routing.html#confirming-routes

[kenjis]

@AaronMk44 If you still have the issue, feel free to reopen or create a new issue.