lexik / LexikJWTAuthenticationBundle

JWT authentication for your Symfony API
MIT License
2.52k stars 610 forks source link

Not working with Symfony 3.0 #118

Closed noliva closed 8 years ago

noliva commented 8 years ago

For some reason "symfony/framework-bundle": "~2.3|~3.0" didn't work for me, I did try to fork and remove the ~2.3 and it did install it.

The output of the error with composer was:

Your requirements could not be resolved to an installable set of packages.

Problem 1

Potential causes:

Is someone else having this issue?

sukei commented 8 years ago

It's because the bundle has not been tagged yet. The "~2.3|~3.0" constraint is only available on the master branch, which is "dev" for composer. If your composer.json is not configured on "minimum-stability": "dev", then you won't be able to install the bundle on Symfony 3.0.

flo-sch commented 8 years ago

I actually got it working using the dev-master branch, but adding a "prefers-stable" tag on my composer.json file :

{
    "require": {
        [...]
        "lexik/jwt-authentication-bundle": "dev-master"
    },
    "minimum-stability": "stable",
    "prefers-stable": true
}
ooflorent commented 8 years ago

Since master branch seems to be compatible, could you tag it?

slashfan commented 8 years ago

Hi, juste released the 1.4.0 tag including basic symfony 3.0 support. There is a PR pending #126 to replace the legacy request service by the request_stack but it has not yet been merged as it will introduce BC break for 2.3 and I'd like to keep the 1.* branch of the bundle alive until 2.3 end of life in a few months.

Anyone willing to work on a "hack" to try to make it both symfony 2.3+ and 3.0+ compatible ? Like we did with the security storage / security context deprecation.

flo-sch commented 8 years ago

Excellent, thanks! I'll try to see tomorrow if I can create a PR to keep 2.3 & 3.0 together.

flo-sch commented 8 years ago

Hum, okay I had a quick look. Symfony3 is by definition breaking BC about this case.

I can either inject the whole service container, then choose the right method to inject the request, basically something like :

The definition:

// Lexik\Bundle\JWTAuthenticationBundle\Resources\config\services.xml
<service id="lexik_jwt_authentication.jwt_manager" [...]>
    [...]
    <call method="setRequest">
        <argument type="service" id="service_container" on-invalid="null" strict="false" />
    </call>
    [...]
</service>

And the implementation:

// Lexik\Bundle\JWTAuthenticationBundle\Services\JWTManager.php
/**
 * @param Container $container
 */
public function setRequest($container = null)
{
    if ($container->has('request_stack')) {
        $this->request = $container->get('request_stack')->getCurrentRequest();
    } elseif ($container->has('request')) {
        $this->request = $container->get('request');
    } else {
        // throw something
    }
}

I totally dislike this kind of method, but I don't see any better way to handle it. Any idea about that?

The only other way I think about would be to release a 2.0 branch now, without Symfony2.3, even as a dev version.

flo-sch commented 8 years ago

Any alternative option I can think about would be to pass both request_stack and request as arguments, and check if the first is null.

The definition:

// Lexik\Bundle\JWTAuthenticationBundle\Resources\config\services.xml
<service id="lexik_jwt_authentication.jwt_manager" [...]>
    [...]
    <call method="setRequest">
        <argument type="service" id="request_stack" on-invalid="null" strict="false" /> <!-- for Symfony >= 2.4 -->
        <argument type="service" id="request" on-invalid="null" strict="false" /> <!-- for Symfony < 2.4 -->
    </call>
    [...]
</service>

And the implementation:

// Lexik\Bundle\JWTAuthenticationBundle\Services\JWTManager.php
/**
 * @param RequestStack $requestStack
 * @param Request $request
 */
public function setRequest(RequestStack $requestStack = null, Request $request = null)
{
    if ($requestStack !== null) {
        $this->request = $requestStack->getCurrentRequest();
    } elseif ($request !== null) {
        $this->request = $request;
    } else {
        throw new \InvalidArgumentException('JWTManager::setRequest method needs at least one valid argument, either a Symfony\Component\HttpFoundation\RequestStack instance or a Symfony\Component\HttpFoundation\Request instance.');
    }
}

Would you prefer this way?

tcardonne commented 8 years ago

see #126

flo-sch commented 8 years ago

Yeah, it actually looks cleaner with a compiler pass indeed, I'll sleep smarter tonight :)

tcardonne commented 8 years ago

All the credit goes to Spomky :p

slashfan commented 8 years ago

Thanks guys it has been merged and tagged (1.4.1) !

SamuelBreton commented 8 years ago

:thumbsup: