From 422088298157e10d119dec82ac23f764a3c6e5c3 Mon Sep 17 00:00:00 2001 From: James Gunn Date: Tue, 9 Apr 2024 16:46:25 +0100 Subject: [PATCH] Use a cookie to prevent FormFlow journey theft (#1268) --- .../FormFlow/DummyCurrentUserIdProvider.cs | 8 -- .../FormFlowSessionCurrentUserIdProvider.cs | 12 +++ .../FormFlow/FormFlowSessionIdFeature.cs | 6 ++ .../FormFlow/FormFlowSessionMiddleware.cs | 90 +++++++++++++++++++ .../Program.cs | 4 +- 5 files changed, 111 insertions(+), 9 deletions(-) delete mode 100644 TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/DummyCurrentUserIdProvider.cs create mode 100644 TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionCurrentUserIdProvider.cs create mode 100644 TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionIdFeature.cs create mode 100644 TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionMiddleware.cs diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/DummyCurrentUserIdProvider.cs b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/DummyCurrentUserIdProvider.cs deleted file mode 100644 index 95879c41b..000000000 --- a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/DummyCurrentUserIdProvider.cs +++ /dev/null @@ -1,8 +0,0 @@ -using TeachingRecordSystem.SupportUi.Infrastructure.FormFlow; - -namespace TeachingRecordSystem.AuthorizeAccess.Infrastructure.FormFlow; - -public class DummyCurrentUserIdProvider : ICurrentUserIdProvider -{ - public string GetCurrentUserId() => Guid.Empty.ToString(); -} diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionCurrentUserIdProvider.cs b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionCurrentUserIdProvider.cs new file mode 100644 index 000000000..d909da11b --- /dev/null +++ b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionCurrentUserIdProvider.cs @@ -0,0 +1,12 @@ +using TeachingRecordSystem.SupportUi.Infrastructure.FormFlow; + +namespace TeachingRecordSystem.AuthorizeAccess.Infrastructure.FormFlow; + +public class FormFlowSessionCurrentUserIdProvider(IHttpContextAccessor httpContextAccessor) : ICurrentUserIdProvider +{ + public string GetCurrentUserId() + { + var httpContext = httpContextAccessor.HttpContext ?? throw new InvalidOperationException("No HttpContext."); + return httpContext.Features.Get()?.SessionId ?? throw new InvalidOperationException($"No {nameof(FormFlowSessionIdFeature)} set."); + } +} diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionIdFeature.cs b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionIdFeature.cs new file mode 100644 index 000000000..69cdb2dc6 --- /dev/null +++ b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionIdFeature.cs @@ -0,0 +1,6 @@ +namespace TeachingRecordSystem.AuthorizeAccess.Infrastructure.FormFlow; + +public class FormFlowSessionIdFeature(string sessionId) +{ + public string SessionId { get; } = sessionId; +} diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionMiddleware.cs b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionMiddleware.cs new file mode 100644 index 000000000..2586b2901 --- /dev/null +++ b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Infrastructure/FormFlow/FormFlowSessionMiddleware.cs @@ -0,0 +1,90 @@ +using System.Diagnostics.CodeAnalysis; +using System.Security.Cryptography; +using Microsoft.AspNetCore.DataProtection; + +namespace TeachingRecordSystem.AuthorizeAccess.Infrastructure.FormFlow; + +public class FormFlowSessionMiddleware(RequestDelegate next, IDataProtectionProvider dataProtectionProvider) +{ + private const string CookieName = "ffsessid"; + + private readonly IDataProtector _dataProtector = dataProtectionProvider.CreateProtector(nameof(FormFlowSessionMiddleware)); + + private readonly CookieOptions _cookieOptions = new() + { + HttpOnly = true, + IsEssential = true, + Secure = true, + }; + + public async Task Invoke(HttpContext context) + { + if (!TryExtractSessionIdFromRequest(context, out var sessionId)) + { + sessionId = AddNewSessionIdToResponse(context); + } + + context.Features.Set(new FormFlowSessionIdFeature(sessionId)); + + try + { + await next(context); + } + finally + { + context.Features.Set(null); + } + } + + private string AddNewSessionIdToResponse(HttpContext context) + { + var sessionId = CreateSessionId(); + var cookieValue = _dataProtector.Protect(sessionId); + + context.Response.OnStarting( + state => + { + var (context, cookieValue) = ((HttpContext, string))state; + context.Response.Cookies.Append(CookieName, cookieValue, _cookieOptions); + return Task.CompletedTask; + }, + (context, cookieValue)); + + return sessionId.ToString(); + + static string CreateSessionId() + { + Span guidBytes = stackalloc byte[16]; + RandomNumberGenerator.Fill(guidBytes); + return new Guid(guidBytes).ToString(); + } + } + + private bool TryExtractSessionIdFromRequest(HttpContext context, [NotNullWhen(true)] out string? sessionId) + { + if (context.Request.Cookies.TryGetValue(CookieName, out var cookieValue)) + { + try + { + var bytes = _dataProtector.Unprotect(cookieValue); + sessionId = new Guid(bytes).ToString(); + return true; + } + catch (CryptographicException) + { + } + } + + sessionId = null; + return false; + } +} + +public class FormFlowSessionMiddlewareStartupFilter : IStartupFilter +{ + public Action Configure(Action next) => app => + { + app.UseMiddleware(); + next(app); + }; +} diff --git a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Program.cs b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Program.cs index da7e8c9b8..4590bfc8c 100644 --- a/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Program.cs +++ b/TeachingRecordSystem/src/TeachingRecordSystem.AuthorizeAccess/Program.cs @@ -151,11 +151,13 @@ static SecurityKey LoadKey(string configurationValue) .AddTransient() .AddTransient() .AddTransient() + .AddHttpContextAccessor() + .AddSingleton() .AddFormFlow(options => { options.JourneyRegistry.RegisterJourney(SignInJourneyState.JourneyDescriptor); }) - .AddSingleton() + .AddSingleton() .AddTransient() .AddSingleton, FormTagHelperInitializer>() .AddPersonSearch();