Skip to content

Commit bab1899

Browse files
twsouthwicktwsouthwick
authored
Use a common remote module to ensure standard checks (#177)
This change creates a module base class that enables exposing endpoints from the service in a standard way. This pattern is somewhat testable as well as ensures things like API key checks are done as expected.
1 parent fa01406 commit bab1899

File tree

9 files changed

+126
-140
lines changed

9 files changed

+126
-140
lines changed

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/Authentication/RemoteAppAuthenticationExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static ISystemWebAdapterRemoteServerAppBuilder AddAuthentication(this ISy
2727

2828
builder.Services.AddScoped<IHttpModule, RemoteAppAuthenticationModule>();
2929
var options = builder.Services.AddOptions<RemoteAppAuthenticationServerOptions>()
30-
.Validate(options => !string.IsNullOrEmpty(options.AuthenticationEndpointPath), "AuthenticationEndpointPath must be set");
30+
.ValidateDataAnnotations();
3131

3232
if (configure is not null)
3333
{

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/Authentication/RemoteAppAuthenticationModule.cs‎

Lines changed: 11 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1,32 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
5-
using System.Security.Policy;
64
using System.Web;
75
using Microsoft.Extensions.Options;
86

97
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication;
108

11-
internal sealed class RemoteAppAuthenticationModule : IHttpModule
9+
internal sealed class RemoteAppAuthenticationModule : RemoteModule
1210
{
13-
private readonly RemoteAppServerOptions _remoteAppOptions;
14-
private readonly RemoteAppAuthenticationServerOptions _authOptions;
15-
private readonly RemoteAppAuthenticationHttpHandler _remoteAppAuthHandler;
16-
1711
public RemoteAppAuthenticationModule(IOptions<RemoteAppServerOptions> remoteAppOptions, IOptions<RemoteAppAuthenticationServerOptions> authOptions)
12+
: base(remoteAppOptions)
1813
{
1914
if (authOptions is null)
2015
{
2116
throw new ArgumentNullException(nameof(authOptions));
2217
}
2318

24-
if (remoteAppOptions is null)
25-
{
26-
throw new ArgumentNullException(nameof(remoteAppOptions));
27-
}
28-
29-
if (string.IsNullOrEmpty(authOptions.Value.AuthenticationEndpointPath))
30-
{
31-
throw new ArgumentOutOfRangeException(nameof(authOptions.Value.AuthenticationEndpointPath), "Options must specify remote authentication path.");
32-
}
19+
Path = authOptions.Value.AuthenticationEndpointPath;
3320

34-
if (string.IsNullOrEmpty(remoteAppOptions.Value.ApiKey))
35-
{
36-
throw new ArgumentOutOfRangeException(nameof(remoteAppOptions.Value.ApiKey), "Options must specify API key.");
37-
}
38-
39-
if (string.IsNullOrEmpty(remoteAppOptions.Value.ApiKeyHeader))
40-
{
41-
throw new ArgumentOutOfRangeException(nameof(remoteAppOptions.Value.ApiKeyHeader), "Options must specify API key header name.");
42-
}
21+
var handler = new RemoteAppAuthenticationHttpHandler();
4322

44-
_authOptions = authOptions.Value;
45-
_remoteAppOptions = remoteAppOptions.Value;
46-
_remoteAppAuthHandler = new RemoteAppAuthenticationHttpHandler();
23+
MapGet(context => handler);
4724
}
4825

49-
public void Init(HttpApplication context)
50-
{
51-
context.PostMapRequestHandler += (s, _) =>
52-
{
53-
var context = ((HttpApplication)s).Context;
54-
if (string.Equals(context.Request.Path, _authOptions.AuthenticationEndpointPath, StringComparison.OrdinalIgnoreCase)
55-
&& context.Request.HttpMethod.Equals("GET", StringComparison.OrdinalIgnoreCase))
56-
{
57-
MapRemoteAuthenticationHandler(new HttpContextWrapper(context));
58-
59-
if (context.Handler is null)
60-
{
61-
context.ApplicationInstance.CompleteRequest();
62-
}
63-
}
64-
};
65-
}
26+
protected override string Path { get; }
6627

67-
public void MapRemoteAuthenticationHandler(HttpContextBase context)
28+
protected override bool Authenticate(HttpContextBase context)
6829
{
69-
var apiKey = context.Request.Headers.Get(_remoteAppOptions.ApiKeyHeader);
7030
var migrationAuthenticateHeader = context.Request.Headers.Get(AuthenticationConstants.MigrationAuthenticateRequestHeaderName);
7131

7232
if (migrationAuthenticateHeader is null)
@@ -94,25 +54,17 @@ public void MapRemoteAuthenticationHandler(HttpContextBase context)
9454
context.Response.StatusCode = 400;
9555
}
9656

97-
// Clear any existing handler as this request is now completely handled
98-
context.Handler = null;
57+
return false;
9958
}
100-
else if (apiKey is null || !string.Equals(_remoteAppOptions.ApiKey, apiKey, StringComparison.Ordinal))
59+
else if (!HasValidApiKey(context))
10160
{
10261
// Requests to the authentication endpoint must include a valid API key.
10362
// Requests without an API key or with an invalid API key are considered malformed.
10463
context.Response.StatusCode = 400;
10564

106-
// Clear any existing handler as this request is now completely handled
107-
context.Handler = null;
108-
}
109-
else
110-
{
111-
context.Handler = _remoteAppAuthHandler;
65+
return false;
11266
}
113-
}
11467

115-
public void Dispose()
116-
{
68+
return true;
11769
}
11870
}

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/Authentication/RemoteAppAuthenticationServerOptions.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.ComponentModel.DataAnnotations;
5+
46
namespace Microsoft.AspNetCore.SystemWebAdapters.Authentication;
57

68
public class RemoteAppAuthenticationServerOptions
79
{
10+
[Required]
811
public string AuthenticationEndpointPath { get; set; } = AuthenticationConstants.DefaultEndpoint;
912
}

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
</ItemGroup>
3030

3131
<ItemGroup>
32+
<PackageReference Include="Microsoft.Extensions.Options.DataAnnotations" Version="6.0.0" />
3233
<PackageReference Include="System.Text.Json" Version="6.0.0" />
3334
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0" />
3435
<PackageReference Include="Microsoft.Extensions.Options" Version="6.0.0" />

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerExtensions.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ public static ISystemWebAdapterBuilder AddRemoteAppServer(this ISystemWebAdapter
2424
throw new ArgumentNullException(nameof(configure));
2525
}
2626

27-
var options = builder.Services.AddOptions<RemoteAppServerOptions>()
28-
.Validate(options => !string.IsNullOrEmpty(options.ApiKey), "ApiKey must be set")
29-
.Validate(options => !string.IsNullOrEmpty(options.ApiKeyHeader), "ApiKeyHeader must be set");
27+
builder.Services.AddOptions<RemoteAppServerOptions>()
28+
.ValidateDataAnnotations();
3029

3130
configure(new Builder(builder.Services));
3231

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteModule.cs‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Web;
5+
using Microsoft.Extensions.Options;
6+
7+
namespace Microsoft.AspNetCore.SystemWebAdapters;
8+
9+
internal abstract class RemoteModule : IHttpModule
10+
{
11+
private readonly IOptions<RemoteAppServerOptions> _options;
12+
13+
private Func<HttpContextBase, IHttpHandler?>? _get;
14+
private Func<HttpContextBase, IHttpHandler?>? _put;
15+
16+
protected RemoteModule(IOptions<RemoteAppServerOptions> options)
17+
{
18+
_options = options;
19+
}
20+
21+
protected abstract string Path { get; }
22+
23+
protected void MapGet(Func<HttpContextBase, IHttpHandler?> handler)
24+
=> _get = handler;
25+
26+
protected void MapPut(Func<HttpContextBase, IHttpHandler?> handler)
27+
=> _put = handler;
28+
29+
protected bool HasValidApiKey(HttpContextBase context)
30+
{
31+
var apiKey = context.Request.Headers.Get(_options.Value.ApiKeyHeader);
32+
33+
return string.Equals(_options.Value.ApiKey, apiKey, StringComparison.Ordinal);
34+
}
35+
36+
protected virtual bool Authenticate(HttpContextBase context)
37+
{
38+
if (HasValidApiKey(context))
39+
{
40+
return true;
41+
}
42+
43+
context.Response.StatusCode = 401;
44+
return false;
45+
}
46+
47+
void IHttpModule.Dispose()
48+
{
49+
}
50+
51+
void IHttpModule.Init(HttpApplication context)
52+
{
53+
context.PostMapRequestHandler += (s, _) =>
54+
{
55+
var context = ((HttpApplication)s).Context;
56+
57+
if (!string.Equals(context.Request.Path, Path, StringComparison.Ordinal))
58+
{
59+
return;
60+
}
61+
62+
context.Handler = null;
63+
64+
HandleRequest(new HttpContextWrapper(context));
65+
66+
if (context.Handler is null)
67+
{
68+
context.ApplicationInstance.CompleteRequest();
69+
}
70+
};
71+
}
72+
73+
public void HandleRequest(HttpContextBase context)
74+
{
75+
if (!Authenticate(context))
76+
{
77+
}
78+
else if (string.Equals("GET", context.Request.HttpMethod, StringComparison.OrdinalIgnoreCase) && _get is { } get)
79+
{
80+
context.Handler = get(context);
81+
}
82+
else if (string.Equals("PUT", context.Request.HttpMethod, StringComparison.OrdinalIgnoreCase) && _put is { } put)
83+
{
84+
context.Handler = put(context);
85+
}
86+
else
87+
{
88+
context.Response.StatusCode = 405;
89+
}
90+
}
91+
}

‎src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/SessionState/RemoteSession/RemoteSessionModule.cs‎

Lines changed: 14 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,82 +1,36 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
54
using System.Web;
65
using Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization;
76
using Microsoft.Extensions.Options;
87

98
namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.RemoteSession;
109

11-
internal sealed class RemoteSessionModule : IHttpModule
10+
internal sealed class RemoteSessionModule : RemoteModule
1211
{
13-
private readonly RemoteAppServerOptions _remoteAppOptions;
14-
private readonly RemoteAppSessionStateServerOptions _sessionOptions;
15-
private readonly ReadOnlySessionHandler _readonlyHandler;
16-
private readonly GetWriteableSessionHandler _writeableHandler;
17-
private readonly StoreSessionStateHandler _saveHandler;
18-
1912
public RemoteSessionModule(IOptions<RemoteAppSessionStateServerOptions> sessionOptions, IOptions<RemoteAppServerOptions> remoteAppOptions, ILockedSessionCache cache, ISessionSerializer serializer)
13+
: base(remoteAppOptions)
2014
{
21-
_sessionOptions = sessionOptions?.Value ?? throw new ArgumentNullException(nameof(sessionOptions));
22-
_remoteAppOptions = remoteAppOptions?.Value ?? throw new ArgumentNullException(nameof(remoteAppOptions));
23-
24-
if (string.IsNullOrEmpty(_remoteAppOptions.ApiKey))
15+
if (sessionOptions is null)
2516
{
26-
throw new ArgumentOutOfRangeException(nameof(_remoteAppOptions.ApiKey), "API key must not be empty.");
17+
throw new ArgumentNullException(nameof(sessionOptions));
2718
}
2819

29-
_readonlyHandler = new ReadOnlySessionHandler(serializer);
30-
_writeableHandler = new GetWriteableSessionHandler(serializer, cache);
31-
_saveHandler = new StoreSessionStateHandler(cache, _sessionOptions.CookieName);
32-
}
33-
34-
public void Init(HttpApplication context)
35-
{
36-
context.PostMapRequestHandler += (s, _) =>
37-
{
38-
var context = ((HttpApplication)s).Context;
20+
var options = sessionOptions.Value;
3921

40-
// Filter out requests that are not the correct path so we don't create a wrapper for every request
41-
if (!string.Equals(context.Request.Path, _sessionOptions.SessionEndpointPath, StringComparison.OrdinalIgnoreCase))
42-
{
43-
return;
44-
}
22+
Path = options.SessionEndpointPath;
4523

46-
MapRemoteSessionHandler(new HttpContextWrapper(context));
24+
var readonlyHandler = new ReadOnlySessionHandler(serializer);
25+
var writeableHandler = new GetWriteableSessionHandler(serializer, cache);
26+
var saveHandler = new StoreSessionStateHandler(cache, options.CookieName);
4727

48-
if (context.Handler is null)
49-
{
50-
context.ApplicationInstance.CompleteRequest();
51-
}
52-
};
53-
}
28+
MapGet(context => GetIsReadonly(context.Request) ? readonlyHandler : writeableHandler);
29+
MapPut(context => saveHandler);
5430

55-
public void MapRemoteSessionHandler(HttpContextBase context)
56-
{
57-
if (!string.Equals(_remoteAppOptions.ApiKey, context.Request.Headers.Get(_remoteAppOptions.ApiKeyHeader), StringComparison.Ordinal))
58-
{
59-
context.Response.StatusCode = 401;
60-
}
61-
else if (context.Request.HttpMethod.Equals("GET", StringComparison.OrdinalIgnoreCase))
62-
{
63-
context.Handler = GetIsReadonly(context.Request) ? _readonlyHandler : _writeableHandler;
64-
}
65-
else if (context.Request.HttpMethod.Equals("PUT", StringComparison.OrdinalIgnoreCase))
66-
{
67-
context.Handler = _saveHandler;
68-
}
69-
else
70-
{
71-
// HTTP methods other than GET (read) or PUT (write) are not accepted
72-
context.Response.StatusCode = 405; // Method not allowed
73-
}
74-
}
75-
76-
public void Dispose()
77-
{
31+
static bool GetIsReadonly(HttpRequestBase request)
32+
=> bool.TryParse(request.Headers.Get(SessionConstants.ReadOnlyHeaderName), out var result) && result;
7833
}
7934

80-
private static bool GetIsReadonly(HttpRequestBase request)
81-
=> bool.TryParse(request.Headers.Get(SessionConstants.ReadOnlyHeaderName), out var result) && result;
35+
protected override string Path { get; }
8236
}

‎test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Authentication/RemoteAppAuthenticationModuleTests.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public void VerifyAuthenticationRequestHandling(string apiKey, string authMigrat
5858
var request = new Mock<HttpRequestBase>();
5959
request.Setup(r => r.Headers).Returns(headers);
6060
request.Setup(r => r.QueryString).Returns(queryStrings);
61+
request.Setup(r => r.HttpMethod).Returns("GET");
6162
request.Setup(r => r.Url).Returns(new Uri("http://localhost:8080"));
6263

6364
var response = new Mock<HttpResponseBase>();
@@ -70,7 +71,7 @@ public void VerifyAuthenticationRequestHandling(string apiKey, string authMigrat
7071
context.SetupProperty(c => c.Handler);
7172

7273
// Act
73-
module.MapRemoteAuthenticationHandler(context.Object);
74+
module.HandleRequest(context.Object);
7475

7576
// Assert
7677
Assert.Equal(expectedStatusCode, response.Object.StatusCode);

‎test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/RemoteSession/RemoteSessionModuleTests.cs‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,6 @@ public RemoteSessionModuleTests()
2424
_fixture = new Fixture();
2525
}
2626

27-
[InlineData(null)]
28-
[InlineData("")]
29-
[Theory]
30-
public void VeriyApiKeyIsNotNullOrEmpty(string apiKey)
31-
{
32-
// Arrange
33-
var sessionOptions = Options.Create(new RemoteAppSessionStateServerOptions());
34-
var remoteAppOptions = Options.Create(new RemoteAppServerOptions { ApiKey = apiKey });
35-
var sessions = new Mock<ILockedSessionCache>();
36-
var serializer = new Mock<ISessionSerializer>();
37-
38-
// Act/Assert
39-
Assert.Throws<ArgumentOutOfRangeException>(() => new RemoteSessionModule(sessionOptions, remoteAppOptions, sessions.Object, serializer.Object));
40-
}
41-
4227
[InlineData("GET", "true", 401, ApiKey1, ApiKey2, null)]
4328
[InlineData("GET", "true", 0, ApiKey1, ApiKey1, typeof(ReadOnlySessionHandler))]
4429
[InlineData("GET", "false", 0, ApiKey1, ApiKey1, typeof(GetWriteableSessionHandler))]
@@ -82,7 +67,7 @@ public void VerifyCorrectHandler(string method, string readOnlyHeaderValue, int
8267
context.SetupProperty(c => c.Handler);
8368

8469
// Act
85-
module.MapRemoteSessionHandler(context.Object);
70+
module.HandleRequest(context.Object);
8671

8772
// Assert
8873
Assert.Equal(statusCode, response.Object.StatusCode);

0 commit comments

Comments
 (0)