Skip to content

WASM+Identity same-site & antiforgery updates #31888

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

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Feb 22, 2024

Fixes #31389
Addresses #31205
Addresses #28161

Sample updates

FIRST ... before reviewing the article DIFF on this PR ... let's sort out the sample updates at ...

dotnet/blazor-samples#210

  • NIT updates on the PR
    • Move CORS in the pipeline above the logout endpoint.
    • Lowercase the logout endpoint from /Logout to /logout.
    • Change conditional check empty != null to empty is not null.
    • Place a line of spacing above returns ... that's by convention for Blazor example code.
    • Remove null-forgiving ops from email and password, as they can't be null.
    • Remove the UserBasic class, as it isn't used.
  • Same-site/secure policy settings: Add commented-out default-setting code for it.
  • Add a quick-'n-dirty data processing experience.
    • Form in a page that uses the Auth client to POST to the Backend app.
    • Minimal API endpoint that processes the data and returns a string.
    • NOTE: The same errors shown in CORS help blazor-samples#161 are seen here when that code processes an unauthenticated request to the /data-processing endpoint. I'll place the error at the bottom of this OP. We think now that this is resolved by the sample updates.

Article updates

Security Overview

This section applies to all Blazor app hosting models/templates. The primary goal here is to add a reminder that ...

  • Antiforgery mitigation is for form submissions encoded as application/x-www-form-urlencoded, multipart/form-data, or text/plain with a cross-link to the Blazor Forms article coverage on forms antiforgery support. Updated on review.
  • Server API endpoints with application/json-encoded content and CORS don't require CSRF protection. Updated on review.

Standalone with Identity article

  • Adds a new section on Cross-domain hosting (same-site configuration).
  • Adds a new section on antiforgery support.
    • Explain the situation with the logout endpoint.
    • Add the reminders on forms antiforgery and server API/JSON/CORS.

I'm still a bit concerned ... even if this is all correct (or close to correct) ... that the bit about forms submission in a standalone WASM situation could use more work. Using an HTTP client with a JSON request and CORS is what we focus on throughout the docs. This will be the first time that we explicitly say that antiforgery isn't a concern in that scenario. However, EditForm is useful client-side to just collect the data for a JSON POST to a server API. I don't want readers to think that anything is happening for EditForm in such an app vis-a-vis antiforgery. I might need to add more content on this point.

... and BTW Jeremy ... it seems to me that one day if I ever get the time that I should convert your plain forms over to EditForm forms with the nice validation ... and roll in the bits to trigger validation failures with the ValidationMessage component. I can do all that ... it's just a question of TIME ⏲️ ... and I won't have any for the foreseeable future 🏃‍♂️😄.

Error

This has been addressed by the updates to the sample app.

Error for unauthenticated hit on the /data-processing endpoint. What the sample is doing now is trapping it in a try-catch and showing a message. Can't we work out something more graceful than that? ... or does this all resolve back to your remarks here ...

dotnet/blazor-samples#161 (comment)


Internal previews

📄 File 🔗 Preview link
aspnetcore/blazor/security/index.md ASP.NET Core Blazor authentication and authorization
aspnetcore/blazor/security/webassembly/standalone-with-identity.md Secure ASP.NET Core Blazor WebAssembly with ASP.NET Core Identity

Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Error for unauthenticated hit on the /data-processing endpoint. What the sample is doing now is trapping it in a try-catch and showing a message. Can't we work out something more graceful than that?

I left a comment on the blazor-samples PR suggesting we remove the try/catch. If CORS is correctly configured, you should not get any exceptions for CORS errors. I asked some follow up questions on the issue.

Whether the request is authenticated or not should have no impact on how CORS policies are enforced as long as you call .AllowCredentials() and use credentials: 'include' via SetBrowserRequestCredentials(BrowserRequestCredentials.Include) as noted in https://learn.microsoft.com/en-us/aspnet/core/security/cors?view=aspnetcore-8.0#credentials-in-cross-origin-requests. And looking at the latest copy of BlazorWebAssemblyStandaloneWithIdentity, bot things seem to be happening.

Edit: I think it might be a middleware ordering issue.

Thanks @halter73. I'll wait a day for a response to your question ...

> @blowdart @GrabYourPitchforks Do either of you think we should hedge this statement in some way?

Co-authored-by: Stephen Halter <[email protected]>
@guardrex
Copy link
Collaborator Author

Thanks @halter73. I'll wait a day for a response to your question ...

@blowdart @GrabYourPitchforks Do either of you think we should hedge this statement in some way?

I can't wait too long ... I need to unblock things fairly fast these days for more work ⛰️⛏️😅.

@guardrex guardrex merged commit 67b3ce2 into main Mar 1, 2024
@guardrex guardrex deleted the guardrex/blazor-security-updates branch March 1, 2024 11:01
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.

Blazor WebAssembly With Identity same-site coverage
3 participants