-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Make enhanced nav much more conservative, and better handle redirecti…
…ons (#50551) This PR makes enhanced navigation/forms **much** more conservative: * For links, enhanced nav remains on by default, but: * It's now easy to turn it off or back on hierarchically or on a per-link basis using a `data-enhance-nav` attribute in HTML. * If the destination turns out to be a non-Blazor endpoint, enhanced nav will not apply, and the client-side JS will retry as a full page load. This is to ensure we don't confuse other pages that aren't designed to be patched into an existing page alongside the Blazor JS logic. * For posts, enhanced nav is now off by default * You turn it on by adding `data-enhance` to the form element or `Enhance` if it's an `EditForm` (they're equivalent). This is nonhierarchical since it's not desirable to enable this globally. * If the destination turns out to be a non-Blazor endpoint, enhanced nav considers this an error (same reason as for links above). But unlike with links, form posts *won't* retry since it's not safe to do that for POST requests. Developers shouldn't enhance forms that post to non-Blazor endpoints. * Redirection handling is much more comprehensive * This is way more tricky than I expected. Allowing for combinations of GET/POST, enhanced nav, streaming, Blazor and non-Blazor endpoints, internal and external redirection destinations turns out to break down into 16 separate cases, which you can find [listed in this E2E page](https://github.com/dotnet/aspnetcore/pull/50551/files#diff-477ec16959ae9dddaca3e76e1e40c4d70147210d0fde068357d9ad3e74896337). * 15 of those 16 cases now work automatically without errors. The remaining case is "form with enhanced nav does POST to a non-Blazor endpoint which redirects to an external URL", which cannot be supported without having some global server-side logic for all endpoints (not just Blazor ones), so that case explicitly throws a clear error telling the developer not to apply enhanced nav to forms that go to non-Blazor endpoints that redirect to external URLs. * There are various subtleties about making the "Back" button work correctly after these enhanced redirections, but I think all of them are covered properly now. The E2E cases all validate clicking "Back" afterwards. * Previously it was possible for an internal URL to redirect to an external URL that enabled CORS for your site, and then we would have received and merged that external content into your page. That's no longer the case since we now put `no-cors` on the fetch request, so it should now be impossible to receive content from an external origin (and it falls back on retrying as a full-page load for GET, or an error with a clear message for POST). * Disclosure of redirection URLs is now avoided. * Previously, for external or streaming redirections, we told the client-side JS code which URL to redirect to. That disclosed more info than a `fetch` normally would. As of this PR, the URL is data protected so all the client can do is navigate to an endpoint that issues a real 302 to the real URL. As such JS no longer has access to any more info than it normally would with a `fetch`. With all this, it should be much less likely for enhanced nav/redirections to run into incompatibility issues. And much easier for developers to disable it for whole regions in the UI if they want.
- Loading branch information
1 parent
f4bcd4f
commit 0005026
Showing
69 changed files
with
899 additions
and
191 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Security.Cryptography; | ||
using System.Text.Encodings.Web; | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.AspNetCore.DataProtection; | ||
using Microsoft.AspNetCore.Http; | ||
using Microsoft.AspNetCore.Routing; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.Extensions.Options; | ||
|
||
namespace Microsoft.AspNetCore.Components.Endpoints; | ||
|
||
internal partial class OpaqueRedirection | ||
{ | ||
// During streaming SSR, a component may try to perform a redirection. Since the response has already started | ||
// this can only work if we communicate the redirection back via some command that can get handled by JS, | ||
// rather than a true 301/302/etc. But we don't want to disclose the redirection target URL to JS because that | ||
// info would not normally be available, e.g., when using 'fetch'. So we data-protect the URL and round trip | ||
// through a special endpoint that can issue a true redirection. | ||
// | ||
// The same is used during enhanced navigation if it happens to go to a Blazor endpoint that calls | ||
// NavigationManager.NavigateTo, for the same reasons. | ||
// | ||
// However, if enhanced navigation goes to a non-Blazor endpoint, the server won't do anything special and just | ||
// returns a regular 301/302/etc. To handle this, | ||
// | ||
// - If it's redirected to an internal URL, the browser will just follow the redirection automatically | ||
// and client-side code will then: | ||
// - Check if it went to a Blazor endpoint, and if so, simply update the client-side URL to match | ||
// - Or if it's a non-Blazor endpoint, behaves like "external URL" below | ||
// - If it's to an external URL: | ||
// - If it's a GET request, the client-side code will retry as a non-enhanced request | ||
// - For other request types, we have to let it fail as it would be unsafe to retry | ||
|
||
private const string RedirectionDataProtectionProviderPurpose = "Microsoft.AspNetCore.Components.Endpoints.OpaqueRedirection,V1"; | ||
private const string RedirectionEndpointBaseRelativeUrl = "_framework/opaque-redirect"; | ||
|
||
public static string CreateProtectedRedirectionUrl(HttpContext httpContext, string destinationUrl) | ||
{ | ||
var protector = CreateProtector(httpContext); | ||
var options = httpContext.RequestServices.GetRequiredService<IOptions<RazorComponentsServiceOptions>>(); | ||
var lifetime = options.Value.TemporaryRedirectionUrlValidityDuration; | ||
var protectedUrl = protector.Protect(destinationUrl, lifetime); | ||
return $"{RedirectionEndpointBaseRelativeUrl}?url={UrlEncoder.Default.Encode(protectedUrl)}"; | ||
} | ||
|
||
public static void AddBlazorOpaqueRedirectionEndpoint(IEndpointRouteBuilder endpoints) | ||
{ | ||
endpoints.MapGet($"/{RedirectionEndpointBaseRelativeUrl}", httpContext => | ||
{ | ||
if (!httpContext.Request.Query.TryGetValue("url", out var protectedUrl)) | ||
{ | ||
httpContext.Response.StatusCode = 400; | ||
return Task.CompletedTask; | ||
} | ||
|
||
var protector = CreateProtector(httpContext); | ||
string url; | ||
|
||
try | ||
{ | ||
url = protector.Unprotect(protectedUrl[0]!); | ||
} | ||
catch (CryptographicException ex) | ||
{ | ||
if (httpContext.RequestServices.GetService<ILogger<OpaqueRedirection>>() is { } logger) | ||
{ | ||
Log.OpaqueUrlUnprotectionFailed(logger, ex); | ||
} | ||
|
||
httpContext.Response.StatusCode = 400; | ||
return Task.CompletedTask; | ||
} | ||
|
||
httpContext.Response.Redirect(url); | ||
return Task.CompletedTask; | ||
}); | ||
} | ||
|
||
private static ITimeLimitedDataProtector CreateProtector(HttpContext httpContext) | ||
{ | ||
var dataProtectionProvider = httpContext.RequestServices.GetRequiredService<IDataProtectionProvider>(); | ||
return dataProtectionProvider.CreateProtector(RedirectionDataProtectionProviderPurpose).ToTimeLimitedDataProtector(); | ||
} | ||
|
||
public static partial class Log | ||
{ | ||
[LoggerMessage(1, LogLevel.Information, "Opaque URL unprotection failed.", EventName = "OpaqueUrlUnprotectionFailed")] | ||
public static partial void OpaqueUrlUnprotectionFailed(ILogger<OpaqueRedirection> logger, Exception exception); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.