The Wayback Machine - https://web.archive.org/web/20220128181120/https://github.com/dotnet/aspnetcore/pull/39838
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

Jump to bottom

Annotate WebAssembly.Authentication #39838

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Annotate WebAssembly.Authentication #39838

wants to merge 4 commits into from
+68 −21

Conversation

@pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Jan 28, 2022

Fixes #30751
Fixes #35162
@pranavkm pranavkm requested a review from as a code owner Jan 28, 2022
@msftbot msftbot bot added the area-blazor label Jan 28, 2022
TanayParikh
TanayParikh approved these changes Jan 28, 2022
View changes
Copy link
Member

@TanayParikh TanayParikh left a comment

LGTM, just some formatting nits

...Components/WebAssembly/Authentication.Msal/src/MsalWebAssemblyServiceCollectionExtensions.cs Outdated Show resolved Hide resolved
...ssembly.Authentication/src/Microsoft.AspNetCore.Components.WebAssembly.Authentication.csproj Outdated
CandidateAssets="@(_InteropBuildOutput)"
RelativePathFilter="**.js"
>
<DefineStaticWebAssets Condition="@(_InteropBuildOutput) != ''" SourceType="Computed" SourceId="$(PackageId)" ContentRoot="$(YarnWorkingDir)dist\$(Configuration)\" BasePath="_content/$(PackageId)" CandidateAssets="@(_InteropBuildOutput)" RelativePathFilter="**.js">
Copy link
Member

@TanayParikh TanayParikh Jan 28, 2022

Choose a reason for hiding this comment

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

Was this unwrapping intentional? If not, could we please revert this bit?

...omponents/WebAssembly/WebAssembly.Authentication/src/Services/RemoteAuthenticationService.cs Outdated Show resolved Hide resolved
...embly/WebAssembly.Authentication/src/WebAssemblyAuthenticationServiceCollectionExtensions.cs Outdated Show resolved Hide resolved
@pranavkm
Copy link
Contributor Author

@pranavkm pranavkm commented Jan 28, 2022 •
edited

Authentication.MSAL was already annotated for trimming. However WebAssembly.Authentication previously wasn't. Investigating this issue, the underlying issue was that our JSRuntime.Invoke* APIs aren't annotated accurately. Essentially, all of the args array is subject to serialization constraints, but we don't express this. Unfortunately, the best we could do is slap a RequiresUnreferencedCode on these methods.

In this particular case, MsalProviderOptions is passed as a JSRuntime.InvokeAsync argument but was being trimmed (all of the getters were being removed). Annotating the entry point as this PR does helps, but it would help to revist JSRuntime's annotations.

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

@TanayParikh TanayParikh

Assignees
No one assigned
Labels
area-blazor
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Blazor WebAssembly MSAL authentication error
Annotate and mark remaining Blazor WASM assemblies
2 participants
@pranavkm @TanayParikh