Update documentation, drop unmaintained Symfony

[tacman]

The documentation still refers to the "new" authentication system, how to implement the Guard authenticator, and the examples use the old annotations rather than attributes.

What do you think of a new release that requires PHP 8.1+ and Symfony 6.4+? (I know, Symfony 5.4 is still maintained, but developers can use version 1 if they need that).

This is a great bundle, but the documentation seems to lean toward making it easy for people on old versions of Symfony rather than promoting best practices on newer ones. For example,

 enable_authenticator_manager: true

was only relevant from Symfony 5.1 to 6.2. Authentication is already a bit complicated, I think the documentation should reflect usage based on current Symfony. To refer to it as the "new" authentication manager when it was released nearly 4 years ago is confusing.

symfony new --webapp auth-demo && cd auth-demo
bin/console make:user
composer require knpuniversity/oauth2-client-bundle

Ideally, the developer could cut and paste from the README and it should work.

I'm happy to help with the documentation, as I often walk through it when I'm adding social media login to a new site. But supporting Symfony 4 at the expense of Symfony 6.4/7 seems wrong.

Related to #432

[bocharsky-bw]

Hey @tacman ,

Thanks for raising this question! Here's my personal opinion on it:

The docs really need to be updated, agree. First of all, this bundle already supports PHP 8.1+, see composer.json - and so let's drop annotations in favor of attributes in the docs.

About dropping Symfony 4.4 - let's drop its support already 👍 About 5.4 - I think it would be too aggressive. I would prefer to keep the 5.4 support, at least for one more year while it's actively supported by maintainers. This bundle has a simplified release policy, so it's easier to stick to Symfony support versions.

I think the documentation should reflect usage based on current Symfony

Yeah, I agree with it. We should promote instructions for the latest version, but also it would be handy to provide some comments and legacy code mentioning legacy versions. I'm against of dropping the legacy instructions completely from the docs as it may cause many issues for users on legacy versions.

If you have some ideas and could help with updating the docs - it would be much appreciated and warmly welcome! ❤️

[tacman]

Can we at least move the Guard section to its own page, with a big ol' note that says "This is deprecated".

I think most people want to just cut and paste, at least initially.

Besides the http->https hack, what did you think of my GithubController in #437 ? We could use that instead of the Facebook one that's in the documentation now, though I'd love to have sample controllers for the most popular ones (Google, Facebook and Instagram at least, and probably X/Twitter).

In fact, I'm working on a bundle that wraps all of these into a single controller, but since I'm having trouble getting that to work, I broke it out into a stand-alone app to make debugging easier. But once I get oauth-demo working I'll document my bundle.

[tacman]

https://github.com/survos-sites/oauth-demo/blob/main/src/Controller/GithubController.php

<?php

namespace App\Controller;

use App\Entity\User;
use App\Security\AppAuthenticator;
use Doctrine\ORM\EntityManagerInterface;
use KnpU\OAuth2ClientBundle\Client\ClientRegistry;
use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
use Symfony\Component\Routing\Attribute\Route;
use Symfony\Component\Security\Http\Authentication\UserAuthenticatorInterface;

class GithubController extends AbstractController
{

    public function __construct(
        private EntityManagerInterface $entityManager,
        private UserAuthenticatorInterface $userAuthenticator,
        private AppAuthenticator $authenticator,
        private UserPasswordHasherInterface $userPasswordHasher
    )
    {
    }
    /**
     * Link to this controller to start the "connect" process
     */
    #[Route('/connect/github', name: 'connect_github_start')]
    public function connect(ClientRegistry $clientRegistry): Response
    {
        // will redirect to Github!
        $redirect = $clientRegistry
            ->getClient('github') // key used in config/packages/knpu_oauth2_client.yaml
            ->redirect([
                 'email' // the scopes you want to access
            ]);
        if (!str_starts_with($redirect->getTargetUrl(), 'https')) {
//            $redirect->setTargetUrl()
        }
        assert(str_starts_with($redirect->getTargetUrl(), 'https'), "Missing https in " . $redirect->getTargetUrl());
        # hack for not returning https
        $redirect->setTargetUrl(str_replace('http%3A', 'https%3A', $redirect->getTargetUrl()));

        return $redirect;
    }

    /**
     * After going to Github, you're redirected back here
     * because this is the "redirect_route" you configured
     * in config/packages/knpu_oauth2_client.yaml and in the Github App page
     */
    #[Route('/connect/github/check', name: 'connect_github_check')]
    public function connectCheckAction(Request $request, ClientRegistry $clientRegistry): Response
    {

        /** @var \KnpU\OAuth2ClientBundle\Client\Provider\GithubClient $client */
        $client = $clientRegistry->getClient('github');

        try {
            // the exact class depends on which provider you're using
            /** @var \League\OAuth2\Client\Provider\GithubResourceOwner $user */
            $accessToken = $client->getAccessToken();
            $oAuthUser = $client->fetchUserFromToken($accessToken);
            $email = $oAuthUser->getEmail();

            if (!$user = $this->entityManager->getRepository(User::class)->findOneBy(['email' => $email])) {
                $user = (new User())
                    ->setEmail($email);
                $this->entityManager->persist($user);
            }
            // better is to redirect to a page requiring the user to set/change their password, or allow null passwords.
            $plaintextPassword = $oAuthUser->getId();
            $hashedPassword = $this->userPasswordHasher->hashPassword(
                $user,
                $plaintextPassword
            );
            $user
                ->setPassword($hashedPassword)
                ->setGithubId($accessToken);
            $this->entityManager->flush();

            $this->userAuthenticator->authenticateUser($user, $this->authenticator, $request);

        } catch (IdentityProviderException $e) {
            // something went wrong!
            // probably you should return the reason to the user
            dd($e->getMessage());
        }

        return $this->redirectToRoute('app_app');
    }
}

[bocharsky-bw]

Can we at least move the Guard section to its own page, with a big ol' note that says "This is deprecated".

yeah, I think we can simplify the main page and move that specific legacy section to its own page. Do you want to help with that?

I don't remember what social provider works with localhost, if GitHub works - that might be a good main example. But you're right, I would like to see other popular providers like Google/Facebook (probably SymfonyConnect) too in the examples, because they are too popular. Maybe show them all on the main docs page is overkill, but do it on a different page would be great.

[tacman]

We should consider adding the most common workflow to the documentation. You can see in my example that I created a user if it didn't already exists, and added the access token to the user record. But since by default password is not nullable, I added one.

For my application, and I'm guessing for most, if the user record doesn't exist I want to redirect them to the registration form, both so they can set their own password but also to accept the T&C and maybe add some additional information, like name or organization.