codecasts / laravel-jwt

Dead simple, plug and play JWT API Authentication for Laravel (5.4+)
MIT License
234 stars 27 forks source link

\Illuminate\Auth\Events\Login not firing #16

Closed imikemiller closed 6 years ago

imikemiller commented 7 years ago

Not detecting the \Illuminate\Auth\Events\Login event. This should be true for all auth events (I havent tested them) as the Codecasts\Auth\JWT\Auth\Guard::$events is not set.

Possible solution is to change the constructor but think it should be set by the framework somewhere.

    /**
     * JWT Guard constructor.
     *
     * @param \Illuminate\Contracts\Foundation\Application $app
     * @param string $name
     * @param \Illuminate\Contracts\Auth\UserProvider $provider
     * @param \Codecasts\Auth\JWT\Contracts\Token\Manager $manager
     */
    public function __construct($app, $name, $provider, $manager)
    {
        // assign constructor arguments into instance scope.
        $this->app = $app;
        $this->name = $name;
        $this->provider = $provider;
        $this->manager = $manager;
        $this->setDispatcher($this->app['events']); //add this to ensure $events has properly populated dispatcher
    }
hernandev commented 7 years ago

True, I know how to solve it. Give me a couple of days.

On Sep 25, 2017 6:26 AM, "Mike Miller" notifications@github.com wrote:

Not detecting the \Illuminate\Auth\Events\Login event. This should be true for all auth events (I havent tested them) as the Codecasts\Auth\JWT\Auth\Guard::$events is not set.

Possible solution is to change the constructor but think it should be set by the framework somewhere.

/**     * JWT Guard constructor.     *     * @param \Illuminate\Contracts\Foundation\Application $app     * @param string $name     * @param \Illuminate\Contracts\Auth\UserProvider $provider     * @param \Codecasts\Auth\JWT\Contracts\Token\Manager $manager     */    public function __construct($app, $name, $provider, $manager)    {        // assign constructor arguments into instance scope.        $this->app = $app;        $this->name = $name;        $this->provider = $provider;        $this->manager = $manager;        $this->setDispatcher($this->app['events']); //add this to ensure $events has properly populated dispatcher    }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/codecasts/laravel-jwt/issues/16, or mute the thread https://github.com/notifications/unsubscribe-auth/ABFyO3oA6fZ178uWC4zq9m8B3MYZvP5Lks5sl3HGgaJpZM4PicAk .

hernandev commented 7 years ago

Just moved as confirmed, the release should not take long.

hernandev commented 7 years ago

This issue was updated with the hacktoberfest label, so anyone which contributes here will be able to count contributions

imikemiller commented 7 years ago

I had another look at this. An alternative would be to change \Codecasts\Auth\JWT\Auth\ServiceProvider::register method

    /**
     * Register Guard.
     */
    public function register()
    {
        // gets the auth factory instance and register on provider attribute.
        $this->authManager = $this->app->make(AuthManager::class);

        // register auth policies.
        $this->registerPolicies();

        // define a "jwt" guard.
        $this->authManager->extend('jwt', function ($app, $name, array $config) {
            // gets a instance of the token manager
            $tokenManager = $this->getTokenManager();

            // gets a instance of the user provider
            $userProvider = $this->getUserProvider($config['provider']);

            // creates a new guard instance passing a provider and a token manager
            $guard = new Guard($app, $name, $userProvider, $tokenManager);

            //sets the event dispatcher
            $guard->setDispatcher($app['events']);

            return $guard;
        });
    }

Laravel does this step in \Illuminate\Auth\AuthManager when creating the guard instance. This seems like the equivalent place and makes more testable architecture than putting it in the constructor. I'm still not 100% but it would resolve the issue

hernandev commented 7 years ago

@imikemiller got it now, the guard instance does not have the events dispatcher instance.

hernandev commented 7 years ago

solving it on the next release 0.9.0, that will be on in the next hour

hernandev commented 7 years ago

Version 0.9.0 was just released and the problem should be fixed now, Let me know if it does not, I'm also testing it on a real app right now.

hernandev commented 6 years ago

Closing by inactivity, feel free to reopen if the problem persists.