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

Add OpenSSLKeyLoader #154

Merged
merged 1 commit into from
Apr 4, 2016
Merged

Conversation

chalasr
Copy link
Collaborator

@chalasr chalasr commented Apr 1, 2016

Add a service that loads keys from parameters (key paths and passphrase as arguments of the service).
The service has been injected in the JWTEncoder rather than the 3 parameters, same for the method checkOpenSSLConfig that has been moved in the service and now takes no argument.

This also Improve checks for keys and their corresponding exception messages (One for file existing/reading problem, one for openssl loading, this for both public and private keys).

This avoid duplicated code (Command & Encoder) and make the bundle configuration easier to debug (centralised key loading logic).

chalasr referenced this pull request Apr 1, 2016
…d_not_be_required

Made the public and private key paths not required…
@chalasr chalasr force-pushed the patch_openssl-checks branch 2 times, most recently from fc2e789 to 996e341 Compare April 1, 2016 22:42
@chalasr chalasr changed the title Add CanCheckOpenSSLTrait Add OpenSSLKeyLoader Apr 1, 2016
@chalasr chalasr force-pushed the patch_openssl-checks branch 7 times, most recently from 560e939 to 8f43677 Compare April 2, 2016 00:05
@slashfan
Copy link
Contributor

slashfan commented Apr 2, 2016

Good idea @chalasr ! Thanks !

@slashfan
Copy link
Contributor

slashfan commented Apr 2, 2016

Do you think you could add some tests for the OpenSSLKeyLoader class ?

@chalasr
Copy link
Collaborator Author

chalasr commented Apr 2, 2016

@slashfan Yes of course, I just realize that I forgotten them. I tell you when it's ok.

Improve checks for keys and their corresponding exception messages.
Move them in a trait used in both Encoder\JWTEncoder and Command\CheckOpenSSLCommand.
Avoid duplicated code and make bundle configuration easier to debug

Optimize trait methods & their usage in JWTEncoder

Replace trait by a OpenSSLKeyLoader service

Fix phpdoc

Fix phpdoc & typo

Improve code quality

Fix bad property name

Fix undeclared property

Spacing

Add OpenSSLKeyLoaderTest
@chalasr
Copy link
Collaborator Author

chalasr commented Apr 2, 2016

@slashfan Unit tests added !

@slashfan
Copy link
Contributor

slashfan commented Apr 4, 2016

Great !

@slashfan slashfan merged commit c012cac into lexik:master Apr 4, 2016
@chalasr chalasr deleted the patch_openssl-checks branch April 4, 2016 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants