aspnet / Templates

This repo is OBSOLETE - please see the README file for information
Other
150 stars 57 forks source link

What Identity LogOff behavior should templates (and samples) adopt? #831

Closed guardrex closed 7 years ago

guardrex commented 7 years ago

TL;DR https://github.com/aspnet/Docs/issues/2867 (big time :smile: ... I'll summarize the issue here)

The Introduction to Identity doc sample might have been derived from the Identity sample and that sample's AccountController, which uses this LogOff action ...

[HttpPost]
[ValidateAntiForgeryToken]
public async Task<IActionResult> LogOff()
{
    await _signInManager.SignOutAsync();
    _logger.LogInformation(4, "User logged out.");
    return RedirectToAction(nameof(HomeController.Index), "Home");
}

The doc sample shows how to set the LogoutPath ...

options.Cookies.ApplicationCookie.LogoutPath = "/Account/LogOff";

However, no returnUrl request parameter (setting the Location header) is going to be honored with the RedirectToAction present on the controller action, which will win battle on where the browser should go after a logoff ... it will go Home.

In the TL;DR issue, @HaoK said:

I believe the original idea is that its trying to keep you on the same page you were at, so if you were in About, and click Logoff, that will generate a returnUrl to /About, and will redirect you back there.

But yeah the templates today no longer seem to support that scenario, I just tried it and it just sends you back to Home... so its either a bug (or the new behavior). You could file a bug in the templates repro asking what the desired behavior is as well...

The options are probably to either ...

  1. Don't set LogoutPath if the controller action is going to force a RedirectToAction, or ...
  2. Drop the RedirectToAction in favor of return new EmptyResult(); and include an asp-route-returnUrl="/" on the <form> tag. However, note that the Identity sample and the doc have no Views; therefore, there's no way via the current sample and doc to show an asp-route-returnUrl attribute.

Thanks to @PinpointTownes for helpful insight.

cc/ @Rick-Anderson @danroth27 @blowdart

grokky1 commented 7 years ago

@guardrex I read that other looooong issue, and this one, and I still don't get it.

If you want after a logout, to be redirected to "/thanks", then must you do

Even now after release of ASP.NET Core 2.0 there is no proper documentation for this anywhere, and the samples are outdated. It seems like you really understand this, so do you mind letting us know how you do it?

guardrex commented 7 years ago

@grokky1 I've only been working on docs since March and not on actual enterprise apps. I haven't had to face this issue with an app since we discussed it here; so actually, I don't have additional general advice.

In your case tho, it seems like your option 3 is clean. As you suggest, you can just manually do it in LogOff (or Logout) by physically setting the RedirectToAction to go to your /thanks view. Nice and clean, as long as you never need to use returnUrl to go anywhere else. They will always be "thanked for visiting." :smile:

guardrex commented 7 years ago

I haven't checked out the new identity stuff (yet!) in 2.0. I do notice tho that they're still doing the same thing with the web starter template: https://github.com/aspnet/Templates/blob/dev/template_feed/Microsoft.DotNet.Web.ProjectTemplates.2.0/content/StarterWeb-CSharp/Controllers/AccountController.cs#L67

danroth27 commented 7 years ago

@blowdart @HaoK to provide guidance

blowdart commented 7 years ago

I like the idea of returning via returnUrl.

Eilon commented 7 years ago

This issue was moved to aspnet/templating#88