AbstractLoginFormAuthenticator & Redirecting to Previous URL

Very happy with all these walkthroughs as always. One question - I have the following authenticate() method with a flag on the user to block login via the form for certain users.
As it currently behaves, it will check whether this block flag is on or not BEFORE the password is checked. This has the side effect that you can find out whether a user exists or not even if you don't pass the correct password.

Where would I put this deny-flag-check in order to do it only once the password is successfully checked (but before the user is authenticated... I understand that I could do it in the onAuthenticationSuccess method, but I want it before the session is approved)

public function authenticate(Request $request): PassportInterface
{

    $email = $request->request->get('email');
    $password = $request->request->get('password');

    return new Passport(
        new UserBadge($email, function($userIdentifier) {
            // optionally pass a callback to load the User manually
            $user = $this->userRepository->findOneBy(['email' => $userIdentifier]);

            if (!$user) {
                throw new UserNotFoundException();
            } elseif ($user->isDenyFormLogin() == true) { //checking for denyFlag on user
                throw new CustomUserMessageAuthenticationException('Form login denied for this user.');
            }

            return $user;
        }),
        new PasswordCredentials($password),
        [
            new CsrfTokenBadge(
                'authenticate',
                $request->request->get('_csrf_token')
            ),
            (new RememberMeBadge())->enable(),
        ]
    );
}

Hey MattWelander!

Happy new year! This is an excellent question. Take this logic out of your authenticator and instead, add it to an event subscriber on the CheckPassportEvent::class event. The PasswordCredentials themselves are actually checked via a listener on this event: https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/Security/Http/EventListener/CheckCredentialsListener.php - you can use it as a guide. To be sure that you're running AFTER the password check, you could set your priority to -1 for this subscriber - so like this:

public static function getSubscribedEvents(): array
{
    return [CheckPassportEvent::class => ['checkPassport', -1]];
}

Let me know if that works out! Btw, a side effect of this (which is probably good) is that if you have any other ways for a user to authenticate (including, iirc, via a "remember me cookie"), those will also be "subject" to this check. In other words, no matter how your user tries to log in, if they fail this check, they will fail authentication. If this is NOT what you want (and you only want this check to happen for this ONE authenticator), that's no problem. Do this:

A) Create a new badge class - e.g. CheckDenyFormLoginBadge. It can basically be empty
B) Add this to your Passport
C) In your subscriber, only run the check if this badge is present.

Cheers!

Hello.

Great guide. it's really helping me a lot.
I am writing this comment because while I was trying this step "AbstractLoginFormAuthenticator & Redirecting to Previous URL", after all the changes the authentication process did not seem to take place.

the solution I found was to change the LoginFormAuthenticator class
adding the motodo

    public function supports(Request $request): bool
    {
        return $request->isMethod('POST') && $this->getLoginUrl($request) === $request->getRequestUri();
    }

this

$this->getLoginUrl($request) === $request->getPathInfo()

it returned false and did not make the magic work.

Hey Andrea Gelmini !

It look like you have a tiny error in your first code example. This is how we do the supports() check

    public function supports(Request $request): ?bool
    {
        return ($request->getPathInfo() === '/login' && $request->isMethod('POST'));
    }

You can notice we hardcode the "/login" string

Cheers!

I'm confused =)

At one stage in this script you say:
"This means that... oh yea... we can remove code! Let's see: delete supports(), onAuthenticationFailure() and also start():"

In the resulting example, the method supports() is completely erased. Like Andrea Gelmini says above, this causes the authenticator to go completely disconnect. the login form is no longer attached to an authenticator, submitting the form will only render the login form anew.

By your answer to Andrea above, it seems that you confirm that the supports() method should still be in the LoginFormAuthenticator? Was it a mistake to tell us to remove it alltogether and the script needs amending? Or is there something in mine and Andreas environment that is configured differently than in your environment, causing the supports() method from the AbstractLoginFormAuthenticator to go disconnect?

Hmm, I see what you mean. Let me clarify what's going on. I think you likely already know some of this, but just to clear everything up:

A) Every authenticator DOES need a supports() method.
B) But, when we extend AbstractLoginFormAuthenticator, you can remove the supports() method in your class, simply because it already exists in the parent class.

So yes, I think Diego's comment was not quite right.

Now, to the real question:

Or is there something in mine and Andreas environment that is configured differently than in your environment, causing the supports() method from the AbstractLoginFormAuthenticator to go disconnect?

Yes, possibly :). For some reason, the supports() method that you're inheriting is returning false when it should return true. You could add some debugging code to that method to find out why, though I might have an idea: are you running your site at the root of a domain - e.g. http://127.0.0.1:8000 is your homepage? Or is it under a subdirectory - e.g. http://127.0.0.1:8000/site is the homepage? If it is the latter, there is a known bug in that inherited method that makes it not match correctly. In that case, you should keep your supports() method so it works. Wow, and apparently I opened that issue - lol - https://github.com/symfony/symfony/issues/44893 - and it's fixed in Symfony 5.4.13, 6.0.14, 6.1.6 and 6.2.0 and higher.

Anyways, that was just a guess at the problem. If I'm wrong, I'd love to know what you find going wrong in that parent supports() method!

Cheers!

That was in fact a pretty good guess - I'm testing it off of http://localhost:8888/mysite/ 😂 thanks

Yay! Well then I'm double glad that this will, at least, be fixed in later versions!

Have fun!