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

Added exception if secret key or private and public key is missing #123

Conversation

eschricker
Copy link
Contributor

If the secret key (symmetric algo) or private and public key (asymmetric algo) are missing, you will get a type mismatch error from Lcobucci (see #116).

Description

An own exception has been added which will be thrown, when the either the secret key or private/public key are missing. In that case you will get a better error message.

Fixes #116

Checklist:

  • I've added tests for my changes or tests are not applicable
  • I've changed documentations or changes are not required
  • I've added my changes to CHANGELOG.md

@eschricker eschricker self-assigned this Feb 22, 2022
@Messhias
Copy link
Collaborator

@eschricker can we merge it?

@eschricker
Copy link
Contributor Author

From my side it is fine. @mfn would you like to have a look at the changes?

@mfn
Copy link
Contributor

mfn commented Mar 12, 2022

Will review soon!

@Messhias Messhias merged commit 0b33165 into PHP-Open-Source-Saver:main Apr 6, 2022
Copy link
Contributor

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late to the party 😅

LGTM, just one minor suggestion.

Comment on lines +46 to +48
if(is_null($secret) && (is_null($keys['public']) || is_null($keys['private']))) {
throw new SecretMissingException();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only improvement I an think would be to include a nice message, pointing out which secret is missing specifically.

@florentausha
Copy link

This seems like a breaking change for an app that uses this package only for authenticating users.
If you don't create tokens with this app, the private key, passphrase and secret are not required.

JWT_PASSPHRASE, JWT_SECRET and JWT_PRIVATE_KEY are now mandatory environment variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants