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

Fall back to the discovered jwks when no key specified #97

Merged

Conversation

andrewshadura
Copy link
Contributor

@andrewshadura andrewshadura commented Sep 25, 2021

When no key has been explicitly specified, key_or_secret may return an empty value, leading to a token decoding error, when discovery is disabled:

JSON::JWS::VerificationFailed (JSON::JWS::VerificationFailed):
  /usr/lib/ruby/vendor_ruby/json/jws.rb:26:in `verify!'
  /usr/lib/ruby/vendor_ruby/json/jws.rb:149:in `decode_compact_serialized'
  /usr/lib/ruby/vendor_ruby/json/jwt.rb:86:in `decode_compact_serialized'
  /usr/lib/ruby/vendor_ruby/json/jose.rb:52:in `decode'
  ...

The actual discovery is performed in any case (but the discovered values aren’t used), so why not use that key if nothing else is available.

When no key has been explicitly specified, key_or_secret may return an
empty value, so when discovery is disabled, JWT decoding fails:

    JSON::JWS::VerificationFailed (JSON::JWS::VerificationFailed):
      /usr/lib/ruby/vendor_ruby/json/jws.rb:26:in `verify!'
      /usr/lib/ruby/vendor_ruby/json/jws.rb:149:in `decode_compact_serialized'
      /usr/lib/ruby/vendor_ruby/json/jwt.rb:86:in `decode_compact_serialized'
      /usr/lib/ruby/vendor_ruby/json/jose.rb:52:in `decode'
      ...

The actual discovery is performed in any case, so why not use that key if
nothing else is available.

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
@m0n9oose m0n9oose merged commit 1cfda54 into omniauth:master Nov 18, 2021
nTraum added a commit to CitizenLabDotCo/citizenlab that referenced this pull request Jun 9, 2022
…d Bosa FAS (Belgium)

This is an attempt to fix the verification error we are seeing in Sentry for Bosa FAS and Clave Unica.

Specifically I'm eyeballing the fix introduced in 0.4 here omniauth/omniauth_openid_connect#97. The stack trace matches what we see in Sentry:

https://sentry.hq.citizenlab.co/organizations/citizenlab/issues/45980/?referrer=jira_integration
https://sentry.hq.citizenlab.co/organizations/citizenlab/issues/46899/?project=2&query=is%3Aunresolved&statsPeriod=14d

  This is hard to test locally because the documentation at https://www.notion.so/citizenlab/SSO-ID-Verification-263b705ca14e472c8e41b444663e373a#1da11c08825c441687faf4be31fb14f8 is lacking and far as I know we do not have a working sandbox environment for any of the two verification methods. I'd therefore like to blindly deploy it to production and ask Pablo again to verify on a Chilean platform for me.
papayaah added a commit to health2works/omniauth_openid_connect that referenced this pull request Oct 7, 2022
…0.4.0

* upstream/master: (32 commits)
  Fix date in CHANGELOG.md
  Fixes rubocop issues
  Setup Coveralls.io as Github Action
  Fix rubocop errors
  Replace deprecated rubocop_linter_action
  Update README.md
  Fix date on CHANGELOG.md
  Update VERSION to v0.4.0
  Update CHANGELOG.md for v0.4.0
  Support dynamic parameters to the authorize URI (omniauth#90)
  Upgrade Faker and replace Travis with Github Actions (omniauth#102)
  Try to fix rubocop documentation_url not round error.
  Make `omniauth_openid_connect` gem compatible with `omniauth v2.0` (omniauth#95)
  Fall back to the discovered jwks when no key specified (omniauth#97)
  Allow to update omniauth to 2 (omniauth#88)
  Update README.md
  Bump version to 0.3.5
  bugfix: info from decoded id_token is not exposed (omniauth#61)
  bugfix: NoMethodError (undefined method `count' for #<OpenIDConnect::ResponseObject::IdToken:0x0000000008d9dde0>): (omniauth#60)
  Bump version to 0.3.4
  ...
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