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

BlazorWebAssemblyStandaloneWithIdentity uses cookie auth but no antiforgerytoken #31205

Closed
nathan-parkinson opened this issue Dec 5, 2023 · 19 comments
Assignees
Labels
8.0 .NET 8 Blazor doc-enhancement Pri1 Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@nathan-parkinson
Copy link

Description

The BlazorWebAssemblyStandaloneWithIdentity sample uses a cookie to store the authentication token but I believe that this opens the site up to csrf attacks.

This is usually mitigated by using antiforgerytokens but I've found it difficult to apply this to the sample as the api and client run as different sites (although I may have just dome something wrong).

Is is right that antiforgery tokens should be used here or is there a reason it is not required?
If it is required will it be added to this sample or can someone point me in the right direction to get this working?

Page URL

https://learn.microsoft.com/en-us/aspnet/core/blazor/security/webassembly/standalone-with-identity?view=aspnetcore-8.0

Content source URL

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/blazor/security/webassembly/standalone-with-identity.md

Document ID

c4e6ec41-7bea-e600-6473-c5c870aab082

Article author

@guardrex

Copy link
Contributor

github-actions bot commented Dec 5, 2023

🍂🎁 Happy Holidays! ❄️⛄

A green dinosaur 🦖 will be along shortly to assist. Stand-by ........

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Thanks for opening the issue this way. We use the metadata to track work on these.

It should be automatic and transparent. If you look at a request in browser tools, you should see the antiforgery cookie named something like .AspNetCore.Antiforgery.XXXXXXX on each request.

I agree that it would be nice if the article addressed this subject to head off concerns about it.

@JeremyLikness ... Plz confirm for us that devs don't need to sweat XSRF with basic use of this solution, and are there any gotchas 😈 that are worth calling out?

@nathan-parkinson
Copy link
Author

I have run the Backend and BlazorWasmAuth projects, registered and logged in without the Antiforgery cookie being created.

There is no cookie in the dev tools called .AspNetCore.Antiforgery.XXXXXXX. This is, in fact, what I have been trying to implement.

Am I missing something?

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Let me see. I have that test app here ... I'll run it and see what happens. I thought I recalled seeing it, but I might be mistaken. Stand-by ..........................

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Yeah, I see it. Here's a request for the Counter component. Still tho, I do want to hear back from Jeremy on this subject. I think we should say something in the article about it, even if it isn't a concern. If there are any gotchas 😈, we should call them out.

image

@nathan-parkinson
Copy link
Author

Here is mine. I still see no antiforgery token.
Feel like I'm missing something obvious

image

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Let's clarify something ...

Are you running at secure endpoints (https)? How are you running these apps ... command-line dotnet run?

@nathan-parkinson
Copy link
Author

Yes to running https.
No to running from command line. I'm just using F5 in Visual Studio

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Ok ... I just wanted to make sure. You probably know this already, but dotnet run uses the first launch profile, which happens to be insecure HTTP. We've had some issues crop up where the dev just needed to rotate the https launch profile to the top of the list in launchSettings.json (or one can pass -lp https to the command) to get a sample running securely ... thus make security features work properly.

Stand-by for a bit. I'm going to email Jeremy, too, in case he doesn't see the ping here. He has the security chops 👨‍🎓 to explain further any considerations for antiforgery with this particular sample app and probably why I'm seeing an antiforgery cookie but you aren't.

UPDATE: Message sent. He'll be along shortly to assist.

@JeremyLikness
Copy link
Member

Hi,

I will need to make one tweak to secure the logout endpoint:

app.MapPost("/logout", async (
    SignInManager<MyUser> signInManager,
    [FromBody]object empty) =>
{
    if (empty is not null)
    {
        await signInManager.SignOutAsync();
        return Results.Ok();
    }
    return Results.NotFound();
}).RequireAuthorization();

This prevents the endpoint from accepting any forms posts. I spoke with our security team, and they indicated JSON-only endpoints that don't accept form auth or form posts don't need XSRF protections (which is why the MapIdentityEndpoints don't use anti-XSRF). The only way the /logout endpoint above works is if you post a JSON payload of {} which becomes an empty object with no properties but is not null. AFAIK this can't be done from a form.

In ASP.NET Hosted, the anti-XSRF token was rendered in the same page as the client and could be securely grabbed to send back. In standalone Blazor, the client is independent. There is no secure way to send the anti-XSRF token to the client (imagine an endpoint that exposes the token ... but then how do you protect that endpoint?).

There are additional mitigations like SameSite cookies and custom headers and I'll see if those make sense to include in the sample (we definitely should mention them in the documentation).

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Thanks for that understanding, @JeremyLikness.

Let's work it from @nathan-parkinson's issue here. When you're ready, paste content into an issue comment, and I'll place it into the article, edit, and ping you on the PR for review+updates.

WRT to the logout POST endpoint, do you want me to make that update? I can knock that out right now.

@JeremyLikness
Copy link
Member

Go for it!

@guardrex
Copy link
Collaborator

guardrex commented Dec 5, 2023

Ok, that bit is done. I'll add text to the article covering it when I work this issue.

@nathan-parkinson
Copy link
Author

Thank you both for your help with this.

I had wondered how an antiforgery token could work across sites.
It not being required for json only endpoints was the important piece of information I was missing.

Thanks again.

@guardrex
Copy link
Collaborator

guardrex commented Dec 7, 2023

Leaving a note here in case anyone stumbles on this. The code above ☝️ throws the logout into a ☠️ death loop because an error is triggered and the MapPost is executed over and over. I'll get updated code into the sample app as soon as it becomes available. Until then, I reverted the code back to its original form with a code comment to see the sample repo's issue for more information.

dotnet/blazor-samples#132

@JeremyLikness
Copy link
Member

The missing piece is posting the empty content from the client. I'll work on that part (with SPA you just fetch with {} but I'm not sure if for Blazor the solution is to POST with new object() as the payload or I'll need to use JavaScript interop. I'll keep you posted.

@guardrex
Copy link
Collaborator

guardrex commented Dec 8, 2023

Before closing this out, I'll add the text to the article, mostly for localization purposes.

Here are the code remarks ...

// The request checks for an empty body to prevent CSRF attacks. By requiring something
// in the body, the request must be made from JavaScript, which is the only way to
// access the cookie. It can't be accessed by a form-based post.
// This prevents a malicious site from logging the user out.
// Furthermore, the endpoint is protected by authorization to prevent anonymous access.
// The client simply needs to pass an empty object {} in the body of the request.

@guardrex guardrex moved this from Triage to In progress in Blazor.Docs Dec 8, 2023
@guardrex
Copy link
Collaborator

guardrex commented Feb 8, 2024

NOTE TO SELF ... I should be able to close this out with some small updates to the article. We merged the update to the Blazor sample app.

@guardrex guardrex added the 8.0 .NET 8 label Feb 8, 2024
@guardrex guardrex moved this from In progress to 8.0 in Blazor.Docs Feb 8, 2024
@guardrex
Copy link
Collaborator

The logout endpoint was the only endpoint of concern ... and AFAICT analyzing the work that we've done, this is resolved. Closing now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.0 .NET 8 Blazor doc-enhancement Pri1 Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
Archived in project
Development

No branches or pull requests

4 participants