Skip to content

[Security][DX] RFC: A simple way to do programmatic login #40662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Seldaek opened this issue Mar 31, 2021 · 15 comments · Fixed by #41274
Closed

[Security][DX] RFC: A simple way to do programmatic login #40662

Seldaek opened this issue Mar 31, 2021 · 15 comments · Fixed by #41274
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) Help wanted Issues and PRs which are looking for volunteers to complete them. Security

Comments

@Seldaek
Copy link
Member

Seldaek commented Mar 31, 2021

Description
There are various reasons why an app may need to login a user without the user having to go through the configured authenticators. A few examples I ran into are logging the user in after a password reset, or after the registration confirmation email was clicked.

Example
IMO a great API to have would be Security::programmaticLogin(Request $request, UserInterface $user, ?AuthenticatorInterface $authenticator = null)

After discussing this with @wouterj - it appears that the main complexity here is in choosing which Token class should be used. IMO it'd be fine to by default, pick the default authenticator for the current firewall, and make that one authenticate the user via createAuthenticatedToken. If that's not ok the developer can optionally pass the correct authenticator.

Why?
I had to figure this out myself and it took some help from @wouterj + some trial and error to arrive at this code:

        try {
            $this->userChecker->checkPreAuth($event->getUser());
        } catch (AuthenticationException $e) {
            // skip authenticating if any pre-auth check does not pass
            return;
        }
        if (($response = $this->userAuthenticator->authenticateUser($event->getUser(), $this->myPreferredAuthenticator, $event->getRequest()))) {
            return $response;
        }

Now as you can see, you have to know about UserAuthenticatorInterface to get that service injected, you then have to also figure out what the class name of your preferred authenticator is to get that injected, and then I also had to remember to call the user checker otherwise inactive users were being logged in under some circumstances which is not good.

Not too bad, I survived, but I can imagine this would be a bigger roadblock for others less familiar with the framework, so having some easier API in a more visible location would be nice.

@carsonbot carsonbot added DX DX = Developer eXperience (anything that improves the experience of using Symfony) Security labels Mar 31, 2021
@chalasr
Copy link
Member

chalasr commented Apr 1, 2021

👍

@chalasr chalasr added the Help wanted Issues and PRs which are looking for volunteers to complete them. label Apr 1, 2021
@stof
Copy link
Member

stof commented Apr 1, 2021

@wouterj shouldn't authenticateUser take care of the user checker ?

@javiereguiluz
Copy link
Member

If the API is up to debate, would the following work too? It's the API I use to login users manually in tests:

public function login(UserInterface $user, string $firewallName ='main');

@Seldaek
Copy link
Member Author

Seldaek commented Apr 3, 2021

The API is definitely up in the air. It's just a proposal. I would rather avoid calling it only login though because IMO the name should highlight this isn't how you implement a login feature where the user logs in via this method after you checked their password or whatever. It's simply a way to programmatically trigger a login for special cases.

@stloyd
Copy link
Contributor

stloyd commented Apr 3, 2021

Maybe we could use name like:

  • simulateUserLogin(),
  • authenticateUser()

Only suggestion 🙃 but that being said I'm all for it cause I end up in every project implementing such functionality for tests.

@Seldaek
Copy link
Member Author

Seldaek commented Apr 3, 2021

@stloyd Note that for tests, as of 5.1 there is https://symfony.com/blog/new-in-symfony-5-1-simpler-login-in-tests

@gggeek
Copy link

gggeek commented Apr 4, 2021

I won't comment on the API proposal itself, as I have not been following "recent" developments in the auth area of Symfony, but I would warmly welcome simple, well documented and cast-in-stone methods to programatically log in / log out users.

ATM the process for me to log in a specific user in say, a console command, or in a controller for some crazy convoluted usecase is to... google it. The results which come up are generally different, varying on Symfony version, and do not instill great confidence for something as security critical.

@ahundiak
Copy link
Contributor

ahundiak commented Apr 5, 2021

With guard authenticators, make:registration-form gives you a working example of how to login from a controller. I always assumed that it was the 'right way' to do things.

Unlike make:auth, the registration command has not yet been updated to generate the proper line of code for the new 5.2 experimental stuff but I assume that will happen at some point. But it's easy enough to do. Sure you need to know about the UserAuthenticatorInterface and the security bundle's UserAuthenticator class but that seems to be a question of updating the documentation.

As far as logging in from a console command, considering that so much of the new security stuff is in the Security\Http namespace then I don't know how you could even approach it. I get wanting to do it for functional tests but to casually login to a console command and then authorize something? Seems like a hack no matter what you do.

@hktr92
Copy link

hktr92 commented Apr 29, 2021

This would be a great feature of the framework. We needed to authenticate the user after register, so we've created a custom service which handles the authentication by injecting LoginFormAuthenticator, GuardAuthenticatorHandler and RememberMeServicesInterface to make it work.

I'm not sure about code's stability, though, since we kind of bypass the authentication mechanism (by manually calling $guardAuthenticatorHandler->onAuthenticationHandler() after performing other required steps. So having it provided by the framework, it would be great.

Additionally, I'm not sure how this could be wrapped -- AFAIK, Authentication Manager will be the default in 6.0

johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue May 18, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue May 19, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue May 19, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue May 19, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue May 19, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Jun 3, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Jun 3, 2021
@chalasr
Copy link
Member

chalasr commented Jun 7, 2021

This could also be an opportunity to fix #37575.

johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Aug 8, 2021
…ogin (symfony#40662)

Signed-off-by: Arnaud Frézet <arnaud@les-tilleuls.coop>
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Aug 8, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Aug 8, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Aug 8, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 2, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 6, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 6, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 6, 2021
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@Seldaek
Copy link
Member Author

Seldaek commented Dec 8, 2021

Nope, but #41274 is on the case 👍🏻

@carsonbot carsonbot removed the Stalled label Dec 8, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 11, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 11, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 11, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Dec 11, 2021
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Jun 8, 2022
@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@pscheit
Copy link
Contributor

pscheit commented Jun 9, 2022

Carson! Hold your horses! :)

@carsonbot carsonbot removed the Stalled label Jun 9, 2022
johnkrovitch added a commit to johnkrovitch/symfony that referenced this issue Jun 16, 2022
chalasr pushed a commit to johnkrovitch/symfony that referenced this issue Jul 4, 2022
@chalasr chalasr closed this as completed in 6243772 Jul 5, 2022
@Pixelshaped
Copy link

Pixelshaped commented Jul 11, 2022

Up until now this could be done like this:

public function refreshToken(UserInterface $user): void
    {
        $this->container->get(TokenStorageInterface::class)->setToken(
            new UsernamePasswordToken($user, 'main', $user->getRoles())
        );
    }

Looking forward to implement the cleaner method in my project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX DX = Developer eXperience (anything that improves the experience of using Symfony) Help wanted Issues and PRs which are looking for volunteers to complete them. Security
Projects
None yet