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.

Sign up for GitHub

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

Jump to bottom

Implicit Flow does not work #105

Open
hhorikawa opened this issue Feb 2, 2022 · 2 comments · May be fixed by #139
Open

Implicit Flow does not work #105

hhorikawa opened this issue Feb 2, 2022 · 2 comments · May be fixed by #139
Assignees
@hhorikawa
Labels
enhancement

Comments

@hhorikawa
Copy link

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 hhorikawa self-assigned this Feb 2, 2022
@hhorikawa hhorikawa linked a pull request Feb 9, 2022 that will close this issue
The Implicit Flow: Part 1 #107
Closed
@hhorikawa hhorikawa removed a link to a pull request Feb 9, 2022
The Implicit Flow: Part 1 #107
Closed
@hhorikawa hhorikawa linked a pull request Feb 23, 2022 that will close this issue
The Implicit Flow: Part 1 #107
Closed
@hhorikawa
Copy link
Author

hhorikawa commented May 8, 2022 •
edited
Loading

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.

seanpdoyle added a commit to seanpdoyle/omniauth_openid_connect that referenced this issue Jan 4, 2023
@seanpdoyle
Support response_type: ["code", "id_token"]
10ca535 
Closes [omniauth#105][]
Similar to [omniauth#107][]

Some OpenID compatible IdP support hybrid authorizations that accept a
`response_type` with both `code` and `id_token`.

For example, [Microsoft Azure B2C][] accepts them as a URL-encoded
array:

> `response_type`: Must include an ID token for OpenID Connect. If your web application also needs tokens for calling a web API, you can use `code+id_token`.

This commit extends the `OmniAuth::Strategies::OpenIDConnect` to encode
the `response_type` into the query parameter as space-delimited token
list when provided as an array. Similarly, when checking for missing
keys in the response, iterate over the values as if they're an array.

For the originally supported single-value case, the previous behavior is
maintained.

[Microsoft Azure B2C]: https://learn.microsoft.com/en-us/azure/active-directory-b2c/openid-connect#send-authentication-requests
[omniauth#105]: omniauth#105
[omniauth#107]: omniauth#107
seanpdoyle added a commit to seanpdoyle/omniauth_openid_connect that referenced this issue Jan 4, 2023
@seanpdoyle
Support response_type: ["code", "id_token"]
7965667 
Closes [omniauth#105][]
Similar to [omniauth#107][]

Some OpenID compatible IdP support hybrid authorizations that accept a
`response_type` with both `code` and `id_token`.

For example, [Microsoft Azure B2C][] accepts them as a URL-encoded
array:

> `response_type`: Must include an ID token for OpenID Connect. If your web application also needs tokens for calling a web API, you can use `code+id_token`.

This commit extends the `OmniAuth::Strategies::OpenIDConnect` to encode
the `response_type` into the query parameter as space-delimited token
list when provided as an array. Similarly, when checking for missing
keys in the response, iterate over the values as if they're an array.

For the originally supported single-value case, the previous behavior is
maintained.

[Microsoft Azure B2C]: https://learn.microsoft.com/en-us/azure/active-directory-b2c/openid-connect#send-authentication-requests
[omniauth#105]: omniauth#105
[omniauth#107]: omniauth#107
seanpdoyle added a commit to seanpdoyle/omniauth_openid_connect that referenced this issue Jan 4, 2023
@seanpdoyle
Support response_type: ["code", "id_token"]
09af6dc 
Closes [omniauth#105][]
Similar to [omniauth#107][]

Some OpenID compatible IdP support hybrid authorizations that accept a
`response_type` with both `code` and `id_token`.

For example, [Microsoft Azure B2C][] accepts them as a URL-encoded
array:

> `response_type`: Must include an ID token for OpenID Connect. If your web application also needs tokens for calling a web API, you can use `code+id_token`.

This commit extends the `OmniAuth::Strategies::OpenIDConnect` to encode
the `response_type` into the query parameter as space-delimited token
list when provided as an array. Similarly, when checking for missing
keys in the response, iterate over the values as if they're an array.

For the originally supported single-value case, the previous behavior is
maintained.

[Microsoft Azure B2C]: https://learn.microsoft.com/en-us/azure/active-directory-b2c/openid-connect#send-authentication-requests
[omniauth#105]: omniauth#105
[omniauth#107]: omniauth#107
@seanpdoyle seanpdoyle linked a pull request Jan 4, 2023 that will close this issue
Open
@ThisIsMissEm
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hhorikawa hhorikawa

Labels
enhancement
Projects
None yet
Milestone
No milestone
2 participants
@ThisIsMissEm @hhorikawa