-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Offline discussion updates for 8.0 #31693
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
Conversation
:::moniker range=">= aspnetcore-8.0" | ||
|
||
> [!WARNING] | ||
> Only place a `<script>` tag in a component file (`.razor`) if the component is guaranteed to adopt [static server-side rendering (static SSR)](xref:blazor/fundamentals/index#client-and-server-rendering-concepts) because the `<script>` tag can't be updated dynamically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javiercn Is this warning worded strong enough for you? I remember you complaining about people trying to render <script>
tags in triage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me. The reason for not using script tags in interactive components is clearly stated.
Thanks, @halter73 ... but you didn't answer Dan's question in the opening remarks ☝️ (unless by your approval you're saying 'don't sweat CSRF here 😎'). |
I think we should recommend using an auth cookie because that's the only way to prerender content that requires authentication. As for the CSRF concerns, I think we should just link to https://learn.microsoft.com/en-us/aspnet/core/security/anti-request-forgery?view=aspnetcore-8.0. All Blazor endpoints that accept form posts that might be susceptible to CSRF attacks already require an antiforgery token as mentioned in https://learn.microsoft.com/en-us/aspnet/core/blazor/security/?view=aspnetcore-8.0#antiforgery-support, so I don't think there's much to add there. Pinging @javiercn @blowdart in case they disagree Other types of endpoints that accept form posts either require antiforgery tokens by default (like razor pages, controllers with views, and minimal APIs) or have been long documented not to (like |
It will be a bit out-of-sync with our SignalR guidance ...
We seem to have some cookie bits at ... https://learn.microsoft.com/en-us/aspnet/signalr/overview/security/hub-authorization#cookie @halter73 ... Do you want me to try to 🦖 RexHack™ 🙈 some kind of approach based on that, or do we want a real programmer to build out what we'll show for the cookie approach in the SignalR doc? BTW ... This is the current code that we show is, which I suppose we can maintain for standalone WASM apps ... @using Microsoft.AspNetCore.Components.WebAssembly.Authentication
@inject IAccessTokenProvider TokenProvider
@inject NavigationManager Navigation
...
var tokenResult = await TokenProvider.RequestAccessToken();
if (tokenResult.TryGetToken(out var token))
{
hubConnection = new HubConnectionBuilder()
.WithUrl(Navigation.ToAbsoluteUri("/chathub"),
options => { options.AccessTokenProvider = () => Task.FromResult(token?.Value); })
.Build();
...
} |
🦖 NOTE TO SELF
Resolved! ...We already had the link to the Client-side SignalR cross-origin negotiation for authentication section in the Blazor-SignalR tutorial. |
I think it's fine to continue recommend tokens for cases where there isn't prerendering like standalone WASM or non-browser apps. In those kind of apps, you don't have a browser window to login using Razor pages/components. You can have a .NET HttpClient login using cookies with If you're up for it, I'm more than happy to review 🦖 RexHack™ version of cookie based SignalR auth using a .NET client. I expect instead of |
@halter73 ... Indeed! I forgot that section was there. It seems to me that we can just cross-link that from the security article, as I just set up on ... WRT ...
I already conversed with @javiercn on that point, and that's about what he told me to write into the docs. WRT anything else ... @danroth27 / @mkArtakMSFT ... are you OK with going ahead and getting this merged? |
It looks like the gRPC sample code isn't used to initialize any component state, so yeah, I think it's fine as is. No need to persist and use prerendered state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #31688
Addresses #28161
Items to be worked on separate issues
Transient disposable section updates: I have everything I need to resolve this, and I should reach it on Tuesday 🤞🍀.UPDATE: This is resolved now.Configure and use gRPC in components section
Dan ... You said you'd provide a code example, but it 🤔 seems like the current code is ok. The updates to the section are on the DIFF, so you can see the current code there. The focus of the updates thus far is just to 🔪 OUT the little bits of text that pertain to hosted WASM and updating the versioning to return the content for 8.0+. Let me know if I should 🛑 HOLD this PR for new or additional code (or open a separate issue for it). 👂👂👂
Secure a SignalR hub section
Dan asks Stephen/Jeremy ...
The updates to the section are on the DIFF, so the current code is there for inspection. Thus far ... again ... the updates on the PR just focus on 🔪 OUT the little bits pertaining to hosted WASM and updating the versioning to return the content for 8.0+.
Skipped bits
Everything else that we discussed that isn't on the PR doesn't require updates because the current coverage is good.
Internal previews