mariovalney / laravel-keycloak-web-guard

Simple Keycloak Guard to Laravel Web Routes - https://packagist.org/packages/vizir/laravel-keycloak-web-guard
146 stars 80 forks source link

Login Error #19

Closed valentin-b99 closed 4 years ago

valentin-b99 commented 4 years ago

Hi,

A few months ago (around January), I added a Keycloak login feature (laravel-keycloak-web-guard) on my admin panel. This one, worked very well, and today I wanted to update your dependency but I met an error.

When the KeycloakWeb :: getLoginUrl () method located in the login () method of the Vizir \ KeycloakWebGuard \ Controllers \ AuthController controller is called, the server returns an 500 error with this message :

Symfony \ Component \ Debug \ Exception \ FatalThrowableError (E_RECOVERABLE_ERROR) Argument 1 passed to Vizir \ KeycloakWebGuard \ Services \ KeycloakService :: __ construct () must implement interface GuzzleHttp \ ClientInterface, null given

So I tried to move this dependency into an earlier version (https://github.com/Vizir/laravel-keycloak-web-guard/commit/cfe8e761cd7acb0a0c624a1591b72bdef75368fc) and it works well.

Is this a mistake on your part, or do I have to add things?

Thank you in advance for your answers.

Regards, Valentin.

mariovalney commented 4 years ago

Hey, Valentin!

What version are you using? 1.5.5? And Laravel version? Are you extending KeycloakService?

We improved the way GuzzleHttp client is created to allow developers add their stuff. But by mistake I broke version 1.5.4 for who extended the KeycloakService class.

Btw in 1.5.3 we already had ClientInterface in DI...

valentin-b99 commented 4 years ago

Hello,

I'm using version 1.9.1 of Composer and version 5.8.36 of Laravel. No, I am not extending KeycloakService. I'm just extending AuthController for Keycloak routes (including the slightly customized callback route)

PS: I try to remove and require again the dependency and it doesn't work... So I use the version ^1.5 of your dependency.

valentin-b99 commented 4 years ago

Hello,

I coming back to you after doing some research about the source of my error and I found out how to resolve it (however, I don't know if it breaks another system...).

Indeed, after analyzing the differences between the version of this commit that works for me (https://github.com/Vizir/laravel-keycloak-web-guard/commit/cfe8e761cd7acb0a0c624a1591b72bdef75368fc) the only difference is at line 69 of the file KeycloakWebGuardServiceProvider.php.

Diff link: https://github.com/Vizir/laravel-keycloak-web-guard/compare/cfe8e76..e589798

Following this observation, I therefore tried to remove this:

$this->app->when(KeycloakService::class)
        ->needs(ClientInterface::class)
        ->give(static function() {
            new Client(Config::get('keycloak-web.guzzle_options', []));
        });

to put this back there instead: $this->app->bind(ClientInterface::class, Client::class); and it works.

I also tested using a newer commit (https://github.com/Vizir/laravel-keycloak-web-guard/commit/5e8f96f227cf7aa498f35ace572c1de63e1b1d3a) and it works too.

I think there are 2 solutions to solve the problem:

  1. What I said a little above.
  2. Construct with no parameters and set $this->httpClient like this:$this->httpClient = new Client(Config::get('keycloak-web.guzzle_options', []));

So, what do you think about this? Can you update your dependency?

Thank you in advance for your answers. Regards, Valentin

mariovalney commented 4 years ago

Hey. Good monday.

Version 1.5.6 fixes this issue. Thanks for reporting and sorry for the inconvenience.

The problem was introduced on 8cbe58a: we forgot to return the service.