aspnet / Identity

[Archived] ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.96k stars 868 forks source link

/Identity/Account/ResetPassword page is not reachable when auth required globally #1736

Closed dougbu closed 6 years ago

dougbu commented 6 years ago

Looks like at least one page was missed when fixing #1617

Repro steps

  1. dotnet new mvc --name netcoreIndividualAuth --auth Individual --use-local-db (doubt --use-local-db is mandatory)

  2. cd netcoreIndividualAuth

  3. Open project in Visual Studio

  4. Edit Startup.ConfigureServices(...) to require an authenticated user globally i.e. change services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_1); to

    services
      .AddMvc(configuration =>
      {
          var policy = new AuthorizationPolicyBuilder()
              .RequireAuthenticatedUser()
              .Build();
          configuration.Filters.Add(new AuthorizeFilter(policy));
      })
      .SetCompatibilityVersion(CompatibilityVersion.Version_2_1);
  5. Start app in the debugger

  6. Ensure debug option "Enable Just My Code" is disabled and "Microsoft Symbol Servers" are enabled under "Symbols"

  7. Set C# function breakpoint at Microsoft.AspNetCore.Identity.UI.Services.EmailSender.SendEmailAsync

  8. In browser, register a new user

  9. In debugger, copy the URL in the htmlMessage parameter

  10. In browser, head to the copied URL but change &code to &code

  11. In browser, log out, click on "Forgot your password", then enter the new user's email and click Submit

  12. In debugger, copy the URL in the htmlMessage parameter

  13. In browser, head to the copied URL

Expected


This is a small part of #1617 which does not work end-to-end. Found while verifying (aspnet/Release#263) with the 2.1.300-preview2-008523 SDK and Microsoft.AspNetCore.Identity.UI 2.1.0-preview2-30552.

Eilon commented 6 years ago

So... is the suggestion to just add AllowAnonymous to the two pages that should allow anonymous?

dougbu commented 6 years ago

Yes, exactly. Sorry for leaving that out.

Eilon commented 6 years ago

OK so a 2-line fix? That should be easy in RC1.

dougbu commented 6 years ago

It's a 2-line fix unless other pages involved in resetting a password also need to be accessible before user logs in again.

dougbu commented 6 years ago

After lowering the lockout attempts to 2 i.e. services.AddDefaultIdentity<IdentityUser>(options => options.Lockout.MaxFailedAccessAttempts = 2), I found the /Identity/Account/Lockout page also needs [AllowAnonymous] on its page model class. Without this, user is redirected to login page again.

dougbu commented 6 years ago

Related question: Should the /Identity/Account/AccessDenied page have [AllowAnonymous]? This page doesn't appear to be used by default and I'm unsure where it would be used.

Might have missed other pages that should allow anonymous access for specific scenarios.

blowdart commented 6 years ago

Access denied is for someone whose permissions failed auth - so they are logged in, just they don't have access. So it doesn't need allow anonymous, it's point is that someone is logged in.