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

Get default signer at runtime #212

Merged
merged 2 commits into from
Feb 2, 2019
Merged

Get default signer at runtime #212

merged 2 commits into from
Feb 2, 2019

Conversation

sgtpepper43
Copy link
Contributor

@sgtpepper43 sgtpepper43 commented Jan 14, 2019

Setting @joken_default_signer as a module attribute requires that the config is set at compile time. This means that should a secret key be compromised, the user must recompile the whole application. Moving the parse_config method into the existing __default_signer__ method will allow the user to swap out the config without recompiling the app.

Also, in our case and maybe others, besides wanting to be able to swap out the key without recompiling, it's a pain to ensure that environment variables are even available at compile time. Which is more our own problem, but still.

Fixes #211

@victorolinasc
Copy link
Collaborator

Sorry for not looking into it yet. I'll take a look this week.

@victorolinasc
Copy link
Collaborator

I've finally had the time to look into this and as I suspected it has some really minor impact on performance. It is really minimal... on my runs the difference was about less then us but was consistent.

Anyway, I'll be merging this now as I don't think this will impact anyone besides ultra freak performance fanatics. Those would probably not run a release and override the base64url module anyway having a stronger difference...

Thanks once again for your contribution!

@victorolinasc victorolinasc merged commit 0a8a28a into joken-elixir:master Feb 2, 2019
@lucasmazza
Copy link

@victorolinasc any plans for an upcoming release with this patch? 😄

@victorolinasc
Copy link
Collaborator

I'll probably have a release ready this weekend. Sorry for the delay but work + life is not easy lately.

I'll add the PR that is pending review too. Probably gonna fix what I've commented there too.

Once again, sorry for the delay.

@danilokleber
Copy link

How should I use this to build the signer inside of the Joken configured module? Sorry if that is too dumb of a question. I'd like to do something similar to #211 (comment) but in the nicer DSL of the default_signer key.

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.

4 participants