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

feature: dynamic secret/keys via secretOrKeyProvider #29

Closed
wants to merge 4 commits into from
Closed

feature: dynamic secret/keys via secretOrKeyProvider #29

wants to merge 4 commits into from

Conversation

tomsiwik
Copy link
Contributor

@tomsiwik tomsiwik commented Mar 31, 2019

PR Checklist

Please check if your PR fulfils the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behaviour?

There is no way to assign a secret dynamically.
Related to issue number: #26

What is the new behaviour?

With secretOrKeyProvider you can assign a secret or public/private key for each sign/verify request separately.

Does this PR introduce a breaking change?

[ ] Yes
[x] No //changed, see last commit

This PR introduces a breaking change how secretOrPrivateKey is assigned. Before, on the verification step the private Key was used when publicKey was not assigned due to secretOrPrivateKey function. Setting the secret or private/public key is now explicit. The following options set the secret now by priority:

  1. secretOrKeyProvider
  2. secret
  3. publicKey and privateKey

Edit:

secretOrKey was re-added to smoothen out the upgrading process and takes precedence over the listed options above. With a deprecation warning and added tests this should be enough I hope.

Other information

I'm currently in the process of adding tests to this module. However, suggestions, critique or discussions are welcome. My time is quite limited but I'm happy to help and answer questions.

This is my first contribution to open source :) quite exciting tbh.

@tomsiwik tomsiwik changed the title WIP: dynamic secret/keys assignment via secretOrKeyProvider function Dynamic secret/keys assignment via secretOrKeyProvider function Mar 31, 2019
@tomsiwik
Copy link
Contributor Author

@kamilmysliwiec hey, small heads up that the PR is ready. I have added a few tests for the changes I made but I'm not used to testing (especially not with nestjs). Hope everything is fine like that. Please let me know if there is anything to improve.

@tomsiwik
Copy link
Contributor Author

Small note: I re-added secretOrKey with a deprecation warning and tests to make the upgrade smoother between versions. Hope that helps.

@tomsiwik tomsiwik changed the title Dynamic secret/keys assignment via secretOrKeyProvider function feature: dynamic secret/keys via secretOrKeyProvider Apr 12, 2019
@shayded-exe
Copy link

I'm currently having to implement a workaround until this is released. It would be awesome if it was merged soon :)

@tomsiwik
Copy link
Contributor Author

tomsiwik commented May 5, 2019

I'm racing against renovate. Will create a new PR with a single commit

@tomsiwik tomsiwik closed this May 5, 2019
@tomsiwik tomsiwik deleted the feature/secret-provider branch May 5, 2019 13:47
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.

None yet

3 participants