omniauth / omniauth_openid_connect

MIT License
170 stars 187 forks source link

OpenIDConnect::Discovery::DiscoveryFailed when discovery is not enabled #118

Closed nTraum closed 1 year ago

nTraum commented 2 years ago

Hi there,

thanks for providing this gem, we at CitizenLab use it for two identity providers that we integrate with.

The authentication process of both providers started to fail with the error described in in #97 some time ago. We tried to fix this by upgrading to omniauth_openid_connect 0.4.0, but after the upgrade we now see the following exception being thrown:

OpenIDConnect::Discovery::DiscoveryFailed

We don't know exactly yet why this is happening, but the interesting part is that this error happens on two completely separate OpenID providers. I don't know the OpenID protocol very well, but we do not have Discovery enabled in any of the two providers, so why is this error being thrown then?

For reference, this is what the configurations of the providers look like:

# Configuration for identity provider 1
options = env['omniauth.strategy'].options
options[:scope] = %i[openid profile egovnrn]
options[:response_type] = :code
options[:state] = true
options[:nonce] = true
options[:issuer] = "https://#{host}"
options[:acr_values] = 'urn:be:fedict:iam:fas:Level450'
options[:send_scope_to_token_endpoint] = false
options[:client_options] = {
  identifier: config[:identifier],
  secret: config[:secret],
  port: 443,
  scheme: 'https',
  host: host,
  authorization_endpoint: '/fas/oauth2/authorize',
  token_endpoint: '/fas/oauth2/access_token',
  userinfo_endpoint: '/fas/oauth2/userinfo',
  redirect_uri: "#{configuration.base_backend_uri}/auth/bosa_fas/callback"
}

https://github.com/citizenlabdotco/citizenlab/blob/master/back/engines/commercial/id_bosa_fas/app/lib/id_bosa_fas/bosa_fas_omniauth.rb#L18-L36

# Configuration for identiy provider 2
options = env['omniauth.strategy'].options
options[:scope] = %i[openid run name]
options[:response_type] = :code
options[:state] = true
options[:nonce] = true
options[:issuer] = issuer
options[:send_scope_to_token_endpoint] = false
options[:client_options] = {
  identifier: config[:client_id],
  secret: config[:client_secret],
  port: 443,
  scheme: 'https',
  host: host,
  authorization_endpoint: '/openid/authorize',
  token_endpoint: '/openid/token',
  userinfo_endpoint: '/openid/userinfo',
  redirect_uri: "#{configuration.base_backend_uri}/auth/clave_unica/callback"
}

https://github.com/citizenlabdotco/citizenlab/blob/master/back/engines/commercial/id_clave_unica/app/lib/id_clave_unica/clave_unica_omniauth.rb#L21-L42

An exemplary stack trace from config 2:

JSON::ParserError: 859: unexpected token at '<!doctype html>
<html data-n-head-ssr>
  <head >
    <title>Portal ciudadano ClaveÚnica</title><meta data-n-head="ssr" charset="utf-8"><meta data-n-head="ssr" name="viewport" content="width=device-width, initial-scale=1"><meta data-n-head="ssr" data-hid="Portal ciudadano ClaveÚnica" name="Portal ciudadano ClaveÚnica" content="Portal ciudadano ClaveÚnica"><link data-n-head="ssr" rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto+Slab&amp;display=swap"><link data-n-head="ssr" rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto&amp;display=swap"><link data-n-head="ssr" rel="icon" type="image/x-icon" href="/favicon.ico"><script data-n-head="ssr" data-hid="stripe" src="https://unpkg.com/vue-recaptcha@latest/dist/vue-recaptcha.min.js" defer></script><script data-n-head="ssr" data-hid="stripe" src="https://www.google.com/recaptcha/api.js?onload=vueRecaptchaApiLoaded&amp;render=explicit" defer></script><link rel="preload" href="/_nuxt/b57117b.js" as="script"><link rel="preload" href="/_nuxt/31c1e4d.js" as="script"><link rel="preload" href="/_nuxt/e78a47a.js" as="script"><link rel="preload" href="/_nuxt/a257210.js" as="script"><link rel="preload" href="/_nuxt/136ec92.js" as="script"><style data-vue-ssr-id="71f83a6d:0 32df6d42:0 a07579c2:0 a07579c2:1 432fb0d6:0 a5041f02:0 0667d9c0:0 b2d1e312:0 3191d5ad:0">/*!
 * Bootstrap v4.6.1 (https://getbootstrap.com/)
 * Copyright 2011-2021 The Bootstrap Authors
 * Copyright 2011-2021 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/main/LICENSE)
 */:root{--blue:#007bff;--indigo:#6610f2;--purple:#6f42c1;--pink:#e83e8c;--red:#dc3545;--orange:#fd7e14;--yellow:#ffc107;--green:#28a745;--teal:#20c997;--cyan:#17a2b8;--white:#fff;--gray:#6c757d;--gray-dark:#343a40;--primary:#007bff;--secondary:#6c757d;--success:#28a745;--info:#17a2b8;--warning:#ffc107;--danger:#dc3545;--light:#f8f9fa;--dark:#343a40;--breakpoint-xs:0;--breakpoint-sm:576px;--breakpoint-md:768px;--breakpoint-lg:992px;--breakpoint-xl:1200px;--font-family-sans-serif:-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,"Helvetica Neue",Arial,"Noto Sans","Liberation Sans",sans-serif,"Apple Color Emoji","Segoe UI Emoji","Segoe UI Symbol","Noto Color Emoji";--font-family-monospace:SFMono-Regular,Menlo,Monaco,Consolas,"Liberation Mono","Courier New",monospace}*,:after,:before{box-sizing:border-box}html{font-family:sans-serif;line-height:1.15;-webkit-text-size-adjust:100%;-webkit-tap-highlight-color:rgba(0,0,0,0)}article,aside,figcaption,figure,footer,header,hgroup,main,nav,section{display:block}body{margin:0;font-family:-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,"Helvetica Neue",Arial,"Noto Sans","Liberation Sans",sans-serif,"Apple Color Emoji","Segoe UI Emoji","Segoe UI Symbol","Noto Color Emoji";font-size:1rem;font-weight:400;line-height:1.5;color:#212529;text-align:left;background-color:#fff}[tabindex="-1"]:focus:not(.focus-visible),[tabindex="-1"]:focus:not(:focus-visible){outline:0!important}hr{box-sizing:content-box;height:0;overflow:visible}h1,h2,h3,h4,h5,h6{margin-top:0;margin-bottom:.5rem}p{margin-top:0;margin-bottom:1rem}abbr[data-original-title],abbr[title]{text-decoration:underline;-webkit-text-decoration:underline dotted;text-decoration:underline dotted;cursor:help;border-bottom:0;-webkit-text-decoration-skip-ink:none;text-decoration-skip-ink:none}address{font-style:normal;line-height:inherit}address,dl,ol,ul{margin-bottom:1rem}dl,ol,ul{margin-top:0}ol ol,ol ul,ul ol,ul ul{margin-bottom:0}dt{font-weight:700}dd{margin-bottom:.5rem;margin-left:0}blockquote{margin:0 0 1rem}b,strong{font-weight:bolder}small{font-size:80%}sub,sup{position:relative;font-size:75%;line-height:0;vertical-align:baseline}sub{bottom:-.25em}sup{top:-.5em}a{color:#007bff;text-decoration:none;background-color:transparent}a:hover{color:#0056b3;text-decoration:underline}a:not([href]):not([class]),a:not([href]):not([class]):hover{color:inherit;text-decoration:none}code,kbd,pre,samp{font-family:SFMono-Regular,Menlo,Monaco,Consolas,"Liberation Mono","Courier New",monospace;font-size:1em}pre{margin-top:0;margin-bottom:1rem;overflow:auto;-ms-overflow-style:scrollbar}figure{margin:0 0 1rem}img{border-style:none}img,svg{vertical-align:middle}svg{overflow:hidden}table{border-collapse:collapse}caption{padding-top:.75rem;padding-bottom:.75rem;color:#6c757d;text-align:left;caption-side:bottom}th{text-align:inherit;text-align:-webkit-match-parent}label{display:inline-block;margin-bottom:.5rem}button{border-radius:0}button:focus:not(.focus-visible),button:focus:not(:focus-visible){outline:0}button,input,optgroup,select,textarea{margin:0;font-family:inherit;font-size:inherit;line-height:inherit}button,input{overflow:visible}button,select{text-transform:none}[role=button]{cursor:pointer}select{word-wrap:normal}[type=button],[type=reset],[type=submit],button{-webkit-appearance:button}[type=button]:not(:disabled),[type=reset]:not(:disabled),[type=submit]:not(:disabled),button:not(:disabled){cursor:pointer}[type=button]::-moz-focus-inner,[type=reset]::-moz-focus-inner,[type=submit]::-moz-focus-inner,button::-moz-focus-inner{padding:0;border-style:none}input[type=checkbox],input[type=radio]{box-sizing:border-box;padding:0}textarea{overflow:auto;resize:vertical}fieldset{min-width:0;padding:0;margin:0;border:0}legend{display:block;width:100%;max-width:100%;padding:0;margin-bottom:.5rem;font-size:1.5rem;line-height:inherit;color:inherit;white-space:normal}progress{vertical-align:baseline}[type=number]::-webkit-inner-spin-button,[type=number]::-webkit-outer-spin-button{height:auto}[type=search]{outline-offset:-2px;-webkit-appearance:none}[type=search]::-webkit-search-decoration{-webkit-appearance:none}::-webkit-file-upload-button{font:inherit;-webkit-appearance:button}output{display:inline-block}summary{display:list-item;cursor:pointer}template{display:none}[hidden]{display:none!important}.h1,.h2,.h3,.h4,.h5,.h6,h1,h2,h3,h4,h5,h6{margin-bottom:.5rem;font-weight:500;line-height:1.2}.h1,h1{font-size:2.5rem}.h2,h2{font-size:2rem}.h3,h3{font-size:1.75rem}.h4,h4{font-size:1.5rem}.h5,h5{font-size:1.25rem}.h6,h6{font-size:1rem}.lead{font-size:1.25rem;font-weight:300}.display-1{font-size:6rem}.display-1,.display-2{font-weight:300;line-height:1.2}.display-2{font-size:5.5rem}.display-3{font-size:4.5rem}.display-3,.display-4{font-weight:300;line-height:1.2}.display-4{font-size:3.5rem}hr{margin-top:1rem;margin-bottom:1rem;border:0;border-top:1px solid rgba(0,0,0,.1)}.small,small{font-size:80%;font-weight:400}.mark,mark{padding:.2em;background-color:#fcf8e3}.list-inline,.list-unstyled{padding-left:0;list-style:none}.list-inline-item{display:inline-block}.list-inline-item:not(:last-child){margin-right:.5rem}.initialism{font-size:90%;text-transform:uppercase}.blockquote{margin-bottom:1rem;font-size:1.25rem}.blockquote-footer{display:block;font-size:80%;color:#6c757d}.blockquote-footer:before{content:"\2014\00A0"}.img-fluid,.img-thumbnail{max-width:100%;height:auto}.img-thumbnail{padding:.25rem;background-color:#fff;border:1px solid #dee2e6;border-radius:.25rem}.figure{display:inline-block}.figure-img{margin-bottom:.5rem;line-height:1}.figure-caption{font-size:90%;color:#6c757d}code{font-size:87.5%;color:#e83e8c;word-wrap:break-word}a>code{color:inherit}kbd{padding:.2rem .4rem;font-size:87.5%;color:#fff;background-color:#212529;border-radius:.2rem}kbd kbd{padding:0;font-size:100%;font-weight:700}pre{display:block;font-size:87.5%;color:#212529}pre code{font-size:inherit;color:inherit;word-break:normal}.pre-scrollable{max-height:340px;overflow-y:scroll}.container,.container-fluid,.container-lg,.container-md,.container-sm,.container-xl{width:100%;padding-right:15px;padding-left:15px;margin-right:auto;margin-left:auto}@media (min-width:576px){.container,.container-sm{max-width:540px}}@media (min-width:768px){.container,.container-md,.container-sm{max-width:720px}}@media (min-width:992px){.container,.container-lg,.container-md,.container-sm{max-width:960px}}@media (min-width:1200px){.container,.container-lg,.container-md,.container-sm,.container-xl{max-width:1140px}}.row{display:flex;flex-wrap:wrap;margin-right:-15px;margin-
  from json (2.6.1) lib/json/common.rb:216:in `parse'
  from json (2.6.1) lib/json/common.rb:216:in `parse'
  from swd (1.3.0) lib/swd/resource.rb:37:in `handle_response'
  from swd (1.3.0) lib/swd/resource.rb:19:in `block in discover!'
  from swd (1.3.0) lib/swd/cache.rb:4:in `fetch'
  from swd (1.3.0) lib/swd/resource.rb:18:in `discover!'
  from openid_connect (7e5136cb3712) lib/openid_connect/discovery/provider/config.rb:7:in `discover!'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:98:in `config'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:187:in `public_key'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:234:in `decode_id_token'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:354:in `verify_id_token!'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:228:in `access_token'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:126:in `callback_phase'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:238:in `callback_call'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:189:in `call!'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:169:in `call'
  from omniauth (1.9.1) lib/omniauth/builder.rb:45:in `call'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:420:in `call_app!'
  from omniauth_openid_connect (0.4.0) lib/omniauth/strategies/openid_connect.rb:144:in `other_phase'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:190:in `call!'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:169:in `call'
  from omniauth (1.9.1) lib/omniauth/builder.rb:45:in `call'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:192:in `call!'
  from omniauth (1.9.1) lib/omniauth/strategy.rb:169:in `call'
  from omniauth (1.9.1) lib/omniauth/builder.rb:45:in `call'
  from rack-attack (6.6.1) lib/rack/attack.rb:127:in `call'
  from engines/ee/multi_tenancy/config/initializers/apartment.rb:92:in `block in call'
  from ros-apartment (2.11.0) lib/apartment/adapters/abstract_adapter.rb:89:in `switch'
  from forwardable.rb:235:in `switch'
  from engines/ee/multi_tenancy/config/initializers/apartment.rb:92:in `call'
  from rack (2.2.3.1) lib/rack/session/abstract/id.rb:266:in `context'
  from rack (2.2.3.1) lib/rack/session/abstract/id.rb:260:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/cookies.rb:689:in `call'
  from rack (2.2.3.1) lib/rack/etag.rb:27:in `call'
  from rack (2.2.3.1) lib/rack/conditional_get.rb:27:in `call'
  from rack (2.2.3.1) lib/rack/head.rb:12:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
  from activesupport (6.1.6) lib/active_support/callbacks.rb:98:in `run_callbacks'
  from actionpack (6.1.6) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/actionable_exceptions.rb:18:in `call'
  from sentry-rails (5.3.1) lib/sentry/rails/rescued_exception_interceptor.rb:12:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call'
  from sentry-ruby-core (5.3.1) lib/sentry/rack/capture_exceptions.rb:26:in `block (2 levels) in call'
  from sentry-ruby-core (5.3.1) lib/sentry/hub.rb:199:in `with_session_tracking'
  from sentry-ruby-core (5.3.1) lib/sentry-ruby.rb:351:in `with_session_tracking'
  from sentry-ruby-core (5.3.1) lib/sentry/rack/capture_exceptions.rb:17:in `block in call'
  from sentry-ruby-core (5.3.1) lib/sentry/hub.rb:59:in `with_scope'
  from sentry-ruby-core (5.3.1) lib/sentry-ruby.rb:331:in `with_scope'
  from sentry-ruby-core (5.3.1) lib/sentry/rack/capture_exceptions.rb:16:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
  from rails_semantic_logger (4.10.0) lib/rails_semantic_logger/rack/logger.rb:43:in `call_app'
  from rails_semantic_logger (4.10.0) lib/rails_semantic_logger/rack/logger.rb:28:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/request_id.rb:26:in `call'
  from rack (2.2.3.1) lib/rack/runtime.rb:22:in `call'
  from activesupport (6.1.6) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/executor.rb:14:in `call'
  from rack (2.2.3.1) lib/rack/sendfile.rb:110:in `call'
  from actionpack (6.1.6) lib/action_dispatch/middleware/host_authorization.rb:142:in `call'
  from rack-cors (1.1.1) lib/rack/cors.rb:100:in `call'
  from railties (6.1.6) lib/rails/engine.rb:539:in `call'
  from puma (5.6.4) lib/puma/configuration.rb:252:in `call'
  from puma (5.6.4) lib/puma/request.rb:77:in `block in handle_request'
  from puma (5.6.4) lib/puma/thread_pool.rb:340:in `with_force_shutdown'
  from puma (5.6.4) lib/puma/request.rb:76:in `handle_request'
  from puma (5.6.4) lib/puma/server.rb:441:in `process_client'
  from puma (5.6.4) lib/puma/thread_pool.rb:147:in `block in spawn_thread'

The stack trace seems to indicate that an HTML page is being parsed where JSON is expected. Is this a configuration issue on our side?

Thanks in advance for any hints. We'll investigate further from our side as well and will update the issue in case of any revelations.

hbrmedia commented 2 years ago

Without Discovery You should provide client_signing_algand client_jwk_signing_key in options, otherwise it will fallback to discovery even you provide false to discovery option.

With Discovery Provide jwks_uri to keys json or "#{options[:issuer]}/.well-known/openid-configuration" should return valid json with configuration, including jwks_uri

ssoulless commented 2 years ago

Why aren't client_signing_alg and client_jwk_signing_key in the docs of the gem?

stanhu commented 1 year ago

This should be fixed in 0.5.0 via https://github.com/omniauth/omniauth_openid_connect/pull/133 if discovery is disabled.