Fix up how OIDC errors flow

[Tratcher]

#4384 @PinpointTownes
This tweaks the new AccessDenied feature (aspnet/Security#1887) to avoid changing the error and losing fidelity for the OIDC scenario.

[HaoK]

Can we add some basic tests, this already regressed once.

[Tratcher]

We don't even have the infrastructure set up yet... https://github.com/aspnet/Security/issues/772

I'd like to test it with the template test that caught the regression but I think we'd have to check in for that to run.

[kevinchalet]

#4384 @PinpointTownes
This tweaks the new AccessDenied feature (aspnet/Security#1887) to avoid changing the error and losing fidelity for the OIDC scenario.

Interesting case (that I had obviously not taken into account 😄)

I guess what you have now is fine, but I'm tempted to think it would be a good idea to also fix the B2C code to use the new event (instead of determining whether the exception message of OpenIdConnectProtocolException contains a magical code, which really looks weird).

[Tratcher]

Yes B2C could use the new AccessDenied event, but it would still end up checking for a specific error code associated with password resets.

[HaoK]

I think this should drive adding the infrastructure to at least add a test for this regression

[Tratcher]

@HaoK let's merge this to unblock preview2.

[HaoK]

Well constantly punting on test coverage in OIDC is what got us to this place, can't you just add something basic at least for the regression flow?

[HaoK]

How can we even be sure this fixes the issue for example with no tests?

[Tratcher]

I did find some tests for this scenario here: https://github.com/aspnet/AspNetCore/blob/c7d63649005c4c2863b34794994eef57595e7bb7/src/AADIntegration/test/Microsoft.AspNetCore.Authentication.AzureADB2C.UI.Test/AzureAdB2COpenIDConnectEventHandlersTests.cs#L99-L113

They hadn't failed because they were mocking the RemoteFailureContext. Since B2C is relying on specific types and strings I would only expect a behavior change like this to be caught by a functional test in B2C. I'll try writing one there.

[HaoK]

Sounds good to me, yeah just one regression test in the b2c stuff as part of this PR to show that we fixed something and will catch at least this regression in the future works

[Tratcher]

Updated