Skip to content
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

failure_path is not used when "connect" feature is disabled #2008

Open
Prometee opened this issue Jul 9, 2024 · 2 comments
Open

failure_path is not used when "connect" feature is disabled #2008

Prometee opened this issue Jul 9, 2024 · 2 comments

Comments

@Prometee
Copy link

Prometee commented Jul 9, 2024

Q A
Bug? yes
New Feature? no
Support question? yes
Version 2.x

Actual Behavior

Using a custom authenticator throwing an AccountNotLinkedException (I'm not using "connect" feature), the user is redirected to the login_path with an http request parameter named error containing the error message.

Expected Behavior

With hwi/oauth-bundle previous version, the failure_path was used and a Session parameter containing the authentication error was available.
Right now the failure_path is not used if we are not using "connect" feature (I'm aware of this PR #1990 that fix part of the issue but not the session parameter issue).

Steps to Reproduce

I over simplify the Authenticator in purpose, the real one contains much more code.

<?php

declare(strict_types=1);

namespace App\Security\Http\Authenticator;

use HWI\Bundle\OAuthBundle\Connect\AccountConnectorInterface;
use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
use HWI\Bundle\OAuthBundle\Security\Core\Exception\AccountNotLinkedException;
use HWI\Bundle\OAuthBundle\Security\Core\User\OAuthAwareUserProviderInterface;
use Sylius\Bundle\UserBundle\Provider\UsernameOrEmailProvider as BaseUserProvider;
use Symfony\Component\Security\Core\User\UserInterface;

class SyliusShopUserProvider extends BaseUserProvider implements AccountConnectorInterface, OAuthAwareUserProviderInterface
{

    public function loadUserByOAuthUserResponse(UserResponseInterface $response): UserInterface
    {
        throw new AccountNotLinkedException();
    }

    public function connect(UserInterface $user, UserResponseInterface $response): void
    {
        throw new AccountNotLinkedException();
    }
}

Possible Solutions

Right now I override the failure handler service definition like this (⚠️ this can work only if you are not using "connect" feature) :

    hwi_oauth.authentication.failure_handler:
        parent: security.authentication.failure_handler

Finding a way to keep the Symfony default failure handler while adding the specific things needed for the "connect" feature can be a way to avoid those issues. WDTY @stloyd ?

BTW reading how the "connect" feature is getting the authentication error, I think there is a way to simply remove this custom failure handler and simply change how the RegisterController is retrieving the authentication error.
I do it like this in my own custom one :

<?php

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;

final class RegistrationController extends AbstractController
{
    public function __construct(
        private AuthenticationUtils $authenticationUtils,
    ) {
    }
    
    public function __invoke(): Response
    {
        $error = $this->authenticationUtils->getLastAuthenticationError();
    }
}
@Prometee Prometee changed the title failure_path is not used when "connect" feature is not used failure_path is not used when "connect" feature is disabled Jul 10, 2024
Copy link

Message to comment on stale issues. If none provided, will not mark issues stale

@github-actions github-actions bot added the Stale label Sep 18, 2024
@Prometee
Copy link
Author

Do not stale

@github-actions github-actions bot removed the Stale label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant