dotnet / Scaffolding

Code generators to speed up development.
MIT License
640 stars 228 forks source link

Blazor server app with authorization, after scaffold identity - logout not working #1287

Closed danroth27 closed 4 years ago

danroth27 commented 4 years ago

From @sikira on Thursday, December 12, 2019 7:57:30 PM

In blazor server app with authorization, after scaffold identity into an MVC project with authorization, user can't logout from blazor ( LoginDisplay.razor ). When user click on logout button in LoginDisplay.razor, it makes bad request

Request URL:https://localhost:5001/Identity/Account/LogOut Request Method:POST Remote Address:127.0.0.1:5001 Status Code:400 ( Bad Request) Version:HTTP/2.0

after this bad POST request :

Using this documentation. https://docs.microsoft.com/en-us/aspnet/core/security/authentication/scaffold-identity?view=aspnetcore-2.2&tabs=netcore-cli#scaffold-identity-into-an-mvc-project-with-authorization

To Reproduce

  1. dotnet new blazorserver --auth Individual
  2. create new user for testing ( user@user.com / Pass12345! )
  3. login and logout and it's working
  4. install if not already ( dotnet tool install --global dotnet-aspnet-codegenerator --version 3.1.0 )
  5. add package to project | dotnet add package Microsoft.VisualStudio.Web.CodeGeneration.Design --version 3.1.0
  6. add package to project | dotnet add package Microsoft.EntityFrameworkCore.SqlServer --version 3.1.0
  7. do a scaffold | dotnet aspnet-codegenerator identity -dc BlazorScaffoldedIdentity.Data.ApplicationDbContext --force
  8. logout from blazor - not working
  9. using instructions from ScaffoldingReadMe.txt
  10. logout from blazor - not working

    NOTE:

  11. if user go to https://localhost:5001/Identity/Account/Manage , then from _MangeNav.cshtml can succesfuly LogOut from app.

WORKAROUND NUMBER 1:

  1. Add [IgnoreAntiforgeryToken] in "LogOut.cshtml.cs" file

WORKAROUND NUMBER 2:

  1. delete files in areas/pages/account "LogOut.cshtml" and "LogOut.cshtml.cs", and create new file that is like the one before scaffold ( "LogOut.cshtml" )
  2. if not using --force , then Building project ...Build Failed. ( but possibly to specify every file except "LogOut.cshtml" , --files "Account.Register;Account.Login" )
  @page
  @using Microsoft.AspNetCore.Identity
  @attribute [IgnoreAntiforgeryToken]
  @inject SignInManager<IdentityUser> SignInManager
  @functions {
      public async Task<IActionResult> OnPost()
      {
          if (SignInManager.IsSignedIn(User)){await SignInManager.SignOutAsync();}
          return Redirect("~/");
      }
  }

REPOS

and the orginal version with wrong behaviour https://github.com/sikira/BlazorScaffoldedIdentity/tree/withbug

repo with sample project with workaround https://github.com/sikira/BlazorScaffoldedIdentity/tree/master

SIDE NOTES:
  1. This behaviour happend in version 3.0.100 and in 3.1.0, but in .Net Core 3.0.100 this is writen in console: info: Microsoft.AspNetCore.Routing.EndpointMiddleware[1] Executed endpoint '/_blazor' Microsoft.AspNetCore.Routing.EndpointMiddleware: Information: Executed endpoint '/_blazor' info: Microsoft.AspNetCore.Hosting.Diagnostics[2] Request finished in 21743.366ms 101 Microsoft.AspNetCore.Hosting.Diagnostics: Information: Request finished in 21743.366ms 101 info: Microsoft.AspNetCore.Mvc.ViewFeatures.Filters.AutoValidateAntiforgeryTokenAuthorizationFilter[1] Antiforgery token validation failed. The required antiforgery request token was not provided in either form field "RequestVerificationToken" or header value "RequestVerificationToken". Microsoft.AspNetCore.Antiforgery.AntiforgeryValidationException: The required antiforgery request token was not provided in either form field "RequestVerificationToken" or header value "RequestVerificationToken". at Microsoft.AspNetCore.Antiforgery.DefaultAntiforgery.ValidateRequestAsync(HttpContext httpContext) at Microsoft.AspNetCore.Mvc.ViewFeatures.Filters.ValidateAntiforgeryTokenAuthorizationFilter.OnAuthorizationAsync(AuthorizationFilterContext context) Microsoft.AspNetCore.Mvc.ViewFeatures.Filters.AutoValidateAntiforgeryTokenAuthorizationFilter: Information: Antiforgery token validation failed. The required antiforgery request token was not provided in either form field "RequestVerificationToken" or header value "RequestVerificationToken". Microsoft.AspNetCore.Antiforgery.AntiforgeryValidationException: The required antiforgery request token was not provided in either form field "RequestVerificationToken" or header value "RequestVerificationToken". at Microsoft.AspNetCore.Antiforgery.DefaultAntiforgery.ValidateRequestAsync(HttpContext httpContext) at Microsoft.AspNetCore.Mvc.ViewFeatures.Filters.ValidateAntiforgeryTokenAuthorizationFilter.OnAuthorizationAsync(AuthorizationFilterContext context) info: Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker[3] Authorization failed for the request at filter 'Microsoft.AspNetCore.Mvc.ViewFeatures.Filters.AutoValidateAntiforgeryTokenAuthorizationFilter'. Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker: Information: Authorization failed for the request at filter 'Microsoft.AspNetCore.Mvc.ViewFeatures.Filters.AutoValidateAntiforgeryTokenAuthorizationFilter'. info: Microsoft.AspNetCore.Mvc.StatusCodeResult[1] Executing HttpStatusCodeResult, setting HTTP status code 400 Microsoft.AspNetCore.Mvc.StatusCodeResult: Information: Executing HttpStatusCodeResult, setting HTTP status code 400 info: Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure.PageActionInvoker[4]

Further technical details

.NET Core SDK (reflecting any global.json): Version: 3.1.100 Commit: cd82f021f4

Runtime Environment: OS Name: Windows OS Version: 10.0.17763 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support): Version: 3.1.0 Commit: 65f04fb6db

.NET Core SDKs installed: 3.0.100 [C:\Program Files\dotnet\sdk] 3.1.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

[blazor]
[identity] [scaffold] [logout]

Copied from original issue: dotnet/aspnetcore#17839

danroth27 commented 4 years ago

From @mkArtakMSFT on Friday, December 13, 2019 5:13:30 PM

Thanks for contacting us. Unfortunately Identity scaffolder doesn't work well with Blazor projects yet. We plan to handle this as part of 5.0 release.

danroth27 commented 4 years ago

From @sikira on Thursday, January 2, 2020 9:20:04 AM

enetstudio tnx for suggestion, but it didn't work for me on this issue. This "a" element is a GET request, not POST, and i don't see how it would work without further modification on logout page. Maybe i'm missing something.

danroth27 commented 4 years ago

From @pm64 on Saturday, January 4, 2020 7:51:49 AM

@sikira have you been able to determine WHY your workaround is necessary? Without your workaround, in the scaffolded Logout.cshtml.cs, placing breakpoints everywhere shows that no code associated with this page is ever executed. Why is this? Also, a blank page is rendered (instead of the "not found" behavior), which is equally perplexing, since the framework is acknowledging that the page exists. Do you have any insight into this behavior? I'm baffled as to why OnPost() in the scaffolded Logout.cshtml.cs is not executed.

danroth27 commented 4 years ago

From @sikira on Saturday, January 4, 2020 3:24:42 PM

@pm64 your comment intrigued me to try to investigate more, and i find out that the issue is actually about antiforgery token. Another and better fix would be to put atribute [IgnoreAntiforgeryToken] on top of LogoutModel class in "LogOut.cshtml.cs" file. I've add this fix to orginal issue.

