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

Fix HMAC support #369

Merged
merged 1 commit into from
Jun 12, 2018
Merged

Fix HMAC support #369

merged 1 commit into from
Jun 12, 2018

Conversation

chalasr
Copy link
Collaborator

@chalasr chalasr commented Jul 29, 2017

  • Config
  • Encoders

Fixes #307
Fixes #340

@chalasr chalasr changed the title Fix LcobucciJWSProvider HMAC support Fix HMAC support Jul 29, 2017
@chalasr chalasr changed the base branch from 2.4 to master October 12, 2017 09:03
@chalasr chalasr force-pushed the lcobucci/fix-hmac-support branch 2 times, most recently from 61c9e60 to 72f46ae Compare October 12, 2017 09:08
@chalasr chalasr force-pushed the lcobucci/fix-hmac-support branch 2 times, most recently from 695deaf to d8adaed Compare October 26, 2017 13:26
@chalasr chalasr force-pushed the lcobucci/fix-hmac-support branch 3 times, most recently from 1195745 to b01c822 Compare November 8, 2017 15:53
@chalasr chalasr force-pushed the lcobucci/fix-hmac-support branch 3 times, most recently from bac2e67 to 4ef2b3f Compare January 22, 2018 12:39
@chalasr chalasr removed the Bug label Jan 27, 2018
@mrailton
Copy link

Any update as to when this could be implemented?

@chalasr
Copy link
Collaborator Author

chalasr commented Mar 2, 2018

@railto This will be released by the end of the month, some cleanup needed.

@piteur
Copy link

piteur commented Mar 13, 2018

@chalasr - Can't wait for this to be implemented. It'll be easier to deploy with docker thanks to this PR. No more volume to store only the keys...
Thanks for your work !

@@ -25,12 +25,23 @@ public function getConfigTreeBuilder()
->addDefaultsIfNotSet()
->children()
->scalarNode('private_key_path')
->setDeprecated('The "%path%.%node%" configuration key is deprecated since version 2.5. Use "%path%.secret_key%" instead.')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the new configuration key should contain 2 or 4 % symbols, not 3, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch

->defaultNull()
->end()
->scalarNode('public_key_path')
->setDeprecated('The "%path%.%node%" configuration key is deprecated since version 2.5. Use "%path%.public_key%" instead.')
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@masseelch
Copy link
Contributor

@chalasr What is the progress here? Do you need any help?

@chalasr chalasr force-pushed the lcobucci/fix-hmac-support branch 6 times, most recently from 9448f84 to d4c4954 Compare June 12, 2018 10:15
@chalasr
Copy link
Collaborator Author

chalasr commented Jun 12, 2018

Sorry for being latish guys, this is ready and will be merged today (not now in case one wants to give it a last review).

@chalasr chalasr added Ready and removed In Progress labels Jun 12, 2018
@chalasr chalasr merged commit a6a7718 into lexik:master Jun 12, 2018
chalasr added a commit that referenced this pull request Jun 12, 2018
This PR was merged into the 2.x-dev branch.

Discussion
----------

Fix HMAC support

- [x] Config
- [x] Encoders

Fixes #307
Fixes #340

Commits
-------

a6a7718 Support HMAC based signers from lcobucci/jwt
@chalasr chalasr deleted the lcobucci/fix-hmac-support branch June 12, 2018 21:44
$container->setParameter('lexik_jwt_authentication.public_key_path', $config['public_key']);
}

if (empty($config['public_key']) && empty(['secret_key'])) {
Copy link

Choose a reason for hiding this comment

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

Shouldn't it be empty($config['secret_key'])?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice catch! thanks

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.

6 participants