From 83a03c981c99446e01f305c1e8a2a65802a7e2a2 Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Wed, 24 Aug 2022 15:27:34 -0400 Subject: [PATCH 1/7] Ensure that API Key for remote app configuration is a guid --- samples/ClassLibrary/RemoteServiceUtils.cs | 3 ++- samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs | 3 ++- samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs | 3 ++- samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs | 3 ++- samples/RemoteAuth/Forms/FormsAuthCore/Program.cs | 4 +++- samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs | 3 ++- samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs | 3 ++- .../RemoteAppClientExtensions.cs | 3 ++- .../RemoteAppServerExtensions.cs | 4 ++-- src/Services/RemoteAppOptions.cs | 7 ++++++- .../Authentication/RemoteAppOptionsTests.cs | 5 ++++- 11 files changed, 29 insertions(+), 12 deletions(-) diff --git a/samples/ClassLibrary/RemoteServiceUtils.cs b/samples/ClassLibrary/RemoteServiceUtils.cs index 6e8cb4cafa..b66d67abb7 100644 --- a/samples/ClassLibrary/RemoteServiceUtils.cs +++ b/samples/ClassLibrary/RemoteServiceUtils.cs @@ -5,7 +5,8 @@ namespace ClassLibrary; public class RemoteServiceUtils { - public static string ApiKey = "test-key"; + // Do not re-use this ApiKey; every solution should use a unique ApiKey + public static string ApiKey = "54b69938-90dd-4f79-adcd-27fbd6f0e4b7"; public static void RegisterSessionKeys(IDictionary knownTypes) { diff --git a/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs b/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs index f22709585e..ccf6167caa 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs +++ b/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs @@ -17,7 +17,8 @@ protected void Application_Start() { // A real application would not hard code this, but load it // securely from environment or configuration - options.ApiKey = "TopSecretString"; + // Do not re-use this ApiKey; every solution should use a unique ApiKey + options.ApiKey = "40c807bd-6c00-4e5a-9650-ea20c2e6c02d"; }) .AddAuthentication()); } diff --git a/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs b/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs index cfe1365a81..c5cbde885b 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs +++ b/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs @@ -13,7 +13,8 @@ // A real application would not hard code this, but load it // securely from environment or configuration - options.ApiKey = "TopSecretString"; + // Do not re-use this ApiKey; every solution should use a unique ApiKey + options.ApiKey = "40c807bd-6c00-4e5a-9650-ea20c2e6c02d"; }) // This registers the remote app authentication handler. The boolean argument indicates whether remote app auth diff --git a/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs b/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs index 883d1100f6..b5d0ae41ef 100644 --- a/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs +++ b/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs @@ -19,7 +19,8 @@ void Application_Start(object sender, EventArgs e) .AddRemoteAppServer(remote => remote .Configure(options => { - options.ApiKey = "FormsAuthSampleKey"; + // Do not re-use this ApiKey; every solution should use a unique ApiKey + options.ApiKey = "8e470586-24e5-4f2a-8245-69bbdbf9f767"; }) .AddAuthentication()); } diff --git a/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs b/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs index b73d5f8c4b..586c0dd006 100644 --- a/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs +++ b/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs @@ -11,7 +11,9 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - options.ApiKey = "FormsAuthSampleKey"; + + // Do not re-use this ApiKey; every solution should use a unique ApiKey + options.ApiKey = "8e470586-24e5-4f2a-8245-69bbdbf9f767"; }) .AddAuthentication(true)); diff --git a/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs b/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs index 9a7ef3731b..3fdddb92c1 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs +++ b/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs @@ -17,7 +17,8 @@ protected void Application_Start() SystemWebAdapterConfiguration.AddSystemWebAdapters(this) .AddProxySupport(options => options.UseForwardedHeaders = true) .AddRemoteAppServer(remote => remote - .Configure(options => options.ApiKey = "test-key") + // Do not re-use this ApiKey; every solution should use a unique ApiKey + .Configure(options => options.ApiKey = "121257f2-c121-4f51-b30c-d1f617933290") .AddAuthentication()); } diff --git a/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs b/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs index a6a090304b..25cc8ba6a2 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs +++ b/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs @@ -5,7 +5,8 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - options.ApiKey = "test-key"; + // Do not re-use this ApiKey; every solution should use a unique ApiKey + options.ApiKey = "121257f2-c121-4f51-b30c-d1f617933290"; }) .AddAuthentication(true)); diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs index ceb2133f20..468e269278 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs @@ -25,7 +25,8 @@ public static ISystemWebAdapterBuilder AddRemoteAppClient(this ISystemWebAdapter } builder.Services.AddOptions() - .ValidateDataAnnotations(); + .ValidateDataAnnotations() + .Validate(RemoteAppOptions.Validate, "ApiKey must be a non-empty GUID"); configure(new Builder(builder.Services)); diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerExtensions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerExtensions.cs index c3eed37672..5483262ad0 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerExtensions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerExtensions.cs @@ -25,8 +25,8 @@ public static ISystemWebAdapterBuilder AddRemoteAppServer(this ISystemWebAdapter } var options = builder.Services.AddOptions() - .Validate(options => !string.IsNullOrEmpty(options.ApiKey), "ApiKey must be set") - .Validate(options => !string.IsNullOrEmpty(options.ApiKeyHeader), "ApiKeyHeader must be set"); + .Validate(options => !string.IsNullOrEmpty(options.ApiKeyHeader), "ApiKeyHeader must be set") + .Validate(RemoteAppOptions.Validate, "ApiKey must be a non-empty GUID"); configure(new Builder(builder.Services)); diff --git a/src/Services/RemoteAppOptions.cs b/src/Services/RemoteAppOptions.cs index 92f558f61b..1ac026cb38 100644 --- a/src/Services/RemoteAppOptions.cs +++ b/src/Services/RemoteAppOptions.cs @@ -1,8 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; #if NET6_0_OR_GREATER +using System; using System.ComponentModel.DataAnnotations; using System.Net.Http; #endif @@ -45,4 +45,9 @@ public class RemoteAppOptions /// public HttpClient? BackchannelHttpClient { get; set; } #endif + + // To ensure secure API keys are used, enforce that + // keys are parsable as GUIDs + public static bool Validate(RemoteAppOptions options) => + Guid.TryParse(options.ApiKey, out var keyGuid) && keyGuid != Guid.Empty; } diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs index 63566e9e96..5dc4f2959f 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs @@ -10,7 +10,10 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.Tests; public class RemoteAppOptionsTests { - [InlineData("HeaderName", "MyKey", true)] + [InlineData("HeaderName", "36705d36-eba0-44f9-a2e0-c57e6f521274", true)] + [InlineData(null, "36705d36-eba0-44f9-a2e0-c57e6f521274", false)] + [InlineData("", "36705d36-eba0-44f9-a2e0-c57e6f521274", false)] + [InlineData("HeaderName", "MyKey", false)] [InlineData(null, "MyKey", false)] [InlineData("", "MyKey", false)] [InlineData("HeaderName", null, false)] From 73279446122d7f159db8ac0681ee18b2bd5b350c Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Wed, 24 Aug 2022 15:45:43 -0400 Subject: [PATCH 2/7] Update docs with security best practices for remote app connections --- docs/adapters.md | 2 +- docs/core.md | 1 - .../remote-authentication.md | 17 ++++++++++++++--- docs/session-state/remote-session.md | 15 +++++++++++++-- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/docs/adapters.md b/docs/adapters.md index c1b395c3e0..8f95fa6dc7 100644 --- a/docs/adapters.md +++ b/docs/adapters.md @@ -48,4 +48,4 @@ public class SomeController : Controller } ``` -Notice that since there's a `Controller.Context` property, they can pass that through, but it generally looks the same. Using implicit conversions, the `Microsoft.AspNetCore.Http.HttpContext` can be converted into the adapter that could then be passed around through the levels utilizing the code in the same way. \ No newline at end of file +Notice that since there's a `Controller.Context` property, they can pass that through, but it generally looks the same. Using implicit conversions, the `Microsoft.AspNetCore.Http.HttpContext` can be converted into the adapter that could then be passed around through the levels utilizing the code in the same way. diff --git a/docs/core.md b/docs/core.md index 6031086be4..0256824e50 100644 --- a/docs/core.md +++ b/docs/core.md @@ -42,4 +42,3 @@ app.Run(); ``` This opts into all the behavior (described below), as well as sets up a remote session state (see [here](session-state/session.md) for details). - diff --git a/docs/remote-authentication/remote-authentication.md b/docs/remote-authentication/remote-authentication.md index 20ec016ace..f604248f4d 100644 --- a/docs/remote-authentication/remote-authentication.md +++ b/docs/remote-authentication/remote-authentication.md @@ -15,12 +15,13 @@ SystemWebAdapterConfiguration.AddSystemWebAdapters(this) .AddProxySupport(options => options.UseForwardedHeaders = true) .AddRemoteApp(options => { - options.ApiKey = "MySecretKey"; + // ApiKey is a string representing a GUID + options.ApiKey = "00000000-0000-0000-0000-000000000000"; }) .AddRemoteAppAuthentication(); ``` -In the options configuration method passed to the `AddRemoteApp` call, you must specify an API key which is used to secure the endpoint so that only trusted callers can make requests to it (this same API key will be provided to the ASP.NET Core app when it is configured). +In the options configuration method passed to the `AddRemoteApp` call, you must specify an API key which is used to secure the endpoint so that only trusted callers can make requests to it (this same API key will be provided to the ASP.NET Core app when it is configured). The API key is a string and must be parsable as a GUID (128-bit hex number). Hyphens in the key are optional. ### ASP.NET Core app configuration @@ -31,7 +32,9 @@ builder.Services.AddSystemWebAdapters() .AddRemoteApp(options => { options.RemoteAppUrl = new(builder.Configuration["http://URL-for-the-ASPNet-app"]); - options.ApiKey = "MySecretKey"; + + // ApiKey is a string representing a GUID + options.ApiKey = "00000000-0000-0000-0000-000000000000"; }) .AddRemoteAppAuthentication(true); ``` @@ -52,6 +55,14 @@ Finally, if the ASP.NET Core app didn't previously include authentication middle app.UseAuthentication(); ``` +## Securing the remote app connection + +Because remote app authentication involves serving requests on a new endpoint from the ASP.NET app, it's important that communication to and from the ASP.NET app be secure. + +First, make sure that the API key string used to authenticate the ASP.NET Core app with the ASP.NET app is unique and kept secret. It is a best practice to not store the API key in source control. Instead, load it at runtime from a secure source such as Azure Key Vault or other secure runtime configuration. In order to encourage secure API keys, remote app connections require that the keys be non-empty GUIDs (128-bit hex numbers). + +Second, because it's important for the ASP.NET Core app to be able to trust that it is requesting identity information from the correct ASP.NET app, the ASP.NET app should use HTTPS in any production scenarios so that the ASP.NET Core app can know identity is being served by a trusted source. + ## Design 1. When requests are processed by the ASP.NET Core app, if remote app authentication is the default scheme or specified by the request's endpoint, the `RemoteAuthenticationAuthHandler` will attempt to authenticate the user. diff --git a/docs/session-state/remote-session.md b/docs/session-state/remote-session.md index e1a905217b..dc799e9f7c 100644 --- a/docs/session-state/remote-session.md +++ b/docs/session-state/remote-session.md @@ -31,7 +31,8 @@ builder.Services.AddSystemWebAdapters() options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); // Provide a strong API key that will be used to authenticate the request on the remote app for querying the session - options.ApiKey = "strong-api-key"; + // ApiKey is a string representing a GUID + options.ApiKey = "00000000-0000-0000-0000-000000000000"; }) .AddRemoteAppSession() .AddJsonSessionSerializer(options => @@ -66,7 +67,8 @@ The framework equivalent would look like the following change in `Global.asax.cs ```csharp SystemWebAdapterConfiguration.AddSystemWebAdapters(this) // Provide a strong API key that will be used to authenticate the request on the remote app for querying the session - .AddRemoteApp(options => options.ApiKey = "strong-api-key") + // ApiKey is a string representing a GUID + .AddRemoteApp(options => options.ApiKey = "00000000-0000-0000-0000-000000000000") .AddRemoteAppSession() .AddJsonSessionSerializer(options => { @@ -75,6 +77,15 @@ SystemWebAdapterConfiguration.AddSystemWebAdapters(this) options.RegisterKey("SampleSessionItem"); }); ``` + +## Securing the remote app connection + +Because remote session involves serving requests on a new endpoint from the ASP.NET app, it's important that communication to and from the ASP.NET app be secure. + +First, make sure that the API key string used to authenticate the ASP.NET Core app with the ASP.NET app is unique and kept secret. It is a best practice to not store the API key in source control. Instead, load it at runtime from a secure source such as Azure Key Vault or other secure runtime configuration. In order to encourage secure API keys, remote app connections require that the keys be non-empty GUIDs (128-bit hex numbers). + +Second, because it's important for the ASP.NET Core app to be able to trust that it is requesting session information from the correct ASP.NET app, the ASP.NET app should use HTTPS in any production scenarios so that the ASP.NET Core app can know responses are being served by a trusted source. + # Protocol ## Readonly From dc1f0a273b09b4a63889d2c3780851303d7419a3 Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Wed, 24 Aug 2022 16:30:43 -0400 Subject: [PATCH 3/7] Pull remote app setup instructions into dedicated doc --- docs/getting_started.md | 2 +- docs/remote-app-setup.md | 51 +++++++++++++++++++ .../remote-authentication.md | 18 ++----- docs/session-state/remote-session.md | 16 +----- src/Services/RemoteAppOptions.cs | 6 ++- 5 files changed, 62 insertions(+), 31 deletions(-) create mode 100644 docs/remote-app-setup.md diff --git a/docs/getting_started.md b/docs/getting_started.md index 4eb14c2583..6e510e33a2 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -41,7 +41,7 @@ Session is a commonly used feature of ASP.NET that shares a name with a feature ## Enable shared authentication support -It is possible to share authentication between the original ASP.NET app and the new ASP.NET Core app by using the System.Web adapters remote authentication feature. This feature allows the ASP.NET Core app to defer authentication to the ASP.NET app. Please see the [remote authentication docs]((remote-authentication/remote-authentication.md)) for more details. +It is possible to share authentication between the original ASP.NET app and the new ASP.NET Core app by using the System.Web adapters remote authentication feature. This feature allows the ASP.NET Core app to defer authentication to the ASP.NET app. Please see the [remote app connection](remote-app-setup.md) and [remote authentication]((remote-authentication/remote-authentication.md)) docs for more details. ## General Usage Guidance diff --git a/docs/remote-app-setup.md b/docs/remote-app-setup.md new file mode 100644 index 0000000000..b880e36947 --- /dev/null +++ b/docs/remote-app-setup.md @@ -0,0 +1,51 @@ +# Remote app setup + +In some incremental upgrade scenarios, it's useful for the new ASP.NET Core app to be able to communicate with the original ASP.NET app. + +Specifically, this capability is used, currently, for [remote app authentication](remote-authentication/remote-authentication.md) and [remote session](session-state/remote-session.md) features. + +## Configuration + +To enable the ASP.NET Core app to communicate with the ASP.NET app, it's necessary to make a couple small changes to each app. + +### ASP.NET app configuration + +To setup the ASP.NET app to be able to receive requests from the ASP.NET Core app, call the `AddRemoteApp` extension method on the `ISystemWebAdapterBuilder` as shown here. + +```CSharp +SystemWebAdapterConfiguration.AddSystemWebAdapters(this) + .AddRemoteApp(options => + { + // ApiKey is a string representing a GUID + options.ApiKey = "00000000-0000-0000-0000-000000000000"; + }); +``` + +In the options configuration method passed to the `AddRemoteApp` call, you must specify an API key which is used to secure the endpoint so that only trusted callers can make requests to it (this same API key will be provided to the ASP.NET Core app when it is configured). The API key is a string and must be parsable as a GUID (128-bit hex number). Hyphens in the key are optional. + +### ASP.NET Core app + +To setup the ASP.NET Core app to be able to send requests to the ASP.NET app, you need to make a similar change, calling `AddRemoteApp` after registering System.Web adapter services with `AddSystemWebAdapters`. + +```CSharp +builder.Services.AddSystemWebAdapters() + .AddRemoteApp(options => + { + options.RemoteAppUrl = new(builder.Configuration["http://URL-for-the-ASPNet-app"]); + + // ApiKey is a string representing a GUID + options.ApiKey = "00000000-0000-0000-0000-000000000000"; + }); +``` + +The `AddRemoteApp` call is used to configure the remote app's URL and the shared secret API key. + +With both the ASP.NET and ASP.NET Core app updated, extension methods can now be used to setup [remote app authentication](remote-authentication/remote-authentication.md) or [remote session](session-state/remote-session.md), as needed. + +## Securing the remote app connection + +Because remote app features involve serving requests on new endpoints from the ASP.NET app, it's important that communication to and from the ASP.NET app be secure. + +First, make sure that the API key string used to authenticate the ASP.NET Core app with the ASP.NET app is unique and kept secret. It is a best practice to not store the API key in source control. Instead, load it at runtime from a secure source such as Azure Key Vault or other secure runtime configuration. In order to encourage secure API keys, remote app connections require that the keys be non-empty GUIDs (128-bit hex numbers). + +Second, because it's important for the ASP.NET Core app to be able to trust that it is requesting information from the correct ASP.NET app, the ASP.NET app should use HTTPS in any production scenarios so that the ASP.NET Core app can know responses are being served by a trusted source. diff --git a/docs/remote-authentication/remote-authentication.md b/docs/remote-authentication/remote-authentication.md index f604248f4d..65f7f5384c 100644 --- a/docs/remote-authentication/remote-authentication.md +++ b/docs/remote-authentication/remote-authentication.md @@ -6,9 +6,11 @@ The System.Web adapter's remote authentication feature allows an ASP.NET Core ap There are just a few small code changes needed to enable remote authentication in a solution that's already set up according to the [Getting Started](../getting_started.md). +First, follow the [remote app setup](../remote-app-setup.md) instructions to connect the ASP.NET Core and ASP.NET apps. Then, there are just a couple extra extension methods to call to enable remote app authentication. + ### ASP.NET app configuration -First, the ASP.NET app needs to be configured to add the authentication endpoint. This is done by calling the `AddRemoteApp` extension method on the `ISystemWebAdapterBuilder` to configure receiving remote calls, and by calling `AddRemoteAuthentication` to set up the HTTP module that will watch for requests to the authentication endpoint. Note that remote authentication scenarios typically want to add proxy support, as well, so that any auth-related redirects will correctly route to the ASP.NET Core app rather than the ASP.NET one. +First, the ASP.NET app needs to be configured to add the authentication endpoint. This is done by calling the `AddRemoteAuthentication` extension method to set up the HTTP module that will watch for requests to the authentication endpoint. Note that remote authentication scenarios typically want to add proxy support, as well, so that any auth-related redirects will correctly route to the ASP.NET Core app rather than the ASP.NET one. ```CSharp SystemWebAdapterConfiguration.AddSystemWebAdapters(this) @@ -21,11 +23,9 @@ SystemWebAdapterConfiguration.AddSystemWebAdapters(this) .AddRemoteAppAuthentication(); ``` -In the options configuration method passed to the `AddRemoteApp` call, you must specify an API key which is used to secure the endpoint so that only trusted callers can make requests to it (this same API key will be provided to the ASP.NET Core app when it is configured). The API key is a string and must be parsable as a GUID (128-bit hex number). Hyphens in the key are optional. - ### ASP.NET Core app configuration -Next, the ASP.NET Core app needs to be configured to enable the authentication handler that will authenticate users by making an HTTP request to the ASP.NET app. Again, this is done by calling `AddRemoteApp` and `AddRemoteAppAuthentication` when registering System.Web adapters services: +Next, the ASP.NET Core app needs to be configured to enable the authentication handler that will authenticate users by making an HTTP request to the ASP.NET app. Again, this is done by calling `AddRemoteAppAuthentication` when registering System.Web adapters services: ```CSharp builder.Services.AddSystemWebAdapters() @@ -39,8 +39,6 @@ builder.Services.AddSystemWebAdapters() .AddRemoteAppAuthentication(true); ``` -The `AddRemoteApp` call is used to configure the remote app's URL and the shared secret API key. - The boolean that is passed to the `AddRemoteAuthentication` call specifies whether remote app authentication should be the default authentication scheme. Passing `true` will cause the user to be authenticated via remote app authentication for all requests, whereas passing `false` means that the user will only be authenticated with remote app authentication if the remote app scheme is specifically requested (with `[Authorize(AuthenticationSchemes = RemoteAppAuthenticationDefaults.AuthenticationScheme)]` on a controller or action method, for example). Passing false for this parameter has the advantage of only making HTTP requests to the original ASP.NET app for authentication for endpoints that require remote app authentication but has the disadvantage of requiring annotating all such endpoints to indicate that they will use remote app auth. In addition to the require boolean, an optional callback may be passed to `AddRemoteAppAuthentication` to modify some other aspects of the remote authentication process's behavior: @@ -55,14 +53,6 @@ Finally, if the ASP.NET Core app didn't previously include authentication middle app.UseAuthentication(); ``` -## Securing the remote app connection - -Because remote app authentication involves serving requests on a new endpoint from the ASP.NET app, it's important that communication to and from the ASP.NET app be secure. - -First, make sure that the API key string used to authenticate the ASP.NET Core app with the ASP.NET app is unique and kept secret. It is a best practice to not store the API key in source control. Instead, load it at runtime from a secure source such as Azure Key Vault or other secure runtime configuration. In order to encourage secure API keys, remote app connections require that the keys be non-empty GUIDs (128-bit hex numbers). - -Second, because it's important for the ASP.NET Core app to be able to trust that it is requesting identity information from the correct ASP.NET app, the ASP.NET app should use HTTPS in any production scenarios so that the ASP.NET Core app can know identity is being served by a trusted source. - ## Design 1. When requests are processed by the ASP.NET Core app, if remote app authentication is the default scheme or specified by the request's endpoint, the `RemoteAuthenticationAuthHandler` will attempt to authenticate the user. diff --git a/docs/session-state/remote-session.md b/docs/session-state/remote-session.md index dc799e9f7c..da36977ac7 100644 --- a/docs/session-state/remote-session.md +++ b/docs/session-state/remote-session.md @@ -16,12 +16,9 @@ builder.Services.AddSystemWebAdapters() ## Configuration -In order to configure it, both the framework and core app must set an API key as well as register known app settings types. These properties are: +First, follow the [remote app setup](../remote-app-setup.md) instructions to connect the ASP.NET Core and ASP.NET apps. Then, there are just a couple extra extension methods to call to enable remote app session state. -- `ApiKeyHeader` - header name that will contain an API key to secure the endpoint added on .NET Framework -- `ApiKey` - the shared API key that will be validated in the .NET Framework handler - -Configuration for ASP.NET Core would look similar to the following: +Configuration for ASP.NET Core involves calling `AddRemoteAppSession` and `AddJsonSessionSerializer` to register known session item types. The code should look similar to the following: ```csharp builder.Services.AddSystemWebAdapters() @@ -61,7 +58,6 @@ app.MapDefaultControllerRoute() .RequireSystemWebAdapterSession(); ``` - The framework equivalent would look like the following change in `Global.asax.cs`: ```csharp @@ -78,14 +74,6 @@ SystemWebAdapterConfiguration.AddSystemWebAdapters(this) }); ``` -## Securing the remote app connection - -Because remote session involves serving requests on a new endpoint from the ASP.NET app, it's important that communication to and from the ASP.NET app be secure. - -First, make sure that the API key string used to authenticate the ASP.NET Core app with the ASP.NET app is unique and kept secret. It is a best practice to not store the API key in source control. Instead, load it at runtime from a secure source such as Azure Key Vault or other secure runtime configuration. In order to encourage secure API keys, remote app connections require that the keys be non-empty GUIDs (128-bit hex numbers). - -Second, because it's important for the ASP.NET Core app to be able to trust that it is requesting session information from the correct ASP.NET app, the ASP.NET app should use HTTPS in any production scenarios so that the ASP.NET Core app can know responses are being served by a trusted source. - # Protocol ## Readonly diff --git a/src/Services/RemoteAppOptions.cs b/src/Services/RemoteAppOptions.cs index 1ac026cb38..01e9c8cbfa 100644 --- a/src/Services/RemoteAppOptions.cs +++ b/src/Services/RemoteAppOptions.cs @@ -48,6 +48,8 @@ public class RemoteAppOptions // To ensure secure API keys are used, enforce that // keys are parsable as GUIDs - public static bool Validate(RemoteAppOptions options) => - Guid.TryParse(options.ApiKey, out var keyGuid) && keyGuid != Guid.Empty; + internal static bool Validate(RemoteAppOptions options) => + options is not null + && Guid.TryParse(options.ApiKey, out var keyGuid) + && keyGuid != Guid.Empty; } From 9999978792f664409d764a133fe5d9817c2bbd25 Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Wed, 24 Aug 2022 17:39:21 -0400 Subject: [PATCH 4/7] Pull samples' API keys out into configuration This demonstrates better practice and makes it less likely users will copy the keys directly --- samples/ClassLibrary/RemoteServiceUtils.cs | 3 --- samples/MvcApp/Global.asax.cs | 3 ++- samples/MvcApp/Web.config | 3 +++ samples/MvcCoreApp/Program.cs | 4 +++- samples/MvcCoreApp/appsettings.json | 3 ++- samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs | 4 ++-- samples/RemoteAuth/Bearer/RemoteBearer/Web.config | 3 +++ samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs | 2 +- samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json | 3 ++- samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs | 4 ++-- samples/RemoteAuth/Forms/FormsAuth/Web.config | 4 ++++ samples/RemoteAuth/Forms/FormsAuthCore/Program.cs | 2 +- samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json | 3 ++- samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs | 4 ++-- samples/RemoteAuth/OIDC/OIDCAuth/Web.config | 3 +++ samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs | 2 +- samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json | 3 ++- 17 files changed, 35 insertions(+), 18 deletions(-) diff --git a/samples/ClassLibrary/RemoteServiceUtils.cs b/samples/ClassLibrary/RemoteServiceUtils.cs index b66d67abb7..25bcc932db 100644 --- a/samples/ClassLibrary/RemoteServiceUtils.cs +++ b/samples/ClassLibrary/RemoteServiceUtils.cs @@ -5,9 +5,6 @@ namespace ClassLibrary; public class RemoteServiceUtils { - // Do not re-use this ApiKey; every solution should use a unique ApiKey - public static string ApiKey = "54b69938-90dd-4f79-adcd-27fbd6f0e4b7"; - public static void RegisterSessionKeys(IDictionary knownTypes) { knownTypes.Add("test-value", typeof(int)); diff --git a/samples/MvcApp/Global.asax.cs b/samples/MvcApp/Global.asax.cs index 987fc2b9b4..3e7edae6c3 100644 --- a/samples/MvcApp/Global.asax.cs +++ b/samples/MvcApp/Global.asax.cs @@ -1,3 +1,4 @@ +using System.Configuration; using System.Web; using System.Web.Http; using System.Web.Mvc; @@ -20,7 +21,7 @@ protected void Application_Start() .AddProxySupport(options => options.UseForwardedHeaders = true) .AddJsonSessionSerializer(options => ClassLibrary.RemoteServiceUtils.RegisterSessionKeys(options.KnownKeys)) .AddRemoteAppServer(remote => remote - .Configure(options => options.ApiKey = ClassLibrary.RemoteServiceUtils.ApiKey) + .Configure(options => options.ApiKey = ConfigurationManager.AppSettings["RemoteAppApiKey"]) .AddAuthentication() .AddSession()); } diff --git a/samples/MvcApp/Web.config b/samples/MvcApp/Web.config index 4bb800a819..6fb64057af 100644 --- a/samples/MvcApp/Web.config +++ b/samples/MvcApp/Web.config @@ -12,6 +12,9 @@ + + + diff --git a/samples/MvcCoreApp/Program.cs b/samples/MvcCoreApp/Program.cs index 818240af28..8d66c974bf 100644 --- a/samples/MvcCoreApp/Program.cs +++ b/samples/MvcCoreApp/Program.cs @@ -21,7 +21,9 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - options.ApiKey = ClassLibrary.RemoteServiceUtils.ApiKey; + + // Do not re-use this API key. Every solution should have a unique API key. + options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) .AddAuthentication(true) .AddSession()) diff --git a/samples/MvcCoreApp/appsettings.json b/samples/MvcCoreApp/appsettings.json index 77caf24274..5e812bd130 100644 --- a/samples/MvcCoreApp/appsettings.json +++ b/samples/MvcCoreApp/appsettings.json @@ -1,4 +1,5 @@ { + "RemoteAppApiKey": "54b69938-90dd-4f79-adcd-27fbd6f0e4b7", "Logging": { "LogLevel": { "Default": "Information", @@ -26,4 +27,4 @@ } } } -} \ No newline at end of file +} diff --git a/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs b/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs index ccf6167caa..9d1056a945 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs +++ b/samples/RemoteAuth/Bearer/RemoteBearer/Global.asax.cs @@ -1,3 +1,4 @@ +using System.Configuration; using System.Web; using System.Web.Http; using System.Web.Mvc; @@ -17,8 +18,7 @@ protected void Application_Start() { // A real application would not hard code this, but load it // securely from environment or configuration - // Do not re-use this ApiKey; every solution should use a unique ApiKey - options.ApiKey = "40c807bd-6c00-4e5a-9650-ea20c2e6c02d"; + options.ApiKey = ConfigurationManager.AppSettings["RemoteAppApiKey"]; }) .AddAuthentication()); } diff --git a/samples/RemoteAuth/Bearer/RemoteBearer/Web.config b/samples/RemoteAuth/Bearer/RemoteBearer/Web.config index e78971f3f4..b51870c376 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearer/Web.config +++ b/samples/RemoteAuth/Bearer/RemoteBearer/Web.config @@ -5,6 +5,9 @@ --> + + + diff --git a/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs b/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs index c5cbde885b..ae0d376c6b 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs +++ b/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs @@ -14,7 +14,7 @@ // A real application would not hard code this, but load it // securely from environment or configuration // Do not re-use this ApiKey; every solution should use a unique ApiKey - options.ApiKey = "40c807bd-6c00-4e5a-9650-ea20c2e6c02d"; + options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) // This registers the remote app authentication handler. The boolean argument indicates whether remote app auth diff --git a/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json b/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json index 77caf24274..f4c5ff9104 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json +++ b/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json @@ -1,4 +1,5 @@ { + "RemoteAppApiKey": "40c807bd-6c00-4e5a-9650-ea20c2e6c02d", "Logging": { "LogLevel": { "Default": "Information", @@ -26,4 +27,4 @@ } } } -} \ No newline at end of file +} diff --git a/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs b/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs index b5d0ae41ef..15a4ec9662 100644 --- a/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs +++ b/samples/RemoteAuth/Forms/FormsAuth/Global.asax.cs @@ -1,4 +1,5 @@ using System; +using System.Configuration; using System.Web; using System.Web.Optimization; using System.Web.Routing; @@ -19,8 +20,7 @@ void Application_Start(object sender, EventArgs e) .AddRemoteAppServer(remote => remote .Configure(options => { - // Do not re-use this ApiKey; every solution should use a unique ApiKey - options.ApiKey = "8e470586-24e5-4f2a-8245-69bbdbf9f767"; + options.ApiKey = ConfigurationManager.AppSettings["RemoteAppApiKey"]; }) .AddAuthentication()); } diff --git a/samples/RemoteAuth/Forms/FormsAuth/Web.config b/samples/RemoteAuth/Forms/FormsAuth/Web.config index ff73a00c6a..3efc4ade5b 100644 --- a/samples/RemoteAuth/Forms/FormsAuth/Web.config +++ b/samples/RemoteAuth/Forms/FormsAuth/Web.config @@ -1,5 +1,9 @@ + + + + diff --git a/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs b/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs index 586c0dd006..e67119474d 100644 --- a/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs +++ b/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs @@ -13,7 +13,7 @@ options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); // Do not re-use this ApiKey; every solution should use a unique ApiKey - options.ApiKey = "8e470586-24e5-4f2a-8245-69bbdbf9f767"; + options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) .AddAuthentication(true)); diff --git a/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json b/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json index 77caf24274..abb3b8cec2 100644 --- a/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json +++ b/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json @@ -1,4 +1,5 @@ { + "RemoteAppApiKey": "8e470586-24e5-4f2a-8245-69bbdbf9f767", "Logging": { "LogLevel": { "Default": "Information", @@ -26,4 +27,4 @@ } } } -} \ No newline at end of file +} diff --git a/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs b/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs index 3fdddb92c1..8355e0aaea 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs +++ b/samples/RemoteAuth/OIDC/OIDCAuth/Global.asax.cs @@ -1,3 +1,4 @@ +using System.Configuration; using System.Web; using System.Web.Mvc; using System.Web.Optimization; @@ -17,8 +18,7 @@ protected void Application_Start() SystemWebAdapterConfiguration.AddSystemWebAdapters(this) .AddProxySupport(options => options.UseForwardedHeaders = true) .AddRemoteAppServer(remote => remote - // Do not re-use this ApiKey; every solution should use a unique ApiKey - .Configure(options => options.ApiKey = "121257f2-c121-4f51-b30c-d1f617933290") + .Configure(options => options.ApiKey = ConfigurationManager.AppSettings["RemoteAppApiKey"]) .AddAuthentication()); } diff --git a/samples/RemoteAuth/OIDC/OIDCAuth/Web.config b/samples/RemoteAuth/OIDC/OIDCAuth/Web.config index 0d8f37dc5f..010424675e 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuth/Web.config +++ b/samples/RemoteAuth/OIDC/OIDCAuth/Web.config @@ -5,6 +5,9 @@ --> + + + diff --git a/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs b/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs index 25cc8ba6a2..aee45f7b21 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs +++ b/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs @@ -6,7 +6,7 @@ { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); // Do not re-use this ApiKey; every solution should use a unique ApiKey - options.ApiKey = "121257f2-c121-4f51-b30c-d1f617933290"; + options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) .AddAuthentication(true)); diff --git a/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json b/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json index 77caf24274..20013f1d18 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json +++ b/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json @@ -1,4 +1,5 @@ { + "RemoteAppApiKey": "121257f2-c121-4f51-b30c-d1f617933290", "Logging": { "LogLevel": { "Default": "Information", @@ -26,4 +27,4 @@ } } } -} \ No newline at end of file +} From e4a419907ab7aeb2d1a8400bf0cfa555ae05fde6 Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Tue, 30 Aug 2022 14:55:53 -0400 Subject: [PATCH 5/7] Implement API key validation via regex --- .../RemoteAppClientOptions.cs | 28 ++++++++- .../RemoteAppServerOptions.cs | 29 ++++++++- .../RemoteAppClientOptionsTests.cs | 60 +++++++++++++++++++ ...ests.cs => RemoteAppServerOptionsTests.cs} | 4 +- 4 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs rename test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/{Authentication/RemoteAppOptionsTests.cs => RemoteAppServerOptionsTests.cs} (89%) diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs index 492e260fe2..dab9d03949 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs @@ -12,6 +12,32 @@ namespace Microsoft.AspNetCore.SystemWebAdapters; /// public class RemoteAppClientOptions { + // Matches an empty GUID (32 0s) with dashes, parentheses, and braces optional + private const string EmptyGuidRegex = + "[({]?" + // Optional starting brace or parenthesis + "0{8}-?" + // 8 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{12}-?" + // 12 0s followed, optionally, by a - + "[})]?"; // Optional closing brace or parenthesis + + // Matches a GUID with dashes, parentheses, and braces optional + private const string GuidRegex = + "[({]?" + // Optional starting brace or parenthesis + "[0-9a-fA-F]{8}-?" + // 8 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{12}-?" + // 12 hex digits followed, optionally, by a - + "[})]?"; // Optional closing brace or parenthesis + + // Matches a GUID that is not an empty GUID + private const string NonEmptyGuidRegex = + $"^" + // Beginning of string anchor + $"(?!{EmptyGuidRegex})" + // Looking ahead does *not* match empty GUID + $"{GuidRegex}$"; // Matches GUID + /// /// Gets or sets the header used to store the API key /// @@ -21,7 +47,7 @@ public class RemoteAppClientOptions /// /// Gets or sets an API key used to secure the endpoint /// - [Required] + [Required, RegularExpression(NonEmptyGuidRegex, ErrorMessage = "API Key must be 32 hex characters (for example a GUID)")] public string ApiKey { get; set; } = null!; /// diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs index 5f97ec5742..37d20842b8 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.ComponentModel.DataAnnotations; namespace Microsoft.AspNetCore.SystemWebAdapters; @@ -11,6 +10,32 @@ namespace Microsoft.AspNetCore.SystemWebAdapters; /// public class RemoteAppServerOptions { + // Matches an empty GUID (32 0s) with dashes, parentheses, and braces optional + private const string EmptyGuidRegex = + "[({]?" + // Optional starting brace or parenthesis + "0{8}-?" + // 8 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{12}-?" + // 12 0s followed, optionally, by a - + "[})]?"; // Optional closing brace or parenthesis + + // Matches a GUID with dashes, parentheses, and braces optional + private const string GuidRegex = + "[({]?" + // Optional starting brace or parenthesis + "[0-9a-fA-F]{8}-?" + // 8 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{12}-?" + // 12 hex digits followed, optionally, by a - + "[})]?"; // Optional closing brace or parenthesis + + // Matches a GUID that is not an empty GUID + private const string NonEmptyGuidRegex = + $"^" + // Beginning of string anchor + $"(?!{EmptyGuidRegex})" + // Looking ahead does *not* match empty GUID + $"{GuidRegex}$"; // Matches GUID + /// /// Gets or sets the header used to store the API key /// @@ -20,6 +45,6 @@ public class RemoteAppServerOptions /// /// Gets or sets an API key used to secure the endpoint /// - [Required] + [Required, RegularExpression(NonEmptyGuidRegex, ErrorMessage = "API Key must be 32 hex characters (for example a GUID)")] public string ApiKey { get; set; } = null!; } diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs new file mode 100644 index 0000000000..ec7c8a1a41 --- /dev/null +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/RemoteAppClientOptionsTests.cs @@ -0,0 +1,60 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Xunit; +using System.Web; +using System.Security.Policy; +using System; + +namespace Microsoft.AspNetCore.SystemWebAdapters.Tests; + +public class RemoteAppClientOptionsTests +{ + [InlineData("HeaderName", "36705d36-eba0-44f9-a2e0-c57e6f521274", "http://test", true)] + [InlineData("HeaderName", "{36705d36eba044f9a2e0c57e6f521274}", "http://test", true)] + [InlineData("HeaderName", "00000000-0000-0000-0000-000000000000", "http://test", false)] + [InlineData(null, "36705d36-eba0-44f9-a2e0-c57e6f521274", "http://test", false)] + [InlineData("", "36705d36-eba0-44f9-a2e0-c57e6f521274", "http://test", false)] + [InlineData("HeaderName", "MyKey", "http://test", false)] + [InlineData(null, "MyKey", "http://test", false)] + [InlineData("", "MyKey", "http://test", false)] + [InlineData("HeaderName", null, "http://test", false)] + [InlineData("HeaderName", "", "http://test", false)] + [InlineData("HeaderName", "36705d36-eba0-44f9-a2e0-c57e6f521274", null, false)] + [Theory] + public void VerifyIsCalled(string apiKeyHeader, string apiKey, string remoteAppUrl, bool shouldSucceed) + { + // Arrange + var services = new ServiceCollection(); + var builder = new TestBuilder { Services = services }; + + builder.AddRemoteAppClient(remote => remote + .Configure(options => + { + options.ApiKey = apiKey; + options.ApiKeyHeader = apiKeyHeader; + options.RemoteAppUrl = (remoteAppUrl is null ? null : new Uri(remoteAppUrl, UriKind.Absolute))!; + })); + + using var serviceProvider = services.BuildServiceProvider(); + + var options = serviceProvider.GetRequiredService>(); + + // Act/Assert + if (shouldSucceed) + { + _ = options.Value; + } + else + { + Assert.Throws(() => options.Value); + } + } + + private class TestBuilder : ISystemWebAdapterBuilder + { + public IServiceCollection Services { get; set; } = null!; + } +} diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/RemoteAppServerOptionsTests.cs similarity index 89% rename from test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs rename to test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/RemoteAppServerOptionsTests.cs index fc392a386d..a6e54b3b12 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppOptionsTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/RemoteAppServerOptionsTests.cs @@ -8,9 +8,11 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.Tests; -public class RemoteAppOptionsTests +public class RemoteAppServerOptionsTests { [InlineData("HeaderName", "36705d36-eba0-44f9-a2e0-c57e6f521274", true)] + [InlineData("HeaderName", "{36705d36eba044f9a2e0c57e6f521274}", true)] + [InlineData("HeaderName", "00000000-0000-0000-0000-000000000000", false)] [InlineData(null, "36705d36-eba0-44f9-a2e0-c57e6f521274", false)] [InlineData("", "36705d36-eba0-44f9-a2e0-c57e6f521274", false)] [InlineData("HeaderName", "MyKey", false)] From 5024aa40e706d73b38f8bc9b4a2e583e18c4ca60 Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Tue, 30 Aug 2022 15:22:01 -0400 Subject: [PATCH 6/7] Move warnings about re-using API key into config files --- samples/MvcCoreApp/Program.cs | 2 -- samples/MvcCoreApp/appsettings.json | 1 + samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs | 4 ---- samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json | 1 + samples/RemoteAuth/Forms/FormsAuthCore/Program.cs | 2 -- samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json | 1 + samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs | 1 - samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json | 1 + 8 files changed, 4 insertions(+), 9 deletions(-) diff --git a/samples/MvcCoreApp/Program.cs b/samples/MvcCoreApp/Program.cs index 8d66c974bf..5b02e07f5e 100644 --- a/samples/MvcCoreApp/Program.cs +++ b/samples/MvcCoreApp/Program.cs @@ -21,8 +21,6 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - - // Do not re-use this API key. Every solution should have a unique API key. options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) .AddAuthentication(true) diff --git a/samples/MvcCoreApp/appsettings.json b/samples/MvcCoreApp/appsettings.json index 5e812bd130..196b7185b8 100644 --- a/samples/MvcCoreApp/appsettings.json +++ b/samples/MvcCoreApp/appsettings.json @@ -1,4 +1,5 @@ { + // Do not re-use this API key. Every solution should have a unique API key. "RemoteAppApiKey": "54b69938-90dd-4f79-adcd-27fbd6f0e4b7", "Logging": { "LogLevel": { diff --git a/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs b/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs index ae0d376c6b..54fa37d44e 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs +++ b/samples/RemoteAuth/Bearer/RemoteBearerCore/Program.cs @@ -10,10 +10,6 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - - // A real application would not hard code this, but load it - // securely from environment or configuration - // Do not re-use this ApiKey; every solution should use a unique ApiKey options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) diff --git a/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json b/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json index f4c5ff9104..346bdc0c32 100644 --- a/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json +++ b/samples/RemoteAuth/Bearer/RemoteBearerCore/appsettings.json @@ -1,4 +1,5 @@ { + // Do not re-use this ApiKey; every solution should use a unique ApiKey "RemoteAppApiKey": "40c807bd-6c00-4e5a-9650-ea20c2e6c02d", "Logging": { "LogLevel": { diff --git a/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs b/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs index e67119474d..034790dc19 100644 --- a/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs +++ b/samples/RemoteAuth/Forms/FormsAuthCore/Program.cs @@ -11,8 +11,6 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - - // Do not re-use this ApiKey; every solution should use a unique ApiKey options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) .AddAuthentication(true)); diff --git a/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json b/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json index abb3b8cec2..a22fa64e23 100644 --- a/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json +++ b/samples/RemoteAuth/Forms/FormsAuthCore/appsettings.json @@ -1,4 +1,5 @@ { + // Do not re-use this ApiKey; every solution should use a unique ApiKey "RemoteAppApiKey": "8e470586-24e5-4f2a-8245-69bbdbf9f767", "Logging": { "LogLevel": { diff --git a/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs b/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs index aee45f7b21..3d1df77ad4 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs +++ b/samples/RemoteAuth/OIDC/OIDCAuthCore/Program.cs @@ -5,7 +5,6 @@ .Configure(options => { options.RemoteAppUrl = new(builder.Configuration["ReverseProxy:Clusters:fallbackCluster:Destinations:fallbackApp:Address"]); - // Do not re-use this ApiKey; every solution should use a unique ApiKey options.ApiKey = builder.Configuration["RemoteAppApiKey"]; }) .AddAuthentication(true)); diff --git a/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json b/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json index 20013f1d18..bf814fab72 100644 --- a/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json +++ b/samples/RemoteAuth/OIDC/OIDCAuthCore/appsettings.json @@ -1,4 +1,5 @@ { + // Do not re-use this ApiKey; every solution should use a unique ApiKey "RemoteAppApiKey": "121257f2-c121-4f51-b30c-d1f617933290", "Logging": { "LogLevel": { From 7a461e82c82448195a24bdaa6867272aea89b7eb Mon Sep 17 00:00:00 2001 From: Mike Rousos Date: Tue, 30 Aug 2022 15:35:26 -0400 Subject: [PATCH 7/7] Consolidate API key validation in a shared ApiKeyAttribute --- .../RemoteAppClientOptions.cs | 28 +------------ .../RemoteAppServerOptions.cs | 28 +------------ src/Services/ApiKeyAttribute.cs | 40 +++++++++++++++++++ 3 files changed, 42 insertions(+), 54 deletions(-) create mode 100644 src/Services/ApiKeyAttribute.cs diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs index dab9d03949..a08dac34dd 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs @@ -12,32 +12,6 @@ namespace Microsoft.AspNetCore.SystemWebAdapters; /// public class RemoteAppClientOptions { - // Matches an empty GUID (32 0s) with dashes, parentheses, and braces optional - private const string EmptyGuidRegex = - "[({]?" + // Optional starting brace or parenthesis - "0{8}-?" + // 8 0s followed, optionally, by a - - "0{4}-?" + // 4 0s followed, optionally, by a - - "0{4}-?" + // 4 0s followed, optionally, by a - - "0{4}-?" + // 4 0s followed, optionally, by a - - "0{12}-?" + // 12 0s followed, optionally, by a - - "[})]?"; // Optional closing brace or parenthesis - - // Matches a GUID with dashes, parentheses, and braces optional - private const string GuidRegex = - "[({]?" + // Optional starting brace or parenthesis - "[0-9a-fA-F]{8}-?" + // 8 hex digits followed, optionally, by a - - "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - - "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - - "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - - "[0-9a-fA-F]{12}-?" + // 12 hex digits followed, optionally, by a - - "[})]?"; // Optional closing brace or parenthesis - - // Matches a GUID that is not an empty GUID - private const string NonEmptyGuidRegex = - $"^" + // Beginning of string anchor - $"(?!{EmptyGuidRegex})" + // Looking ahead does *not* match empty GUID - $"{GuidRegex}$"; // Matches GUID - /// /// Gets or sets the header used to store the API key /// @@ -47,7 +21,7 @@ public class RemoteAppClientOptions /// /// Gets or sets an API key used to secure the endpoint /// - [Required, RegularExpression(NonEmptyGuidRegex, ErrorMessage = "API Key must be 32 hex characters (for example a GUID)")] + [Required, ApiKey] public string ApiKey { get; set; } = null!; /// diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs index 37d20842b8..9e21c7a9c6 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerOptions.cs @@ -10,32 +10,6 @@ namespace Microsoft.AspNetCore.SystemWebAdapters; /// public class RemoteAppServerOptions { - // Matches an empty GUID (32 0s) with dashes, parentheses, and braces optional - private const string EmptyGuidRegex = - "[({]?" + // Optional starting brace or parenthesis - "0{8}-?" + // 8 0s followed, optionally, by a - - "0{4}-?" + // 4 0s followed, optionally, by a - - "0{4}-?" + // 4 0s followed, optionally, by a - - "0{4}-?" + // 4 0s followed, optionally, by a - - "0{12}-?" + // 12 0s followed, optionally, by a - - "[})]?"; // Optional closing brace or parenthesis - - // Matches a GUID with dashes, parentheses, and braces optional - private const string GuidRegex = - "[({]?" + // Optional starting brace or parenthesis - "[0-9a-fA-F]{8}-?" + // 8 hex digits followed, optionally, by a - - "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - - "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - - "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - - "[0-9a-fA-F]{12}-?" + // 12 hex digits followed, optionally, by a - - "[})]?"; // Optional closing brace or parenthesis - - // Matches a GUID that is not an empty GUID - private const string NonEmptyGuidRegex = - $"^" + // Beginning of string anchor - $"(?!{EmptyGuidRegex})" + // Looking ahead does *not* match empty GUID - $"{GuidRegex}$"; // Matches GUID - /// /// Gets or sets the header used to store the API key /// @@ -45,6 +19,6 @@ public class RemoteAppServerOptions /// /// Gets or sets an API key used to secure the endpoint /// - [Required, RegularExpression(NonEmptyGuidRegex, ErrorMessage = "API Key must be 32 hex characters (for example a GUID)")] + [Required, ApiKey] public string ApiKey { get; set; } = null!; } diff --git a/src/Services/ApiKeyAttribute.cs b/src/Services/ApiKeyAttribute.cs new file mode 100644 index 0000000000..305f09b8df --- /dev/null +++ b/src/Services/ApiKeyAttribute.cs @@ -0,0 +1,40 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel.DataAnnotations; + +internal class ApiKeyAttribute: RegularExpressionAttribute +{ + private const string DefaultErrorMessage = "API Key must be 32 hex characters (for example a GUID)"; + + // Matches an empty GUID (32 0s) with dashes, parentheses, and braces optional + private const string EmptyGuidRegex = + "[({]?" + // Optional starting brace or parenthesis + "0{8}-?" + // 8 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{4}-?" + // 4 0s followed, optionally, by a - + "0{12}-?" + // 12 0s followed, optionally, by a - + "[})]?"; // Optional closing brace or parenthesis + + // Matches a GUID with dashes, parentheses, and braces optional + private const string GuidRegex = + "[({]?" + // Optional starting brace or parenthesis + "[0-9a-fA-F]{8}-?" + // 8 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{4}-?" + // 4 hex digits followed, optionally, by a - + "[0-9a-fA-F]{12}-?" + // 12 hex digits followed, optionally, by a - + "[})]?"; // Optional closing brace or parenthesis + + // Matches a GUID that is not an empty GUID + private const string NonEmptyGuidRegex = + $"^" + // Beginning of string anchor + $"(?!{EmptyGuidRegex})" + // Looking ahead does *not* match empty GUID + $"{GuidRegex}$"; // Matches GUID + + public ApiKeyAttribute() : base(NonEmptyGuidRegex) + { + ErrorMessage = DefaultErrorMessage; + } +}