-
Notifications
You must be signed in to change notification settings - Fork 376
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 regular expressions and procs to verify issuer #437
Allow regular expressions and procs to verify issuer #437
Conversation
Hello, @rewritten! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
881e688
to
fb3f4eb
Compare
|
||
```ruby | ||
JWT.decode token, hmac_secret, true, | ||
iss: method(:valid_issuer?), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the method definition need to be before calling JWT.decode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method definition order does not matter, they are all available when an instance is created. Of course I expect the valid_issuer?
method to be defined at the class level, not inside the same code block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this would have been a script or a snipped pasted into a console i think the order would matter.
@rewritten Are you planning on continuing on the feature. Just trying to ignore the tests on the old rubies would be enough to get the workflow happy. Also thinking that after the next release we will drop the official support fo EOLed Rubies and this PR can be merged without changes. |
@anakinj owww I completely missed your message. I will keep this PR updated to the latest master and make it work on non-EOLed rubies (and therefore I don't have to dance around them) |
@rewritten EOL ruby support is now dropped from the master branch. Think a rebase will make everything greener. |
I'll rebase this week |
5adee48
to
bb9f7b9
Compare
SourceLevel has finished reviewing this Pull Request and has found:
|
@anakinj ready for CI |
Should be fixed now - I have run rubocop locally and it does not complain anymore |
Thanks for your contribution @rewritten. Highly appreciated. |
NP, count on me for further PRs if you want to offload work |
This PR adds support for regexp/callable object in issuer validation:
The use case is when one has a generic public key resolver (using a block) and there is a need of additional restrictions in a specific context. Of course the rules here are completely arbitrary.