-
Notifications
You must be signed in to change notification settings - Fork 597
Add response_mode=query support for OpenID Connect #279
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,11 +139,19 @@ protected override async Task<bool> HandleUnauthorizedAsync([NotNull] ChallengeC | |
// [brentschmaltz] - #215 this should be a property on RedirectToIdentityProviderNotification not on the OIDCMessage. | ||
RequestType = OpenIdConnectRequestType.AuthenticationRequest, | ||
Resource = Options.Resource, | ||
ResponseMode = Options.ResponseMode, | ||
ResponseType = Options.ResponseType, | ||
Scope = Options.Scope | ||
}; | ||
|
||
// Omitting the response_mode parameter when it already corresponds to the default | ||
// response_mode used for the specified response_type is recommended by the specifications. | ||
// See http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes | ||
if (!string.Equals(Options.ResponseType, OpenIdConnectResponseTypes.Code, StringComparison.Ordinal) || | ||
!string.Equals(Options.ResponseMode, OpenIdConnectResponseModes.Query, StringComparison.Ordinal)) | ||
{ | ||
message.ResponseMode = Options.ResponseMode; | ||
} | ||
|
||
if (Options.ProtocolValidator.RequireNonce) | ||
{ | ||
message.Nonce = Options.ProtocolValidator.GenerateNonce(); | ||
|
@@ -236,8 +244,22 @@ protected override async Task<AuthenticationTicket> HandleAuthenticateAsync() | |
|
||
OpenIdConnectMessage message = null; | ||
|
||
if (string.Equals(Request.Method, "GET", StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
message = new OpenIdConnectMessage(Request.Query); | ||
|
||
// response_mode=query (explicit or not) and a response_type containing id_token | ||
// or token are not considered as a safe combination and MUST be rejected. | ||
// See http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#Security | ||
if (!string.IsNullOrWhiteSpace(message.IdToken) || !string.IsNullOrWhiteSpace(message.Token)) | ||
{ | ||
Logger.LogError("An OpenID Connect response cannot contain an identity token " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll move the hardcoded message to the .resx file when we're done with the other changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Eilon is moving logging messages to .resx still the recommended approach? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be in a resource file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I asked @Eilon if adding logging messages to the .resx file was still the recommended approach and got no answer 😄 I'll fix that, thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure we don't localize log messages. They are currently intended to be English only because there's no way to do localized logs. The problem with having localized log messages is which culture do you use for them? Surely you don't want to use the current thread's culture, because that is usually set to whatever the end user's culture is. So if a French user visits, I get French logs. And the next user is German, so the next log is German. Clearly that is not going to work. So in the meantime we decided to not localize them. |
||
"or an access token when using response_mode=query"); | ||
return null; | ||
} | ||
} | ||
// assumption: if the ContentType is "application/x-www-form-urlencoded" it should be safe to read as it is small. | ||
if (string.Equals(Request.Method, "POST", StringComparison.OrdinalIgnoreCase) | ||
else if (string.Equals(Request.Method, "POST", StringComparison.OrdinalIgnoreCase) | ||
&& !string.IsNullOrWhiteSpace(Request.ContentType) | ||
// May have media/type; charset=utf-8, allow partial match. | ||
&& Request.ContentType.StartsWith("application/x-www-form-urlencoded", StringComparison.OrdinalIgnoreCase) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenIdConnectResponseTypes.CodeOnly
has been renamed toOpenIdConnectResponseTypes.Code
.AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#156