omniauth / omniauth_openid_connect

MIT License
170 stars 187 forks source link

Implicit Flow does not work #105

Open hhorikawa opened 2 years ago

hhorikawa commented 2 years ago

Test sample application: https://gitlab.com/netsphere/rails-omniauth-oidc-rp-sample/ Ruby 3.0, Rails 6.1, OmniAuth 2.0 The Code Flow is OK. But the Implicit Flow does not seem to work. I'll investigate the cause.

undefined method `[]' for nil:NilClass
fail!(error_attrs[:key], error_attrs[:exception_class].new(params['error']))

omniauth_openid_connect (0.4.0.pre) lib/omniauth/strategies/openid_connect.rb:337:in `valid_response_type?'
omniauth_openid_connect (0.4.0.pre) lib/omniauth/strategies/openid_connect.rb:114:in `callback_phase'
omniauth (2.0.4) lib/omniauth/strategy.rb:272:in `callback_call'
omniauth (2.0.4) lib/omniauth/strategy.rb:194:in `call!'
omniauth (2.0.4) lib/omniauth/strategy.rb:169:in `call'
omniauth (2.0.4) lib/omniauth/strategy.rb:470:in `call_app!'
omniauth_openid_connect (0.4.0.pre) lib/omniauth/strategies/openid_connect.rb:143:in `other_phase'
omniauth (2.0.4) lib/omniauth/strategy.rb:195:in `call!'
omniauth (2.0.4) lib/omniauth/strategy.rb:169:in `call'
omniauth (2.0.4) lib/omniauth/strategy.rb:470:in `call_app!'
omniauth_openid_connect (0.4.0.pre) lib/omniauth/strategies/openid_connect.rb:143:in `other_phase'
omniauth (2.0.4) lib/omniauth/strategy.rb:195:in `call!'
omniauth (2.0.4) lib/omniauth/strategy.rb:169:in `call'
omniauth (2.0.4) lib/omniauth/strategy.rb:470:in `call_app!'
omniauth_openid_connect (0.4.0.pre) lib/omniauth/strategies/openid_connect.rb:143:in `other_phase'
omniauth (2.0.4) lib/omniauth/strategy.rb:195:in `call!'
omniauth (2.0.4) lib/omniauth/strategy.rb:169:in `call'
omniauth (2.0.4) lib/omniauth/strategy.rb:470:in `call_app!'
omniauth_openid_connect (0.4.0.pre) lib/omniauth/strategies/openid_connect.rb:143:in `other_phase'
omniauth (2.0.4) lib/omniauth/strategy.rb:195:in `call!'
omniauth (2.0.4) lib/omniauth/strategy.rb:169:in `call'
omniauth (2.0.4) lib/omniauth/builder.rb:45:in `call'
rack (2.2.3) lib/rack/tempfile_reaper.rb:15:in `call'
rack (2.2.3) lib/rack/etag.rb:27:in `call'
rack (2.2.3) lib/rack/conditional_get.rb:40:in `call'
rack (2.2.3) lib/rack/head.rb:12:in `call'
actionpack (6.1.4.1) lib/action_dispatch/http/permissions_policy.rb:22:in `call'
actionpack (6.1.4.1) lib/action_dispatch/http/content_security_policy.rb:18:in `call'
rack (2.2.3) lib/rack/session/abstract/id.rb:266:in `context'
rack (2.2.3) lib/rack/session/abstract/id.rb:260:in `call'
actionpack (6.1.4.1) lib/action_dispatch/middleware/cookies.rb:689:in `call'
activerecord (6.1.4.1) lib/active_record/migration.rb:601:in `call'
actionpack (6.1.4.1) lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
activesupport (6.1.4.1) lib/active_support/callbacks.rb:98:in `run_callbacks'
actionpack (6.1.4.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
actionpack (6.1.4.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (6.1.4.1) lib/action_dispatch/middleware/actionable_exceptions.rb:18:in `call'
actionpack (6.1.4.1) lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call'
web-console (4.2.0) lib/web_console/middleware.rb:132:in `call_app'
hhorikawa commented 2 years ago

I have investigated this problem. The omniauth/omniauth_openid_connect repository version accepts only code and id_token as response_type. In the Implicit flow, the id_token makes the IdP return only the id_token as the authentication response. It works correctly only with some IdPs that return it extended from the specification. For example, Azure AD adds additional fields that identify the user, such as an email address. However, Yahoo JAPAN, which strictly meets the specifications, does return only minimal fields as the authentication response, and the client must request the user information using the access token. For such IdPs, you must specify ['id_token', 'token'] as the response_type. So, omniauth/omniauth_openid_connect should be able to accept it as the response_type.

ThisIsMissEm commented 1 year ago

I'd probably argue that you shouldn't support implicit flow, as it's largely considered insecure and outdated: https://www.ietf.org/archive/id/draft-ietf-oauth-security-topics-24.html#name-implicit-grant