Update Identity Components in Blazor project template

[halter73]

Description

This PR addresses a large amount of feedback from #50722 which was merged before they could all be addressed to unblock Accessibility Testing effort. The primary impacts are:

Runtime changes

• Public API change to make AddComponentRenderMode's renderMode param nullable to support disabling interactivity on a per-page basis with the help of @rendermode="null" (effectively).
  • IMPORTANT: This will need follow up in the Razor Compiler. See Support null @rendermode razor#9343
  • API Proposal issue: Make RenderTreeBuilder.AddComponentRenderMode's renderMode parameter nullable #51170
  • This is a e necessary to support the changes to add global interactivity to Identity components @SteveSandersonMS made in Auth with global interactivity #50920 and have now been included in this PR.
• Add antiforgery token to forms rendered interactively on the server
  • This bug fix is necessary to make the logout button work without throwing antiforgery errors when it is rendered interactively on the server.

Template changes

• Fix compilation error due to missing using in Program.cs when the individual auth option is selected with no interactivity.
• Add support for global (--all-interactive) instead of just per-page interactivity to the new Identity components.
• Fix "Apply Migrations" link on the DatabaseErrorPage by calling UseMigrationsEndPoint() when necessary.
• Add support for non-root base paths to the new Identity components.
• Improve folder layout by putting most of the additional auth and Identity related files in the same /Account folder.
• Use the new IEmailSender<ApplicationUser> API instead of IEmailSender for easier customization of emails.
• Remove usage of IHttpContextAccessor from the template because that is generally regarded as bad practice due to the unnecessary reliance on AsyncLocal.
• Remove underscore (_) from private field names.
• Reduce usage of null! and default!.
• Use normal <button> for logout link in nav bar rather than <a onclick="document.getElementById('logout-form').submit();">, and remove separate LogoutForm.razor.

Customer Impact

This fixes several bugs in the Blazor project template when choosing the individual auth option and makes several runtime fixes that will be beneficial to any global interactive Blazor application that needs to include some components that must always be statically rendered.

Regression?

• Yes
• No

Risk

• High
• Medium
• Low

Obviously, we would rather not make such a large change after RC2. Particularly when it's a change that touches public API. Fortunately, the runtime changes are very small, and only to parts of the runtime that were last updated in RC2 (see #50181 and #50946).

The vast majority of the changes in the PR only affect the Blazor project template when the non-default individual auth option is selected. This was merged very late in RC2 (#50722) with the expectation that we would make major changes prior to GA.

Verification

• Manual (required)
• Automated

Packaging changes reviewed?

• Yes
• No
• N/A

[ghost]

Hi @halter73. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

[javiercn]

@halter73 took a look at the changes.

Overall looks good, the main questions that I have are:

• The use of required which is "controversial"
• Whether the identity pages should live inside an Identity subfolder. It might feel unnecessary but its better for the longer term, as it enables people to use _Import.razor files that target only identity and nothing more.
• Should we have an IdentityPage base component?
  • There's a lot of repetition on the pages, injecting a bunch of services, getting the HttpContext, etc. all of which can be centralized into a base class.
  • We can then use an @inherits IdentityPage on an _Imports.razor file to apply the base to all the Identity pages in one go.

[halter73]

• The use of required which is "controversial"

There was earlier discussion about this in the PR that originally added the Identity components to the project template when I added it the old way without required and instead used = default!. #50722 (comment).

I think

[CascadingParameter]
public required HttpContext HttpContext { get; set; }

looks a lot nicer than

[CascadingParameter]
public HttpContext HttpContext { get; set; } = default!;

And we even got some community agreement. Although they also noted that this goes directly against our guidance:

Don't use the required modifier or init accessor on component parameter properties. Components are usually instantiated and assigned parameter values using reflection, which bypasses the guarantees that init and required are designed to make.

@MackinnonBuck also brought up a good point about how it could prevent code-generated parameter setting from working in the future if we ever decided to do that. We're only using required in situations like static server rendering where we theoretically could ensure the parameters like the HttpContext [CascadingParameter] already exist during component construction meaning it's possible we could make code generation work, but this would require initializing the parameters earlier than before which might break backwards compatibility to some extent.

I'm fine with whatever we decide. We're still using private ApplicationUser user = default!; in a bunch of places elsewhere, so it's not like these required modifiers is allowing us to completely remove default! from the template. I'd like to hear from @SteveSandersonMS before removing the required's though, since he suggested it.

• Whether the identity pages should live inside an Identity subfolder. It might feel unnecessary but its better for the longer term, as it enables people to use _Import.razor files that target only identity and nothing more.

The Account folder already serves this purpose. The identity-specific _Imports.razor file you propopse already exists too, thanks to @SteveSandersonMS's changes in #50920.

• Should we have an IdentityPage base component?

Here's the discussion from when you suggested it in the original PR. @SteveSandersonMS mentioined:

While having a base class would work, it is a bit of a constraining solution (people may have their own other base classes they want to use). I'd be in favour of just adding [CascadingParameter] private required HttpContext HttpContext { get; set; } wherever it's needed (and I know it's in about 15 places).

It was actually less than 15 places, but it was pretty close. I agree there is a lot of repetition, but there are also quite a few very simple components like Lockout.razor that gain nothing from a base component.

I'm not sure I buy that a base class is all that constraining. If people have their own base component, couldn't they make Identity base component inherit from their own? But I do think avoiding inheritance where possible is generally a good idea because it shows there's nothing up our sleeves so to speak. All of the services and parameters we use, we declare directly on the page.

[MackinnonBuck]

It's kind of subtle that RedirectTo() never returns, making the return statement redundant. From reading the code, it might be hard to tell at first that each of these if statements acts as a kind of implicit guard clause. But I don't feel a strong way about this so if you think it's fine to remove the returns then I won't fight it! 🙂

[ghost]

Hi @halter73. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

[halter73]

You can review the project's generated with these template changes at https://github.com/halter73/BlazorWebTemplateMatrix#readme. You can just install the latest 8.0 SDK from the https://github.com/dotnet/installers repo (8.0.100-rtm.23506.1 currently) and reference that in the global.json to quickly try any of the variations. The -ai (i.e. --all-interactivity) versions compile but will have nullability warnings and throw ArgumentNullExceptions until we get the runtime fixes in this PR and the razor compiler @rendermode nullablity update tracked by #9343 into the SDK.

[danroth27]

@JeremyLikness @halter73 Are we sure we want to use "auth" in the template? The problem with "auth" is that it's an abbreviation for both authentication & authorization, which makes it ambiguous and it's also pretty informal.

Some alternatives:

• Authenticated.razor
• LoginRequired.razor
• Restricted.razor
• ...

[javiercn]

Not for this PR, but in wasm/server navigationManager.NavigateTo(uri); continues executing, which is a visible behavior difference. /cc: @SteveSandersonMS

[javiercn]

@halter73 I do not care much about the base class vs non base class, it's unlikely impossible for anyone to have a base class that is broadly used and where everything on the base class is used by every derived implementation, it's always a trade-off of convenience vs repetition.

The other main thing is the required modifier that we agreed we were switching back to = default!.

[ghost]

Hi @Kumima. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

[los93sol]

I routinely put an _Imports.razor at the root of my projects with @Attribute [Authorize] to enforce authorization on all pages, but because @Attribute [AllowAnonymous] is not explicitly defined on the razor pages in Pages/Account that ends up breaking the pages and requires me to go specify it manually.

[ghost]

Hi @los93sol. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.