Add response_mode=query support for OpenID Connect

[ryan-w]

This is a backport from AspNetCore.

[dnfclas]

All CLA requirements met.

[Tratcher]

FYI

• I know we don't have good OIDC test coverage. I'll do some manual verification as part of the review.
• We don't have our next release scheduled. I'll review and likely merge this, but we don't know when it will ship.

[ryan-w]

@Tratcher Thank you. Once it's merged, I can work from the dev feed until it's released.

[Tratcher]

This feature is incomplete. What provider and configuration did you use to test it?

It has the check that prohibits IdTokens in the query string, but later it has a comment and check that says codes can only be provided with IdTokens. There's also no automatic code redemption (heince the need for the IdToken), only the AuthorizationCodeReceived notification the caller needs to implement. Adding automatic code redemption is a larger feature but I don't think this PR is useful without it.

AspNetKatana/src/Microsoft.Owin.Security.OpenIdConnect/OpenidConnectAuthenticationHandler.cs

Lines 258 to 264 in 40de801

	// code is only accepted with id_token, in this version, hence check for code is inside this if
	// OpenIdConnect protocol allows a Code to be received without the id_token
	if (string.IsNullOrWhiteSpace(openIdConnectMessage.IdToken))
	{
	_logger.WriteWarning("The id_token is missing.");
	return null;
	}

[ryan-w]

@Tratcher Good point. After building and referencing the locally built Nuget packages I receive error messages like this.

Error CS0012 The type 'IDataProtectionProvider' is defined in an assembly that is not referenced. You must add a reference to assembly 'Microsoft.Owin.Security, Version=2.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'.

It seems that other assemblies are looking for the signed version of this reference and assembly redirecting does not work. I have so far not been able to work past this to test with my local build. Can you point me in the right direction to get this running locally?

[Tratcher]

No need for packages, you can use this project https://github.com/aspnet/AspNetKatana/blob/dev/tests/Katana.Sandbox.WebServer/Startup.cs to run and debug code in the repo. Feel free to comment out unrelated parts like the social providers.

[ryan-w]

I just pushed an update to include the automatic code redemption.

[RupW]

Brilliant, thanks! One issue: you're still calling Options.ProtocolValidator.ValidateAuthenticationResponse even when openIdConnectMessage is now a token response, which gives me a validation failure.

IDX21335: 'refresh_token' cannot be present in a response message received from Authorization Endpoint.

But otherwise works against django-oidc-provider, which ignores response_mode (and never POSTs) and only looks viable with response_type=code.

Coincidentally I've spent the day trying to hack Owin 3.1.0 OpenIdConnect into working with django-oidc-provider, but I didn't know what the problems were when I set out, and your version is /much/ neater. (And I hit different validator problems, since the logic was different back then.) I did copy the backchannel request pattern from the Google auth provider which passes the HttpClient in to the handler's constructor rather than putting it in Options, but I like Options better.

[ryan-w]

@RupW Thanks. I'll take a look at that validation issue.

[ryan-w]

I just pushed a fix for the validation issue.

[ryan-w]

@Tratcher This should be good to go now. Please let me know if you see any issues.

[Tratcher]

Understood, I should get back to it next week.

[Tratcher]

Sorry, something got moved ahead of this, it may be another week or two.

[ryan-w]

The identity provider for some applications that I manage has recently changed (outside of my control) and does not support the form post method. Users currently are not able to sign-in to the applications until I get this update in place. Is there a way to get around the issue of needing signed binaries so I can start implementing these changes right away?

[Tratcher]

As a short term measure you can change the nuspec and csproj to point to stable dependencies:
https://github.com/aspnet/AspNetKatana/blob/dev/src/Microsoft.Owin.Security.OpenIdConnect/Microsoft.Owin.Security.OpenIdConnect.nuspec

AspNetKatana/src/Microsoft.Owin.Security.OpenIdConnect/Microsoft.Owin.Security.OpenIdConnect.csproj

Line 110 in 40de801

<ProjectReference Include="..\Microsoft.Owin.Security\Microsoft.Owin.Security.csproj">

That should allow you to build and run just this package.

[ryan-w]

Ok, thanks. I will give that a try.

[ryan-w]

@Tratcher I’m still having issues with the references. Can you work this in this week?

[ryan-w]

Hey Chris, the last commit should address all the items in the change request. Please let me know if you have any additional feedback so we can keep this moving forward. Thanks!

[Tratcher]

Much better.

Why is the nonce handled differently from Core?

[Tratcher]

Thanks for working through that with us.

[Tratcher]

FYI: Packages should be updated on myget in about an hour (build 1495).

[ryan-w]

You're welcome. Thanks for your help.

[Hobray]

@Tratcher Any idea when the pull @ryan-w made will be in preview on Nuget? I have a scenario where the code flow auth model is the only option I have and the built in code exchange/redemption is super helpful.

[Tratcher]

@Hobray it's scheduled for this month. In the meantime you can get it on Dev myget feed if you want to test it. https://github.com/aspnet/AspNetKatana#nightly-build-feeds

[Tratcher]

Clarification, we're not planning a nuget.org preview, only myget.org. We plan to release the final 4.1.0 build this month.

[Hobray]

@Tratcher Perfect. I'm already using it in a slightly more unconventional way but would clearly prefer to align with an actual release. Knowing that there will be a final build this month is excellent. Thank you.