Skip to content

[release/10.0-preview4] Backport identity template fix. #61795

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

Open
wants to merge 3 commits into
base: release/10.0-preview4
Choose a base branch
from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented May 5, 2025

Partial backport of #61633 to release/10.0-preview4

Description

  • Blazor identity templates (-auth Individual) were expecting NavigateTo to throw. We should remove this expectation for the new way of navigation.
  • Blazor templates do not test behavior at runtime. That is why we did not catch this bug in the PR that changed the navigation flow. We have build and publish and check of produced elements on dotnet new command covered. I don't think we want to add runtime tests for templates, it would add a lot of testing and maintenance load, even if it was added to outerloop. Manual tests caught this problem. Fixes https://github.com/dotnet/AspNetCore-ManualTests/issues/3609.

Fixes #3609

Customer Impact

Customer code based on the template won't be throwing after each redirection because the exceptions located after each redirect action got removed. If the customer misuses the identity manager class, they won't notice if they call RedirectTo from non-static code.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This is a change to templates only.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ilonatommy ilonatommy requested review from lewing and javiercn May 5, 2025 06:58
@ilonatommy ilonatommy self-assigned this May 5, 2025
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 5, 2025
@@ -60,6 +64,11 @@

private async Task OnValidSubmitAsync()
{
if (user is null)
{
throw new InvalidOperationException("User is not loaded; failed to set password.");
Copy link
Member

Choose a reason for hiding this comment

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

We need to make this message more "human-friendly".

Alternatively, within OnIntializedAsync we can check RenderInfo.IsInteractive and potentially throw an exception there, or display an alternative UI with an error message (or redirect to the error page).

@MackinnonBuck since this affects the Identity UX it will be good if we can figure this out together.

Ultimately with the exception going away we need to provide an alternative, it might be throwing, redirecting to the error page or showing a message in this page.

Copy link
Member

Choose a reason for hiding this comment

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

@MackinnonBuck just FYI, we might merge this to unblock things and do a follow up to cleanup things when we've had a chance to talk together through this.

@lewing lewing added the Servicing-consider Shiproom approval is required for the issue label May 6, 2025
Copy link
Contributor

Hi @@ilonatommy. 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.

@lewing lewing removed the Servicing-consider Shiproom approval is required for the issue label May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants