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 regular expressions and procs to verify issuer #437

Merged
merged 5 commits into from
Jan 13, 2022

Conversation

rewritten
Copy link
Contributor

@rewritten rewritten commented Aug 3, 2021

This PR adds support for regexp/callable object in issuer validation:

JWT.decode(token, nil, true, verify_iss: true, iss: /some-regexp/)
JWT.decode(token, nil, true, verify_iss: true, iss: ->(issuer) { check_something(issuer) })
JWT.decode(token, nil, true, verify_iss: true, iss: method(:foo_bar))

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.

# This endpoint will accept JWTs from all registered issuers.
class ApplicationController
  PublicKeyRegistry = lambda do |_, claims|
    retrieve_the_corresponding_pulic_key_or_null(claims['iss'])
  end

  def check_jwt
    token = request.authorization&.delete_prefix('Bearer ')
    JWT.decode(token, nil, true, verify_iss: true, iss: allowed_issuers, algorithm: 'HS256', &PublicKeyRegistry)
  end

  def allowed_issuers = //
end

# This endpoint will only accept calls from a subset of the registered issuers.
class ChildController < ApplicationController
  # only issuers namespaced on one service
  def allowed_issuers = ->(iss) { iss.start_with('my-internal-service') }
end

# This endpoint will only accept calls from a subset of the registered issuers.
class NewController < ApplicationController
  # only issuers with the current year
  def allowed_issuers = ->(iss) { iss[/\d{4}].to_i == Date.today.year }
end

@sourcelevel-bot
Copy link

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.

@rewritten rewritten force-pushed the verify-issuer-with-proc-and-regexp branch from 881e688 to fb3f4eb Compare August 3, 2021 12:33

```ruby
JWT.decode token, hmac_secret, true,
iss: method(:valid_issuer?),
Copy link
Member

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?

Copy link
Contributor Author

@rewritten rewritten Aug 24, 2021

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.

Copy link
Member

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 rewritten requested a review from anakinj August 24, 2021 15:25
@anakinj
Copy link
Member

anakinj commented Oct 1, 2021

@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.

@rewritten
Copy link
Contributor Author

@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)

@anakinj
Copy link
Member

anakinj commented Dec 28, 2021

@rewritten EOL ruby support is now dropped from the master branch. Think a rebase will make everything greener.

@rewritten
Copy link
Contributor Author

I'll rebase this week

@rewritten rewritten force-pushed the verify-issuer-with-proc-and-regexp branch from 5adee48 to bb9f7b9 Compare January 13, 2022 17:19
@sourcelevel-bot
Copy link

SourceLevel has finished reviewing this Pull Request and has found:

  • 1 possible new issue (including those that may have been commented here).

See more details about this review.

@rewritten
Copy link
Contributor Author

@anakinj ready for CI

@rewritten
Copy link
Contributor Author

Should be fixed now - I have run rubocop locally and it does not complain anymore

@anakinj anakinj merged commit 8a84340 into jwt:master Jan 13, 2022
@anakinj
Copy link
Member

anakinj commented Jan 13, 2022

Thanks for your contribution @rewritten. Highly appreciated.

@rewritten
Copy link
Contributor Author

NP, count on me for further PRs if you want to offload work

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.

2 participants