-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[release/7.0] Fix Blazor RemoteAuthenticatorView processing the same action multiple times #43844
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
[release/7.0] Fix Blazor RemoteAuthenticatorView processing the same action multiple times #43844
Conversation
|
@Pilchie This is ready to be merged - would you take a look please? |
|
Approved for .NET 7 RC2. |
|
Are there any plans on back-porting this fix to .NET 6 since it's an LTS release? Looking at the PR for this fix, it doesn't look like it took a whole bunch of wicked re-engineering to resolve it. |
|
Hi @stephajn. 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. |
|
/backport to release/6.0 |
|
Hi @MackinnonBuck. 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. |
|
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/3316194514 |
|
@MackinnonBuck backporting to release/6.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Avoid processing the same action multiple times
Using index info to reconstruct a base tree...
M src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs
M src/Components/WebAssembly/WebAssembly.Authentication/test/RemoteAuthenticatorCoreTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Components/WebAssembly/WebAssembly.Authentication/test/RemoteAuthenticatorCoreTests.cs
CONFLICT (content): Merge conflict in src/Components/WebAssembly/WebAssembly.Authentication/test/RemoteAuthenticatorCoreTests.cs
Auto-merging src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs
CONFLICT (content): Merge conflict in src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Avoid processing the same action multiple times
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
Fix Blazor
RemoteAuthenticatorViewprocessing the same action multiple timesFixes an issue where the
<RemoteAuthenticatorView/>component would sometimes fire the login callback more than once.Description
The previous logic processed the current authentication action each time the component was re-rendered. Depending on the authentication service used, this could result in the login callback firing more than once. This PR fixes the problem by caching the last processed action and disabling authentication processing until the action changes.
Fixes #39507
Customer Impact
It causes difficulty for customers to write their own logic in a way that accounts for login callbacks firing more than once. In addition, other expensive authentication actions may get invoked unnecessarily behind the scenes, which can degrade the experience of the customer's app. Multiple customers have reported being affected by this issue, so fixing it has a relatively high impact.
Regression?
Risk
The fix for this issue is simple, and we have extensive existing test coverage. New tests have been added validating the fix.
Verification
Packaging changes reviewed?