return ReturnUrl when ExternalLogin fails

[jbagga]

Addresses #97

[jbagga]

cc @javiercn

[javiercn]

Just a quick question. Is the returnUrl the same one when a user gets to the login endpoint for the first time than it is when it gets redirected back to the login endpoint after failing external authentication?

If that's the case, go ahead, but if the returnUrl changes we need to look at those differences.

It's also important to test that you can start a log-in, fail external auth and then log-in successfully with external auth so as to ensure we are fixing the scenario and not running into an issue later in it.

Does this make sense?

[Eilon]

@javiercn can you work with @jbagga on how to test this?

[javiercn]

@Eilon Sure. No problem

[javiercn]

I'm signing off for now, because testing this is not trivial. But @HaoK should review it.

[HaoK]

Looks fine, I think you can probably test it by munging the registered app path for the oauth you set up so it comes back with an error instead of a redirect, like if register the wrong localhost port for your web app with google/facebook

[HaoK]

Or if that doesn't work, you can maybe remove something like email from the permissions for google, which I think also causes errors since we always ask for email

[jbagga]

The ExternLoginCallback action is not hit if I try the first suggestion. The external login provider errors out and does not return back to my app. Pressing back on the browser takes me back to the login page.

The second suggestion cannot be configured. At least I can't find how to remove email from permissions for Google or Facebook

[javiercn]

@HaoK Can't you configure scopes for Google? It should be easy enough to put an incorrect value for the scopes on Google and use that to trigger an alert.

[HaoK]

The error flow well end up looking like the other two bugs you filed with fb/twitter when you hit cancel I think.

basically you can add a new scope, like the plus.me like so:

               o.Scope.Add("https://www.googleapis.com/auth/plus.login");

That will prompt the user to allow or deny, and if you deny it causes and error but that ends up showing up with an access denied exception on the signin-google request as well.

But looking at this a bit more, I don't think there's any path that leads to the ExternalLoginCallback action getting called with a remoteError, as the built in logic throws an exception, unless they handle the repsonse themselves: https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs#L102

[HaoK]

So if you just want to test GetExternalLoginInfo failing, you can just delete the Identity.ExternalCookie which would result in that call failing, if you don't have a chance to manually do it, you can just add some code in a middleware at the end that calls SignOut on every request on that scheme.

[javiercn]

@HaoK Can you give @jbagga a sample in a gist?
I'm failing to understand too how the middleware goes from receiving the error on the callback to redirecting to the endpoint.

[jbagga]

@javiercn @Eilon

Since there is no

path that leads to the ExternalLoginCallback action getting called with a remoteError, as the built in logic throws an exception,

I tested the other path where GetExternalLoginInfo returns null and the return url is preserved

[HaoK]

Should we just remove that remoteError parameter/logic since there's no way to trigger it anymore as far as we can tell?

[javiercn]

@HaoK I'm ok with that, but you are the expert in here. Could you take a look in depth to make sure that's the case? Should we file a bug in identity for this to track it? (even though is template code).
I think at least it deserves some investigation by someone more familiar with the area before we take action.

[HaoK]

Sure file a bug in security, we have one which is tracking the error flows already aspnet/Security#1165 but this one probably deserves a separate issue at least initially

[jbagga]

Filed a bug here aspnet/Security#1571