danroth27 commented 4 years ago

From @pm64 on Saturday, January 4, 2020 5:11:17 PM

@sikira amazing catch!! This is precisely the issue, You have saved me many hours of hair-pulling and cursing. Huge thanks!

danroth27 commented 4 years ago

From @pm64 on Wednesday, January 8, 2020 7:09:08 PM

@sikira you might be interested in another issue I discovered in the ASP.Net Core Identity scaffolder: https://github.com/dotnet/aspnetcore/issues/18200

danroth27 commented 4 years ago

From @yeahrow on Monday, January 13, 2020 11:29:43 AM

@sikira It works perfectly for me

danroth27 commented 4 years ago

From @MisinformedDNA on Thursday, January 30, 2020 2:24:31 PM

@mkArtakMSFT Is there any guidance available for how to customize Blazor/Identity without scaffolding? Specifically, I want to add username to the login page. (figured this out)

While we wait for 5.0, what's the best practice?

danroth27 commented 4 years ago

From @idchlife on Saturday, February 1, 2020 11:24:19 PM

@MisinformedDNA could you please tell, how did you create custom login page for blazor server? I'm struggling to figure it out. I tried to just use SignInManager.PasswordSignInAsync in my form, but it seems it's for Http based authentication like in MVC projects, not for blazor.

Maybe you inherited scaffolded form somehow or used another SignInManager method?

danroth27 commented 4 years ago

From @TheAnachronism on Sunday, February 23, 2020 5:04:39 PM

@sikira Thank you so much for that. That really saved some time for me trying to find out why this was happening.

danroth27 commented 4 years ago

From @danroth27 on Thursday, March 19, 2020 9:01:29 PM

@mkArtakMSFT I'm able to reproduce this as well with the latest bits.

danroth27 commented 4 years ago

From @danroth27 on Friday, March 20, 2020 12:56:11 AM

@javiercn So it looks like what's happening here is the default Blazor Server template adds a custom LogOut.cshtml that has the [IgnoreAntiforgeryToken] attribute, but then when you scaffold in Identity that file gets overridden.

I know we have other places in the Identity scaffolder where it won't scaffold certain files if they already exist. Should we maybe implement that logic more generally? Or is there a way to align the logout logic used by Blazor Server with the default logout logic used by the default Identity UI?

danroth27 commented 4 years ago

From @mkArtakMSFT on Friday, March 20, 2020 4:49:06 AM

@danroth27 is this what @HaoK is going to help out with?

danroth27 commented 4 years ago

From @danroth27 on Friday, March 20, 2020 5:23:51 AM

@mkArtakMSFT I think we need to understand what the fix should be first.

danroth27 commented 4 years ago

From @danroth27 on Friday, March 20, 2020 9:29:41 PM

I chatted with @vijayrkn and @javiercn about this. Currently when you say that you want to "Override all files" the Identity scaffolder will happily overwrite existing files. There's an important distinction here between overriding the pages in the default Identity UI, and overwriting files already on disk. Just because you said you want to override a page from the default Identity UI, does not necessarily mean that you want to overwrite an existing page that you already have. The Identity scaffolder already has support for letting you choose which pages you want to override.

It seems like it should be an error if you try to override a page that will result in an existing page getting overwritten. We could also provide an option to force overwriting files if they already exists.

The tooling could help you understand when there are existing files that would get overwritten and let you decide if you want to do that or not.

danroth27 commented 4 years ago

From @danroth27 on Friday, March 20, 2020 9:38:14 PM

@HaoK @deepchoudhery

danroth27 commented 4 years ago

@deepchoudhery Please address for 3.1. Thanks!

deepchoudhery commented 4 years ago

Fixed in VS as it ended not being a aspnet/scaffolding issue. Should be working in VS 16.6 Preview 3 and onwards.

