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

allow files to be used as HMAC secrets #111

Closed
wants to merge 1 commit into from
Closed

Conversation

jbg
Copy link

@jbg jbg commented Jan 13, 2021

This PR resolves the issue I reported in #109, so that HMAC secrets can be read from a file. Previously, if one attempted to read from a file by passing @filename (as the --help text suggests), the literal string @filename was used as the secret instead.

As an aside, anyone who has used this tool with HMAC JWTs must have quite brute-forceable secrets, since they likely contain only printable characters.

@mike-engel
Copy link
Owner

Thanks for this @jbg! Could you also add some tests for slurping files? If you're unsure how to proceed, let me know and I can try to describe it here

@jbg
Copy link
Author

jbg commented Jan 13, 2021

You mean just testing the combination of file token and HMAC? There are already two tests of the file slurping code at the end of jwt-cli.rs

@mike-engel
Copy link
Owner

Correct, just for HMAC file tokens. At some point, I'd like to make the file slurping work for everything, but until then, I'd like to add a test for HMAC since they don't share the same code and to make sure it doesn't break in the future

@jbg
Copy link
Author

jbg commented Jan 13, 2021

Got it! Will take care of this a little later. Thanks for the clarification!

@mike-engel
Copy link
Owner

Hi @jbg, I wanted to check in to see if you've had time to add some tests, or if you need any help. Thanks!

@lizfeed lizfeed mentioned this pull request Jun 16, 2021
3 tasks
@mike-engel
Copy link
Owner

Closing in favor of #130

@mike-engel mike-engel closed this Jul 21, 2021
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

2 participants