Support response_type: ["code", "id_token"]
#139
Conversation
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
Co-authored-by: Stan Hu <[email protected]>
|
It looks like there are some minor Rubocop failures here. |
|
@stanhu I've pushed up changes to resolve the Rubocop violations. |
| @@ -372,16 +372,18 @@ def id_token_callback_phase | |||
| end | |||
|
|
|||
| def valid_response_type? | |||
| return true if params.key?(configured_response_type) | |||
| return true if configured_response_types.all? { |key| params[key].present? } | |||
There was a problem hiding this comment.
In looking at https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#MultiValueResponseTypes and https://www.rfc-editor.org/rfc/rfc6749 closely, is this the correct mapping?
response_type |
Expected parameters |
|---|---|
code token |
code, access_token, token_type |
code id_token |
code, id_token |
id_token token |
id_token, access_token, token_type |
code id_token token |
code, id_token, access_token, token_type |
There was a problem hiding this comment.
So while this change is correct for code id_token, I think we probably want to handle these cases here. It looks to me that every time token shows up, access_token and token_type come along.
There was a problem hiding this comment.
Thank you for laying this out @stanhu. I don't have the bandwidth to round out the other combinations, but this changeset covers our use case.
Could we merge this change, then subsequently cover the rest?
If not, feel free to take this over.
Thank you!
Closes omniauth/omniauth_openid_connect#105
Similar to omniauth/omniauth_openid_connect#107
Some OpenID compatible IdP support hybrid authorizations that accept a
response_typewith bothcodeandid_token.For example, Microsoft Azure B2C accepts them as a URL-encoded array:
This commit extends the
OmniAuth::Strategies::OpenIDConnectto encode theresponse_typeinto 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.