shield icon indicating copy to clipboard operation
shield copied to clipboard

Bug: Error exception occurs when a user with pending registration tries to log in.

Open sammyskills opened this issue 2 years ago • 3 comments

PHP Version

7.4

CodeIgniter4 Version

4.2.6

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?

I tried to log in without entering the registration confirmation code from the email, and I got the error message:

The user has identities for action, so cannot complete login. If you want to start to login with auth action, use startLogin() instead. Or delete identities for action in database. user_id: 23680

Steps to Reproduce

  1. Activate the register action
  2. Enable magic link logins
  3. Register, but do not get the confirmation code/token from your email
  4. Open a new browser, click on magic link to get a one-time-login link
  5. Go to your email address and click on the login link

Expected Output

I expect that the user should be redirected to the auth token confirmation page with a more-descriptive message stating that:

You must complete your registration by confirming the tokens sent to your email address.

Anything else?

No response

sammyskills avatar Oct 04 '22 19:10 sammyskills

Hey @sammyskills , Thanks for the report.

This is not a bug and is intentional. What I understand is that you have a request to add a message to the user. I agree with clearly displaying the messages to the users and I think it makes the end users feel good.

On the other hand, Kenjis is usually concerned about shield security and providing error details. Therefore, my opinion is positive by displaying the error message. But see what @kenjis says.

datamweb avatar Oct 04 '22 20:10 datamweb

Hi @datamweb, Thanks for the reply.

One of the reasons why I raised the issue is that in a production environment, the user will be presented with a whoops page, which IMO is not ideal for user experience. It is not a problem with the software but from the user, so the user should be made to know that he/she caused the issue.

But just as you said, let see what @kenjis and other team members think about it.

sammyskills avatar Oct 04 '22 20:10 sammyskills

This is a bug. Because LogicException should not be thrown in production environments. The logic that uses the Session Auth is incorrect, and should be fixed. The Exception message is for devs, not end users. (All Exception messages are for devs.)

https://github.com/codeigniter4/shield/blob/6c64ec7209fbce08686cc18f0bdd7d732d5d19e9/src/Authentication/Authenticators/Session.php#L695-L703

kenjis avatar Oct 04 '22 21:10 kenjis

I sent a PR: #503

kenjis avatar Nov 02 '22 06:11 kenjis