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

Blazor WASM cookie security for web APIs #32028

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Mar 11, 2024

Fixes #32008

Jeremy ... Three things on this one ...

  • Do you want to say more than this in the Call web API article? I figure that this along with the Standalone with Identity article (+sample), which is cross-linked here, should be enough. Are there any 😈 gotchas to call out or anything else?

  • AFAIK, the only versioning that this needs is for the Standalone with Identity article (>=8.0). I think the rest of this always worked. AFAIK, we just didn't have it because we were pushing token auth for just about everything. We have a bit of this in the next section, which is generalized for Fetch API options. We just didn't have it in a dedicated cookie auth section. Anyway, any concerns about showing this for all versions?

  • The API engineering comment for the handler for AddHttpMessageHandler states emphatically ...

    The handler type must be registered as a transient service.

    ... but it's registered scoped ...

    builder.Services.AddScoped<CookieHandler>();

    What's the story on that? I feel like we need to explain this exception to the rule in the article where I show the service registration.

... and just an FYI that I added a second seeded test user to the sample app and a second roles-authorized endpoint with a page where Leela can call both endpoints and Harry can only call one. dotnet/blazor-samples#241


Internal previews

📄 File 🔗 Preview link
aspnetcore/blazor/call-web-api.md Call a web API from ASP.NET Core Blazor
aspnetcore/blazor/security/webassembly/additional-scenarios.md ASP.NET Core Blazor WebAssembly additional security scenarios

Copy link
Member

@JeremyLikness JeremyLikness left a comment

Choose a reason for hiding this comment

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

LGTM. To answer your questions:

  • I don't believe we need to say more, and the cross-linking should be fine
  • No concerns with showing it for all versions
  • That's a great question I'll need to defer to @halter73 to answer

Regards, Jeremy

@guardrex
Copy link
Collaborator Author

guardrex commented Mar 20, 2024

Thanks @JeremyLikness.

@halter73 ... The API comment for the delegate provided to AddHttpMessageHandler says that the service should be transient. However, the CookieHandler is registered scoped. Because it goes against the API remark, can we explain something about that delta in the article? ... explain why it's ok in this case to depart from the API guidance.

In the app ...

https://github.com/dotnet/blazor-samples/blob/main/8.0/BlazorWebAssemblyStandaloneWithIdentity/BlazorWasmAuth/Program.cs

builder.Services.AddHttpClient(
    "Auth",
    opt => opt.BaseAddress = new Uri(builder.Configuration["BackendUrl"] ?? "https://localhost:5001"))
    .AddHttpMessageHandler<CookieHandler>();
builder.Services.AddScoped<CookieHandler>();

Type Parameters
THandler

The type of the DelegatingHandler. The handler type must be registered as a transient service.

Cross-ref: https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.httpclientbuilderextensions.addhttpmessagehandler?view=dotnet-plat-ext-8.0#microsoft-extensions-dependencyinjection-httpclientbuilderextensions-addhttpmessagehandler-1(microsoft-extensions-dependencyinjection-ihttpclientbuilder)

I've warmed up the change on ...

https://github.com/dotnet/blazor-samples/pull/249/files

... in case it should be transient.

@guardrex
Copy link
Collaborator Author

According to the pattern in a couple of Javier sample apps (in his repo), the service reg should be transient. I'm going to go ahead with that choice, which matches the API remarks for AddHttpMessageHandler.

@guardrex guardrex merged commit c73d571 into main Mar 22, 2024
3 checks passed
@guardrex guardrex deleted the guardrex/blazor-cookie-security branch March 22, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cookie security for Call web API article
2 participants