From 1be7900b488da4ca23f3a1c56053f8ae3e1b4515 Mon Sep 17 00:00:00 2001 From: Felix Cornelissen Date: Mon, 21 Oct 2024 14:39:08 +0200 Subject: [PATCH 1/2] fix: make sure the return path is relative in the challenge endpoint --- .../Authentication/AuthenticationExtensions.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/ODPC.Server/Authentication/AuthenticationExtensions.cs b/ODPC.Server/Authentication/AuthenticationExtensions.cs index 71c217f..b6800a4 100644 --- a/ODPC.Server/Authentication/AuthenticationExtensions.cs +++ b/ODPC.Server/Authentication/AuthenticationExtensions.cs @@ -140,22 +140,25 @@ private static Task HandleLoggedOut(RedirectContext ctx) whe private static Task ChallengeAsync(HttpContext httpContext) { var request = httpContext.Request; - var returnUrl = (request.Query["returnUrl"].FirstOrDefault() ?? string.Empty) - .AsSpan() - .TrimStart('/'); - - var fullReturnUrl = $"{request.Scheme}://{request.Host}{request.PathBase}/{returnUrl}"; + var returnPath = GetSafeReturnPath(request); if (httpContext.User.Identity?.IsAuthenticated ?? false) { - httpContext.Response.Redirect(fullReturnUrl); + httpContext.Response.Redirect(returnPath); return Task.CompletedTask; } return httpContext.ChallengeAsync(new AuthenticationProperties { - RedirectUri = fullReturnUrl, + RedirectUri = returnPath, }); } + + private static string GetSafeReturnPath(HttpRequest request) + { + var returnUrl = request.Query["returnUrl"].FirstOrDefault(); + if (string.IsNullOrWhiteSpace(returnUrl) || new Uri(returnUrl, UriKind.RelativeOrAbsolute).IsAbsoluteUri) return "/"; + return $"/{returnUrl.AsSpan().TrimStart('/')}"; + } } } From a5ee76690fff2efe8af9e0715fcac14b17cf27f0 Mon Sep 17 00:00:00 2001 From: Felix Cornelissen Date: Mon, 21 Oct 2024 15:09:00 +0200 Subject: [PATCH 2/2] review feedback: commentaar toevoegen --- .../Authentication/AuthenticationExtensions.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ODPC.Server/Authentication/AuthenticationExtensions.cs b/ODPC.Server/Authentication/AuthenticationExtensions.cs index b6800a4..9820353 100644 --- a/ODPC.Server/Authentication/AuthenticationExtensions.cs +++ b/ODPC.Server/Authentication/AuthenticationExtensions.cs @@ -140,7 +140,7 @@ private static Task HandleLoggedOut(RedirectContext ctx) whe private static Task ChallengeAsync(HttpContext httpContext) { var request = httpContext.Request; - var returnPath = GetSafeReturnPath(request); + var returnPath = GetRelativeReturnUrl(request); if (httpContext.User.Identity?.IsAuthenticated ?? false) { @@ -154,7 +154,14 @@ private static Task ChallengeAsync(HttpContext httpContext) }); } - private static string GetSafeReturnPath(HttpRequest request) + /// + /// We gebruiken een query parameter om te bepalen waar we naartoe moeten redirecten na inlog. + /// Dat is gebruikersinput. Daarom willen we valideren dat die query parameter daadwerkelijk een relatieve url is. + /// Zo niet, redirecten we naar de root van de applicatie. + /// + /// + /// + private static string GetRelativeReturnUrl(HttpRequest request) { var returnUrl = request.Query["returnUrl"].FirstOrDefault(); if (string.IsNullOrWhiteSpace(returnUrl) || new Uri(returnUrl, UriKind.RelativeOrAbsolute).IsAbsoluteUri) return "/";