Skip to content

Commit f43aa7c

Browse files
Jonas Hendrickxjrmccannon
andauthored
Fix event logging context bug on non-existing endpoints (#354)
Co-authored-by: jrmccannon <[email protected]>
1 parent 6cf41c4 commit f43aa7c

File tree

9 files changed

+285
-12
lines changed

9 files changed

+285
-12
lines changed

src/AdminConsole/Middleware/EventLogStorageCommitMiddleware.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,21 @@
22

33
namespace Passwordless.AdminConsole.Middleware;
44

5-
public class EventLogStorageCommitMiddleware
5+
public partial class EventLogStorageCommitMiddleware
66
{
77
private readonly RequestDelegate _next;
88

99
public EventLogStorageCommitMiddleware(RequestDelegate next) => _next = next;
1010

1111
public async Task InvokeAsync(HttpContext context, IEventLogger eventLogger)
1212
{
13+
// If the request is not in our routing tables, skip this middleware.
14+
if (context.GetEndpoint() == null)
15+
{
16+
await _next(context);
17+
return;
18+
}
19+
1320
try
1421
{
1522
await _next(context);

src/Api/Middleware/EventLogContextMiddleware.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ public class EventLogContextMiddleware
1313

1414
public async Task InvokeAsync(HttpContext context, IEventLogContext eventLogContext, IFeatureContextProvider featureContextProvider)
1515
{
16+
// If the request is not in our routing tables, skip this middleware.
17+
if (context.GetEndpoint() == null)
18+
{
19+
await _next(context);
20+
return;
21+
}
22+
1623
var tenantId = context.Request.GetTenantName();
1724
var requestKey = context.Request.GetPublicApiKey() ?? context.Request.GetApiSecret();
1825
var isAuthenticated = context.User.Identity?.IsAuthenticated ?? false;

src/Api/Middleware/EventLogStorageCommitMiddleware.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ public EventLogStorageCommitMiddleware(RequestDelegate next, ILogger<EventLogSto
2020
/// <param name="provider">Used to get the `IEventLogger` instance based on the current state of the request.</param>
2121
public async Task InvokeAsync(HttpContext context, IServiceProvider provider)
2222
{
23+
// If the request is not in our routing tables, skip this middleware.
24+
if (context.GetEndpoint() == null)
25+
{
26+
await _next(context);
27+
return;
28+
}
29+
2330
try
2431
{
2532
await _next(context);

src/Service/EventLog/Loggers/UnauthenticatedEventLoggerEfWriteStorage.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ public UnauthenticatedEventLoggerEfWriteStorage(
1919

2020
public async override Task FlushAsync()
2121
{
22-
if ((await _featureContextProvider.UseContext()).EventLoggingIsEnabled)
22+
var featuresContext = await _featureContextProvider.UseContext();
23+
24+
if (featuresContext.IsInFeaturesContext && featuresContext.EventLoggingIsEnabled)
2325
{
2426
await base.FlushAsync();
2527
}

src/Service/Features/FeatureContextProvider.cs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,27 @@ public FeatureContextProvider(
2222
if (httpContext == null) return new NullFeaturesContext();
2323

2424
string appId = null;
25-
if (httpContext.Request.RouteValues.ContainsKey("appId"))
26-
{
27-
appId = httpContext.Request.RouteValues["appId"].ToString();
28-
}
29-
if (httpContext.Request.Headers.ContainsKey("ApiKey"))
25+
26+
try
3027
{
31-
appId = ApiKeyUtils.GetAppId(httpContext.Request.Headers["ApiKey"].ToString());
28+
if (httpContext.Request.Headers.ContainsKey("ApiKey"))
29+
{
30+
appId = ApiKeyUtils.GetAppId(httpContext.Request.Headers["ApiKey"].ToString());
31+
}
32+
else if (httpContext.Request.Headers.ContainsKey("ApiSecret"))
33+
{
34+
appId = ApiKeyUtils.GetAppId(httpContext.Request.Headers["ApiSecret"].ToString());
35+
}
36+
else if (httpContext.Request.RouteValues.ContainsKey("appId"))
37+
{
38+
appId = httpContext.Request.RouteValues["appId"].ToString();
39+
}
3240
}
33-
if (httpContext.Request.Headers.ContainsKey("ApiSecret"))
41+
catch (ArgumentException)
3442
{
35-
appId = ApiKeyUtils.GetAppId(httpContext.Request.Headers["ApiSecret"].ToString());
43+
// In case of a bad api key format, we may not be able to obtain the tenant context.
44+
// We'll just return a null context as we won't have a need to log the bad api key.
45+
return new NullFeaturesContext();
3646
}
3747

3848
if (string.IsNullOrWhiteSpace(appId)) return new NullFeaturesContext();

tests/Api.IntegrationTests/Endpoints/Events/EventsTests.cs

Lines changed: 105 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
using Passwordless.Api.IntegrationTests.Helpers.App;
77
using Passwordless.Common.Constants;
88
using Passwordless.Common.EventLog.Enums;
9+
using Passwordless.Common.Extensions;
910
using Passwordless.Common.Models.Apps;
11+
using Passwordless.Service.Models;
1012
using Xunit;
1113
using Xunit.Abstractions;
1214

@@ -176,8 +178,8 @@ public async Task I_can_view_the_event_for_disabling_the_generate_sign_in_token_
176178
using var appCreationResponse = await _client.CreateApplicationAsync(applicationName);
177179
var accountKeysCreation = await appCreationResponse.Content.ReadFromJsonAsync<CreateAppResultDto>();
178180
_client.AddSecretKey(accountKeysCreation!.ApiSecret1);
179-
_ = await _client.EnableEventLogging(applicationName);
180-
using var enableResponse = await _client.PostAsJsonAsync($"admin/apps/{applicationName}/sign-in-generate-token-endpoint/disable",
181+
await _client.EnableEventLogging(applicationName);
182+
await _client.PostAsJsonAsync($"admin/apps/{applicationName}/sign-in-generate-token-endpoint/disable",
181183
new AppsEndpoints.DisableGenerateSignInTokenEndpointRequest(user));
182184

183185
// Act
@@ -193,6 +195,107 @@ public async Task I_can_view_the_event_for_disabling_the_generate_sign_in_token_
193195
enabledEvent!.PerformedBy.Should().Be(user);
194196
}
195197

198+
[Fact]
199+
public async Task I_can_view_the_event_for_using_a_disabled_api_secret()
200+
{
201+
// Arrange
202+
var applicationName = CreateAppHelpers.GetApplicationName();
203+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
204+
var accountKeysCreation = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
205+
_client.AddSecretKey(accountKeysCreation!.ApiSecret1);
206+
await _client.EnableEventLogging(applicationName);
207+
using var getApiKeysResponse = await _client.GetAsync($"/admin/apps/{applicationName}/api-keys");
208+
var apiKeys = await getApiKeysResponse.Content.ReadFromJsonAsync<IReadOnlyCollection<ApiKeyResponse>>();
209+
var keyToLock = apiKeys!.First(x => x.ApiKey.EndsWith(accountKeysCreation.ApiSecret1.GetLast(4)));
210+
_ = await _client.PostAsync($"/admin/apps/{applicationName}/api-keys/{keyToLock.Id}/lock", null);
211+
_ = await _client.GetAsync("credentials/list");
212+
_ = await _client.PostAsync($"/admin/apps/{applicationName}/api-keys/{keyToLock.Id}/unlock", null);
213+
214+
// Act
215+
using var getApplicationEventsResponse = await _client.GetAsync("events?pageNumber=1");
216+
// Assert
217+
getApplicationEventsResponse.StatusCode.Should().Be(HttpStatusCode.OK);
218+
var applicationEvents = await getApplicationEventsResponse.Content.ReadFromJsonAsync<EventLog.GetEventLogEventsResponse>();
219+
applicationEvents.Should().NotBeNull();
220+
applicationEvents!.Events.Should().NotBeEmpty();
221+
applicationEvents.Events.Should().Contain(x => x.EventType == EventType.ApiAuthDisabledSecretKeyUsed.ToString());
222+
}
223+
224+
[Fact]
225+
public async Task I_can_view_the_event_for_using_a_disabled_public_key()
226+
{
227+
// Arrange
228+
var applicationName = CreateAppHelpers.GetApplicationName();
229+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
230+
var accountKeysCreation = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
231+
_client.AddSecretKey(accountKeysCreation!.ApiSecret1);
232+
_client.AddPublicKey(accountKeysCreation.ApiKey1);
233+
await _client.EnableEventLogging(applicationName);
234+
using var getApiKeysResponse = await _client.GetAsync($"/admin/apps/{applicationName}/api-keys");
235+
var apiKeys = await getApiKeysResponse.Content.ReadFromJsonAsync<IReadOnlyCollection<ApiKeyResponse>>();
236+
var keyToLock = apiKeys!.First(x => x.ApiKey.EndsWith(accountKeysCreation.ApiKey1.GetLast(4)));
237+
_ = await _client.PostAsync($"/admin/apps/{applicationName}/api-keys/{keyToLock.Id}/lock", null);
238+
_ = await _client.PostAsJsonAsync("/signin/begin", new SignInBeginDTO { Origin = PasswordlessApiFactory.OriginUrl, RPID = PasswordlessApiFactory.RpId });
239+
_ = await _client.PostAsync($"/admin/apps/{applicationName}/api-keys/{keyToLock.Id}/unlock", null);
240+
241+
// Act
242+
using var getApplicationEventsResponse = await _client.GetAsync("events?pageNumber=1");
243+
244+
// Assert
245+
getApplicationEventsResponse.StatusCode.Should().Be(HttpStatusCode.OK);
246+
var applicationEvents = await getApplicationEventsResponse.Content.ReadFromJsonAsync<EventLog.GetEventLogEventsResponse>();
247+
applicationEvents.Should().NotBeNull();
248+
applicationEvents!.Events.Should().NotBeEmpty();
249+
applicationEvents.Events.Should().Contain(x => x.EventType == EventType.ApiAuthDisabledPublicKeyUsed.ToString());
250+
}
251+
252+
[Fact]
253+
public async Task I_can_view_the_event_for_using_a_non_existent_api_key()
254+
{
255+
// Arrange
256+
var applicationName = CreateAppHelpers.GetApplicationName();
257+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
258+
var accountKeysCreation = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
259+
_client.AddSecretKey(accountKeysCreation!.ApiSecret1);
260+
_client.AddPublicKey($"{applicationName}:public:invalid-public-key");
261+
await _client.EnableEventLogging(applicationName);
262+
_ = await _client.PostAsJsonAsync("/signin/begin", new SignInBeginDTO { Origin = PasswordlessApiFactory.OriginUrl, RPID = PasswordlessApiFactory.RpId });
263+
264+
// Act
265+
using var getApplicationEventsResponse = await _client.GetAsync("events?pageNumber=1");
266+
267+
// Assert
268+
getApplicationEventsResponse.StatusCode.Should().Be(HttpStatusCode.OK);
269+
var applicationEvents = await getApplicationEventsResponse.Content.ReadFromJsonAsync<EventLog.GetEventLogEventsResponse>();
270+
applicationEvents.Should().NotBeNull();
271+
applicationEvents!.Events.Should().NotBeEmpty();
272+
applicationEvents.Events.Should().Contain(x => x.EventType == EventType.ApiAuthInvalidPublicKeyUsed.ToString());
273+
}
274+
275+
[Fact]
276+
public async Task I_can_view_the_event_for_using_a_non_existent_api_secret()
277+
{
278+
// Arrange
279+
var applicationName = CreateAppHelpers.GetApplicationName();
280+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
281+
var accountKeysCreation = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
282+
_client.AddSecretKey(accountKeysCreation!.ApiSecret1);
283+
_client.AddSecretKey($"{applicationName}:secret:invalid-secret-key");
284+
await _client.EnableEventLogging(applicationName);
285+
_ = await _client.GetAsync("credentials/list");
286+
_client.AddSecretKey(accountKeysCreation!.ApiSecret1);
287+
288+
// Act
289+
using var getApplicationEventsResponse = await _client.GetAsync("events?pageNumber=1");
290+
291+
// Assert
292+
getApplicationEventsResponse.StatusCode.Should().Be(HttpStatusCode.OK);
293+
var applicationEvents = await getApplicationEventsResponse.Content.ReadFromJsonAsync<EventLog.GetEventLogEventsResponse>();
294+
applicationEvents.Should().NotBeNull();
295+
applicationEvents!.Events.Should().NotBeEmpty();
296+
applicationEvents.Events.Should().Contain(x => x.EventType == EventType.ApiAuthInvalidSecretKeyUsed.ToString());
297+
}
298+
196299
public void Dispose()
197300
{
198301
_client.Dispose();

tests/Api.IntegrationTests/Helpers/HttpClientTestExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public static bool HasSecretKey(this HttpClient client) =>
2424

2525
public static HttpClient AddSecretKey(this HttpClient client, string secretKey)
2626
{
27+
if (client.DefaultRequestHeaders.Contains("ApiSecret")) client.DefaultRequestHeaders.Remove("ApiSecret");
2728
client.DefaultRequestHeaders.Add("ApiSecret", secretKey);
2829
return client;
2930
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
using System.Net;
2+
using System.Net.Http.Json;
3+
using Passwordless.Api.IntegrationTests.Helpers;
4+
using Passwordless.Api.IntegrationTests.Helpers.App;
5+
using Passwordless.Common.Models.Apps;
6+
using Xunit;
7+
using Xunit.Abstractions;
8+
9+
namespace Passwordless.Api.IntegrationTests.Middleware;
10+
11+
public class AuthorizationIntegrationTests : IClassFixture<PasswordlessApiFactory>
12+
{
13+
private readonly HttpClient _client;
14+
15+
public AuthorizationIntegrationTests(ITestOutputHelper testOutput, PasswordlessApiFactory apiFactory)
16+
{
17+
apiFactory.TestOutput = testOutput;
18+
_client = apiFactory.CreateClient();
19+
}
20+
21+
[Fact]
22+
public async Task I_receive_a_403_when_i_use_a_invalid_api_key_with_an_existing_endpoint()
23+
{
24+
// Arrange
25+
var applicationName = CreateAppHelpers.GetApplicationName();
26+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
27+
_ = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
28+
_client.AddPublicKey($"{Guid.NewGuid():N}:public:{Guid.NewGuid():N}");
29+
30+
// Act
31+
using var actual = await _client.PostAsJsonAsync("/register/begin", string.Empty);
32+
33+
// Assert
34+
Assert.Equal(HttpStatusCode.Unauthorized, actual.StatusCode);
35+
}
36+
37+
[Fact]
38+
public async Task I_receive_a_403_when_i_use_a_badly_formatted_api_key_with_an_existing_endpoint()
39+
{
40+
// Arrange
41+
var applicationName = CreateAppHelpers.GetApplicationName();
42+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
43+
_ = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
44+
_client.AddPublicKey("e=mc2trooper");
45+
46+
// Act
47+
using var actual = await _client.PostAsJsonAsync("/register/begin", string.Empty);
48+
49+
// Assert
50+
Assert.Equal(HttpStatusCode.Unauthorized, actual.StatusCode);
51+
}
52+
53+
[Fact]
54+
public async Task I_receive_a_403_when_i_use_a_invalid_api_secret_with_an_existing_endpoint()
55+
{
56+
// Arrange
57+
var applicationName = CreateAppHelpers.GetApplicationName();
58+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
59+
_ = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
60+
_client.AddSecretKey($"{Guid.NewGuid():N}:secret:{Guid.NewGuid():N}");
61+
62+
// Act
63+
using var actual = await _client.PostAsJsonAsync("/register/token", string.Empty);
64+
65+
// Assert
66+
Assert.Equal(HttpStatusCode.Unauthorized, actual.StatusCode);
67+
}
68+
69+
[Fact]
70+
public async Task I_receive_a_403_when_i_use_a_badly_formatted_api_secret_with_an_existing_endpoint()
71+
{
72+
// Arrange
73+
var applicationName = CreateAppHelpers.GetApplicationName();
74+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
75+
_ = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
76+
_client.AddSecretKey("e=mc2trooper");
77+
78+
// Act
79+
using var actual = await _client.PostAsJsonAsync("/register/token", string.Empty);
80+
81+
// Assert
82+
Assert.Equal(HttpStatusCode.Unauthorized, actual.StatusCode);
83+
}
84+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
using System.Net;
2+
using System.Net.Http.Json;
3+
using Passwordless.Api.IntegrationTests.Helpers;
4+
using Passwordless.Api.IntegrationTests.Helpers.App;
5+
using Passwordless.Common.Models.Apps;
6+
using Xunit;
7+
using Xunit.Abstractions;
8+
9+
namespace Passwordless.Api.IntegrationTests.Middleware;
10+
11+
public class RoutingIntegrationTests : IClassFixture<PasswordlessApiFactory>
12+
{
13+
private readonly HttpClient _client;
14+
15+
public RoutingIntegrationTests(ITestOutputHelper testOutput, PasswordlessApiFactory apiFactory)
16+
{
17+
apiFactory.TestOutput = testOutput;
18+
_client = apiFactory.CreateClient();
19+
}
20+
21+
[Fact]
22+
public async Task I_receive_a_404_when_i_use_a_badly_formatted_api_secret_with_a_non_existing_endpoint()
23+
{
24+
// Arrange
25+
var applicationName = CreateAppHelpers.GetApplicationName();
26+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
27+
_ = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
28+
_client.AddSecretKey("e=mc2trooper");
29+
30+
// Act
31+
using var actual = await _client.GetAsync("/non-existent-endpoint");
32+
33+
// Assert
34+
Assert.Equal(HttpStatusCode.NotFound, actual.StatusCode);
35+
}
36+
37+
[Fact]
38+
public async Task I_receive_a_404_when_i_use_a_badly_formatted_api_key_with_a_non_existing_endpoint()
39+
{
40+
// Arrange
41+
var applicationName = CreateAppHelpers.GetApplicationName();
42+
using var createApplicationMessage = await _client.CreateApplicationAsync(applicationName);
43+
_ = await createApplicationMessage.Content.ReadFromJsonAsync<CreateAppResultDto>();
44+
_client.AddPublicKey("e=mc2trooper");
45+
46+
// Act
47+
using var actual = await _client.GetAsync("/non-existent-endpoint");
48+
49+
// Assert
50+
Assert.Equal(HttpStatusCode.NotFound, actual.StatusCode);
51+
}
52+
}

0 commit comments

Comments
 (0)