sikira commented 4 years ago

Fixed in VS as it ended not being a aspnet/scaffolding issue. Should be working in VS 16.6 Preview 3 and onwards. @deepchoudhery Maybe you have missed, but project was done in VS Code 1.40.2.

danroth27 commented 4 years ago

Hi @sikira. Thanks for filing this detailed feedback, and your patience as we work to address the issue!

The Blazor Server template with Identity enabled includes a custom LogOut.cshtml page to handle the anti-request forgery issue. When you run the identity scaffolder to generate all of the default identity UI related code, it will detect if any of the files already exists, and return an error message specifying which files would get overwritten. If you then pass in --force it will force the identity scaffolder to overwrite these files, which breaks the logout flow in a Blazor Server.

Instead what you need to do is scaffold everything except the LogOut.cshtml file. This is straightforward to do from VS, because you can just uncheck the checkbox for that file (or at least it will be straight forward once we fix VS to first warn you when files already exist like we already do from the command-line).

From the command-line it's a bit more difficult to exclude just one file. You have to list all of the files that you want to scaffold excluding the ones you don't want. You can see the list of all files that you can scaffold using the --listFiles option, and then you can construct the list of all files excluding LogOut.cshtml:

dotnet aspnet-codegenerator identity -dc BlazorScaffoldedIdentity.Data.ApplicationDbContext -fi  Account._StatusMessage;Account.AccessDenied;Account.ConfirmEmail;Account.ConfirmEmailChange;Account.ExternalLogin;Account.ForgotPassword;Account.ForgotPasswordConfirmation;Account.Lockout;Account.Login;Account.LoginWith2fa;Account.LoginWithRecoveryCode;Account.Manage._Layout;Account.Manage._ManageNav;Account.Manage._StatusMessage;Account.Manage.ChangePassword;Account.Manage.DeletePersonalData;Account.Manage.Disable2fa;Account.Manage.DownloadPersonalData;Account.Manage.Email;Account.Manage.EnableAuthenticator;Account.Manage.ExternalLogins;Account.Manage.GenerateRecoveryCodes;Account.Manage.Index;Account.Manage.PersonalData;Account.Manage.ResetAuthenticator;Account.Manage.SetPassword;Account.Manage.ShowRecoveryCodes;Account.Manage.TwoFactorAuthentication;Account.Register;Account.RegisterConfirmation;Account.ResetPassword;Account.ResetPasswordConfirmation

To make this easier from the command-line we can consider adding a feature that lets you specify a file exclusion list instead of an inclusion list. I've filed https://github.com/aspnet/Scaffolding/issues/1296 to track this feature request.

wstaelens commented 4 years ago

@danroth27 I have an existing BSSA (blazor server-side app) with facebook external login. I scaffolded everything and indeed I can't log out anymore. I can only logout if I click on "Log out" from the Manage section in Identity.

after reading this I have deleted the LogOut.cshtml page which was scaffolded, but still logout doesn't work.

How can I make it work?

should I try workaround #1 or #2 from https://github.com/dotnet/aspnetcore/issues/17839 ?

sikira commented 4 years ago

@danroth27 I have an existing BSSA (blazor server-side app) with facebook external login. I scaffolded everything and indeed I can't log out anymore. I can only logout if I click on "Log out" from the Manage section in Identity.

after reading this I have deleted the LogOut.cshtml page which was scaffolded, but still logout doesn't work.

How can I make it work?

should I try workaround #1 or #2 from dotnet/aspnetcore#17839 ?

I would recommend workaround 1. In that case you don't need to delete anything, just add [IgnoreAntiforgeryToken] in "LogOut.cshtml.cs" file

wstaelens commented 4 years ago

@sikira thanks, works!

davidbuckleyni commented 1 year ago

This still seems to be an issue in 2022 I Had ot delete the login.cs and replace it with the method describe please update the templates comon like the issue is 2 year old promised updated in version 5