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

Preserve RemoteAuthenticationContext during JS interop #54225

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Conversation

halter73
Copy link
Member

@halter73 halter73 commented Feb 26, 2024

Preserve RemoteAuthenticationContext from Microsoft.AspNetCore.Components.WebAssembly.Authentication during JS interop in order to avoid errors during sign in and sign out like "There was an error trying to log you in: '"undefined" is not valid JSON'".

This didn't produce trim warnings because JSRuntime suppresses them. #39839 tracks adding trim-safe JSRuntime APIs.

I tried the suggested fix in #53819, but that did not fix the issue because type-level [DynamicallyAccessedMembers] only helps if something calls RemoteAuthenticationContext.GetType(). It does not mean if any part of the type is preserved, preserve all parts of the type. You can look dotnet/linker#1196, dotnet/runtime#49465, and dotnet/linker#1929 for the original motivation and test cases that were added when [DynamicallyAccessedMembers] was first allowed to target classes. The reason that it appeared to work with the DemoRemoteAuthenticationContextWithAttribute test case is because Blazor does not trim application assemblies by default. If you trim the application assembly, you will see the same issue.

Fixes #49956
Fixes #52291
Fixes #52619
Fixes #53131
Fixes #53409

@halter73 halter73 requested a review from javiercn February 26, 2024 16:57
@halter73 halter73 requested a review from a team as a code owner February 26, 2024 16:57
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Feb 26, 2024
@halter73 halter73 removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Mar 19, 2024
@halter73 halter73 merged commit 2804cf7 into main Mar 20, 2024
23 of 26 checks passed
@halter73 halter73 deleted the halter73/49956 branch March 20, 2024 23:28
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview4 milestone Mar 20, 2024
@halter73
Copy link
Member Author

/backport to release/8.0

Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/aspnetcore/actions/runs/8367248800

Copy link
Contributor

@halter73 backporting to release/8.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Preserve RemoteAuthenticationContext during JS interop
Applying: Add trimmed E2E RemoteAuthenticationTest
Using index info to reconstruct a base tree...
M	AspNetCore.sln
M	src/Components/Components.slnf
M	src/Components/test/E2ETest/Infrastructure/ServerFixtures/AspNetSiteServerFixture.cs
M	src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj
M	src/Components/test/E2ETest/Tests/WebAssemblyPrerenderedTest.cs
M	src/Components/test/testassets/Components.TestServer/Components.TestServer.csproj
M	src/Components/test/testassets/Components.TestServer/Program.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Components/test/testassets/Components.TestServer/Program.cs
Auto-merging src/Components/test/testassets/Components.TestServer/Components.TestServer.csproj
Auto-merging src/Components/test/E2ETest/Tests/WebAssemblyPrerenderedTest.cs
Auto-merging src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj
CONFLICT (content): Merge conflict in src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj
Auto-merging src/Components/test/E2ETest/Infrastructure/ServerFixtures/AspNetSiteServerFixture.cs
Auto-merging src/Components/Components.slnf
CONFLICT (content): Merge conflict in src/Components/Components.slnf
Auto-merging AspNetCore.sln
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Add trimmed E2E RemoteAuthenticationTest
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 128

Please backport manually!

Copy link
Contributor

@halter73 an error occurred while backporting to release/8.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment