From aa9a86944b7c5fa59e2dc3163f197dc5a3d0e88e Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Sep 2022 14:10:47 -0700 Subject: [PATCH 1/6] Handle application paths for remote calls This change does two things: 1) Add a DelegatingMessageHandler to ensure the app path is included on the request URI 2) Update the RemoteModule to compare the incoming strings against the app relative path that accounts for any application paths or virtual directories. Fixes #191 --- .../RemoteAppClientExtensions.cs | 35 +++++++++++++++++++ .../RemoteModule.cs | 5 ++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs index be325f9d69..fd47692a8b 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs @@ -3,6 +3,8 @@ using System; using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.SystemWebAdapters; using Microsoft.Extensions.Options; @@ -29,6 +31,7 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter builder.Services.AddOptions() .ValidateDataAnnotations(); + builder.Services.AddTransient(); builder.Services.AddHttpClient(RemoteConstants.HttpClientName) .ConfigurePrimaryHttpMessageHandler(sp => { @@ -42,6 +45,7 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter // Disable cookies in the HTTP client because the service will manage the cookie header directly return new HttpClientHandler { UseCookies = false, AllowAutoRedirect = false }; }) + .AddHttpMessageHandler() .ConfigureHttpClient((sp, client) => { var options = sp.GetRequiredService>().Value; @@ -73,6 +77,37 @@ public static ISystemWebAdapterRemoteClientAppBuilder Configure(this ISystemWebA return builder; } + /// + /// is set automatically, but if this is supposed to have a path, then that gets + /// lost. This handler will append that back if necessary. + /// + private class HandleVirtualDirectoryHandler : DelegatingHandler + { + private readonly string? _path; + + public HandleVirtualDirectoryHandler(IOptions options) + { + _path = options.Value.RemoteAppUrl.AbsolutePath; + + if (_path == "/") + { + _path = null; + } + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (_path is not null && request.RequestUri is { } uri) + { + var builder = new UriBuilder(uri); + builder.Path = $"{_path}{builder.Path}"; + request.RequestUri = builder.Uri; + } + + return base.SendAsync(request, cancellationToken); + } + } + private class Builder : ISystemWebAdapterRemoteClientAppBuilder { public Builder(IServiceCollection services) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteModule.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteModule.cs index 4781652e98..518e28177c 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteModule.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteModule.cs @@ -50,11 +50,14 @@ void IHttpModule.Dispose() void IHttpModule.Init(HttpApplication context) { + var appRelativePath = $"~{Path}"; + context.PostMapRequestHandler += (s, _) => { var context = ((HttpApplication)s).Context; - if (!string.Equals(context.Request.Path, Path, StringComparison.Ordinal)) + // Compare against the AppRelativeCurrentExecutionFilePath to account for potential virtual directories + if (!string.Equals(context.Request.AppRelativeCurrentExecutionFilePath, appRelativePath, StringComparison.Ordinal)) { return; } From 18216ab92444015aa9299789aeb7a6838cf05f8d Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Sep 2022 14:16:30 -0700 Subject: [PATCH 2/6] fix name --- .../RemoteAppClientExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs index fd47692a8b..0b3e4c0024 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs @@ -31,7 +31,7 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter builder.Services.AddOptions() .ValidateDataAnnotations(); - builder.Services.AddTransient(); + builder.Services.AddTransient(); builder.Services.AddHttpClient(RemoteConstants.HttpClientName) .ConfigurePrimaryHttpMessageHandler(sp => { @@ -45,7 +45,7 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter // Disable cookies in the HTTP client because the service will manage the cookie header directly return new HttpClientHandler { UseCookies = false, AllowAutoRedirect = false }; }) - .AddHttpMessageHandler() + .AddHttpMessageHandler() .ConfigureHttpClient((sp, client) => { var options = sp.GetRequiredService>().Value; @@ -81,11 +81,11 @@ public static ISystemWebAdapterRemoteClientAppBuilder Configure(this ISystemWebA /// is set automatically, but if this is supposed to have a path, then that gets /// lost. This handler will append that back if necessary. /// - private class HandleVirtualDirectoryHandler : DelegatingHandler + private class VirtualDirectoryHandler : DelegatingHandler { private readonly string? _path; - public HandleVirtualDirectoryHandler(IOptions options) + public VirtualDirectoryHandler(IOptions options) { _path = options.Value.RemoteAppUrl.AbsolutePath; From da727978e11f4f7f8f5a65ba7f3d25b47803a8fe Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Sep 2022 15:02:30 -0700 Subject: [PATCH 3/6] Use relative path to ensure things combine --- .../RemoteAppAuthenticationClientOptions.cs | 8 ++++- .../RemoteAppAuthenticationService.cs | 2 +- .../RelativePathString.cs | 22 +++++++++++++ .../RemoteAppClientExtensions.cs | 33 ------------------- .../RemoteAppClientOptions.cs | 19 ++++++++++- .../RemoteAppSessionStateClientOptions.cs | 9 ++++- .../RemoteAppSessionStateManager.cs | 4 +-- 7 files changed, 58 insertions(+), 39 deletions(-) create mode 100644 src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RelativePathString.cs diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationClientOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationClientOptions.cs index 0efd829c51..ba43f00084 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationClientOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationClientOptions.cs @@ -41,5 +41,11 @@ public class RemoteAppAuthenticationClientOptions : AuthenticationSchemeOptions /// services. Requests to authenticate are sent to this endpoint. /// [Required] - public PathString AuthenticationEndpointPath { get; set; } = AuthenticationConstants.DefaultEndpoint; + public PathString AuthenticationEndpointPath + { + get => Path.Path; + set => Path = new(value); + } + + internal RelativePathString Path { get; private set; } = new(AuthenticationConstants.DefaultEndpoint); } diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationService.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationService.cs index b422bb0a58..1796302772 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationService.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/Authentication/RemoteAppAuthenticationService.cs @@ -85,7 +85,7 @@ public async Task AuthenticateAsync(HttpRequest o // that may matter for authentication. Also include the original request path as // as a query parameter so that the ASP.NET app can redirect back to it if an // authentication provider attempts to redirect back to the authenticate URL. - var url = $"{_options.AuthenticationEndpointPath}?{AuthenticationConstants.OriginalUrlQueryParamName}={WebUtility.UrlEncode(originalRequest.GetEncodedPathAndQuery())}"; + var url = $"{_options.Path.Relative}?{AuthenticationConstants.OriginalUrlQueryParamName}={WebUtility.UrlEncode(originalRequest.GetEncodedPathAndQuery())}"; using var authRequest = new HttpRequestMessage(HttpMethod.Get, url); AddHeaders(_options.RequestHeadersToForward, originalRequest, authRequest); diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RelativePathString.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RelativePathString.cs new file mode 100644 index 0000000000..e87731fdae --- /dev/null +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RelativePathString.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.SystemWebAdapters; + +internal readonly struct RelativePathString +{ + public RelativePathString(PathString path) + { + Path = path; + Relative = "." + path; + } + + public PathString Path { get; } + + /// + /// Use when you want the path to be relative to whatever URI you may combine it with + /// + public string Relative { get; } +} diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs index 0b3e4c0024..4dacc56610 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs @@ -31,7 +31,6 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter builder.Services.AddOptions() .ValidateDataAnnotations(); - builder.Services.AddTransient(); builder.Services.AddHttpClient(RemoteConstants.HttpClientName) .ConfigurePrimaryHttpMessageHandler(sp => { @@ -45,7 +44,6 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter // Disable cookies in the HTTP client because the service will manage the cookie header directly return new HttpClientHandler { UseCookies = false, AllowAutoRedirect = false }; }) - .AddHttpMessageHandler() .ConfigureHttpClient((sp, client) => { var options = sp.GetRequiredService>().Value; @@ -77,37 +75,6 @@ public static ISystemWebAdapterRemoteClientAppBuilder Configure(this ISystemWebA return builder; } - /// - /// is set automatically, but if this is supposed to have a path, then that gets - /// lost. This handler will append that back if necessary. - /// - private class VirtualDirectoryHandler : DelegatingHandler - { - private readonly string? _path; - - public VirtualDirectoryHandler(IOptions options) - { - _path = options.Value.RemoteAppUrl.AbsolutePath; - - if (_path == "/") - { - _path = null; - } - } - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - if (_path is not null && request.RequestUri is { } uri) - { - var builder = new UriBuilder(uri); - builder.Path = $"{_path}{builder.Path}"; - request.RequestUri = builder.Uri; - } - - return base.SendAsync(request, cancellationToken); - } - } - private class Builder : ISystemWebAdapterRemoteClientAppBuilder { public Builder(IServiceCollection services) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs index a08dac34dd..b73b6b8ee7 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs @@ -12,6 +12,8 @@ namespace Microsoft.AspNetCore.SystemWebAdapters; /// public class RemoteAppClientOptions { + private Uri _remoteAppUrl = null!; + /// /// Gets or sets the header used to store the API key /// @@ -28,7 +30,22 @@ public class RemoteAppClientOptions /// Gets or sets the remote app url /// [Required] - public Uri RemoteAppUrl { get; set; } = null!; + public Uri RemoteAppUrl + { + get => _remoteAppUrl; + set + { + // Path must end in '/' so that it will combine correctly with subpaths + if (!value.AbsolutePath.EndsWith("/")) + { + var builder = new UriBuilder(value); + builder.Path += "/"; + value = builder.Uri; + } + + _remoteAppUrl = value; + } + } /// /// Gets or sets an to use for making requests to the remote app. diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateClientOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateClientOptions.cs index f2b7159a59..34ed3a5034 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateClientOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateClientOptions.cs @@ -2,13 +2,20 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession; public class RemoteAppSessionStateClientOptions { [Required] - public string SessionEndpointPath { get; set; } = SessionConstants.SessionEndpointPath; + public PathString SessionEndpointPath + { + get => Path.Path; + set => Path = new(value); + } + + internal RelativePathString Path { get; private set; } = new(SessionConstants.SessionEndpointPath); /// /// Gets or sets the cookie name that the ASP.NET framework app is expecting to hold the session id diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateManager.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateManager.cs index 4095231af3..fdaea50d42 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateManager.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SessionState/RemoteSession/RemoteAppSessionStateManager.cs @@ -73,7 +73,7 @@ private async Task GetSessionDataAsync(string? sessionId, bool re { // The request message is manually disposed at a later time #pragma warning disable CA2000 // Dispose objects before losing scope - var req = new HttpRequestMessage(HttpMethod.Get, _options.SessionEndpointPath); + var req = new HttpRequestMessage(HttpMethod.Get, _options.Path.Relative); #pragma warning restore CA2000 // Dispose objects before losing scope AddSessionCookieToHeader(req, sessionId); @@ -112,7 +112,7 @@ private async Task GetSessionDataAsync(string? sessionId, bool re /// private async Task SetOrReleaseSessionData(ISessionState? state, CancellationToken cancellationToken) { - using var req = new HttpRequestMessage(HttpMethod.Put, _options.SessionEndpointPath); + using var req = new HttpRequestMessage(HttpMethod.Put, _options.Path.Relative); if (state is not null) { From 7520c2b50dcec87baee3b904d4ef4919893dfea7 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Sep 2022 15:08:05 -0700 Subject: [PATCH 4/6] fix test --- .../RemoteAppClientOptionsTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs index ec7c8a1a41..c6f8771019 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs @@ -35,7 +35,11 @@ public void VerifyIsCalled(string apiKeyHeader, string apiKey, string remoteAppU { options.ApiKey = apiKey; options.ApiKeyHeader = apiKeyHeader; - options.RemoteAppUrl = (remoteAppUrl is null ? null : new Uri(remoteAppUrl, UriKind.Absolute))!; + + if (remoteAppUrl is not null) + { + options.RemoteAppUrl = new Uri(remoteAppUrl, UriKind.Absolute); + } })); using var serviceProvider = services.BuildServiceProvider(); From 4c29940282fb7b5a3b08a7e7c971a3bf3ad52d12 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Sep 2022 16:43:50 -0700 Subject: [PATCH 5/6] remove unused usings --- .../RemoteAppClientExtensions.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs index 4dacc56610..be325f9d69 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs @@ -3,8 +3,6 @@ using System; using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; using Microsoft.AspNetCore.SystemWebAdapters; using Microsoft.Extensions.Options; From c1bcc5c79ff068728ecfd51b9988d320792c9603 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Sep 2022 16:44:42 -0700 Subject: [PATCH 6/6] add null check --- .../RemoteAppClientOptions.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs index b73b6b8ee7..ee4e5674cb 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs @@ -35,6 +35,11 @@ public Uri RemoteAppUrl get => _remoteAppUrl; set { + if (value is null) + { + throw new ArgumentNullException(nameof(RemoteAppUrl)); + } + // Path must end in '/' so that it will combine correctly with subpaths if (!value.AbsolutePath.EndsWith("/")) {