dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.36k stars 9.99k forks source link

OIDC should verify response_mode #5113

Open Tratcher opened 5 years ago

Tratcher commented 5 years ago

"Only if supported_response_modes is present in the discovery doc and form_post is listed there, should the RP ever try to use it. If our RP code is sending response_mode=form_post when the IdP doesn’t support that, that’s a bug in our RP code, and needs to be fixed." ~Mike Jones, OIDC spec master

Warn or fail if the OIDCOptions.ResponseMode value isn't listed in the discovery doc. Note we default to 'form_post' which isn't required to be supported in the spec. The spec default is 'query'.

https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes

@blowdart

kevinchalet commented 5 years ago

It's worth noting that the OIDC middleware uses response_type=id_token by default, which forces using response_type=form_post as neither query (forbidden) nor fragment (not applicable to server-side components) are usable in this case.

This kind of check would only make sense with the authorization code flow (response_type=code), not with implicit or hybrid.

Tratcher commented 5 years ago

That's why the suggested behavior is to warn or fail, not to downgrade. This came up when we found a server that didn't support form_post and resulted in some very odd behavior.

kevinchalet commented 5 years ago

My point is that failing sounds dangerous for a default (and de facto almost mandatory) option like response_mode=form_post, as an OAuth2/OIDC server doesn't have to return the list of response modes it supports.

If response_modes_supported is missing, a server is assumed to support both query and fragment and nothing else. If you decide to throw, then you may break implementations that support form_post but don't explicitly disclose it as part of their discovery document or clients that manually create the OpenIdConnectConfiguration instance.

kevinchalet commented 5 years ago

That's why the suggested behavior is to warn or fail, not to downgrade.

Even if you wanted to, you couldn't, as all/most OIDC servers would reject your request if it used an invalid response_mode/response_type combination.

This came up when we found a server that didn't support form_post and resulted in some very odd behavior.

Out of curiosity, what did it do? If an OAuth2/OIDC server doesn't support the response_mode specified by the client, it should very likely return an explicit error stating a parameter is invalid or not supported.

Eilon commented 5 years ago

Here's the proposal:

Eilon commented 5 years ago

Moving to backlog because we can't get this breaking change into preview5.