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

Moved mandatory dependency in composer.json #349

Closed
wants to merge 1 commit into from

Conversation

zebba
Copy link

@zebba zebba commented Jun 14, 2017

When using an instance of lexik_jwt_authentication.encoder.lcobucci an error message shows up:

  [Symfony\Component\Debug\Exception\ClassNotFoundException]                  
  Attempted to load class "Sha512" from namespace "Lcobucci\JWT\Signer\Rsa".  
  Did you forget a "use" statement for another namespace?

This error can be triggered if you did not install the dev-dependencies by simply retrieving the service from the container:

$service = $this->getContainer()->get('lexik_jwt_authentication.encoder.lcobucci');

Moving the dependency fixes the issue.

Moved a required dependency from require-dev to require, fixing an issue with lexik_jwt_authentication.jws_provider.lcobucci where a signer could not be found
@chalasr
Copy link
Collaborator

chalasr commented Jun 14, 2017

In fact, using the lcobucci encoder is optional, it's not the default one. Making the lcobucci/jwt a soft dependency is made on purpose, it should remain as is until we make it our default JOSE library (which is planed for the next major).

Meanwhile, we should provide a meaningful exception when the package is missing, e.g. in the encoder constructor. Would you mind to do it in this PR instead?

@zebba
Copy link
Author

zebba commented Jun 14, 2017

I actually don't believe throwing an exception is the right way to solve this: it will still prevent the dependency injection container to be build entirely because creating an instance will still fail.

However I assume we can get rid of the Lcobucci-related services from within the LexikJWTAuthenticationExtension if the library is not present.

@chalasr
Copy link
Collaborator

chalasr commented Jun 15, 2017

You're right, the right fix would be to not register these services if the library is missing

@chalasr
Copy link
Collaborator

chalasr commented Jun 21, 2017

@zebba Would you mind to finish this or shall I take it over?

@zebba
Copy link
Author

zebba commented Jul 5, 2017

I've tried to look into it, but was not able to solve it yet.

@chalasr
Copy link
Collaborator

chalasr commented Oct 6, 2017

Fixed in #383. Thanks for trying!

@chalasr chalasr closed this Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants