Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth with global interactivity #50920

Closed

Conversation

SteveSandersonMS
Copy link
Member

This updates the Blazor Web template so that the new "global interactivity" and "auth" options work together.

I don't know whether we're still pushing for this to be included in RC2, but just in case, that's what the PR currently targets. I've manually tested a representative selection of project combinations, but there are a huge number of possible combinations altogether so naturally I haven't manually checked them all.

As discussed with the team earlier, I've added <RedirectToLogin>, but have not changed the "/Account" path matching as that is something we can make more of a decision about later when @halter73 completes his follow-ups and possibly has additional options (e.g., maybe a base class, or some other mechanism).

Note that I have also moved as much of the identity stuff as I could under a single Components/Account subdir so it's easier for people to focus on their own code at the start of their new project. I didn't attempt to move the Data subdir since that is tied into EF migrations.

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 25, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 25, 2023
@SteveSandersonMS SteveSandersonMS requested a review from a team September 25, 2023 17:27
"BlazorWeb-CSharp/Components/Pages/Weather.razor": "./BlazorWeb-CSharp.Client/Pages/Weather.razor",
"BlazorWeb-CSharp/Components/Pages/Weather.razor.css": "./BlazorWeb-CSharp.Client/Pages/Weather.razor.css",
"BlazorWeb-CSharp/Components/Pages/Counter.razor": "./BlazorWeb-CSharp.Client/Pages/Counter.razor",
"BlazorWeb-CSharp/Components/Pages/Counter.razor.css": "./BlazorWeb-CSharp.Client/Pages/Counter.razor.css",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a bug here before with Error.razor being moved into the .Client project if you were using WebAssembly or Auto. To avoid this and future bugs of the same kind, this is now moving individual pages so that newly-added things are not moved by default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, the projects failed to compile before this fix for Error.razor, because the .Client project can't even reference HttpContext (without at least adding a reference to M.A.Http.Abstractions).

Copy link
Contributor

@Bartmax Bartmax Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be that the 'external-account' form (at least in WebAssembly only mode) should be a fullpage post ? I think calling a Challenge result will probably redirect to an Authorization Server that may (own auth server) or may not (google/facebook auth server) allow CORS.

@wtgodbe
Copy link
Member

wtgodbe commented Sep 27, 2023

@SteveSandersonMS if this is still needed it should be retargeted to release/8.0

@SteveSandersonMS
Copy link
Member Author

@wtgodbe

@halter73 Will you be taking over this PR from here? If so can I leave that with you?

@halter73
Copy link
Member

I'll take it from here.

@halter73 halter73 closed this Sep 28, 2023
@wtgodbe wtgodbe added this to the 8.0.0 milestone Oct 3, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 3, 2023
mkArtakMSFT pushed a commit that referenced this pull request Oct 11, 2023
## 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 dotnet/razor#9343
  - API Proposal issue: #51170
  - This is a e necessary to support the changes to add global interactivity to Identity components @SteveSandersonMS made in #50920 and have now been included in this PR.
- [Add antiforgery token to forms rendered interactively on the server](425bd12)
  - 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
- [x] No

## Risk

- [ ] High
- [ ] Medium
- [x] 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

- [x] Manual (required)
- [x] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A
@SteveSandersonMS SteveSandersonMS deleted the stevesa/auth-with-global-interactivity branch November 16, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants