diff --git a/src/studio/modules/Elsa.Studio.Authentication.Abstractions/LocalReturnPath.cs b/src/studio/modules/Elsa.Studio.Authentication.Abstractions/LocalReturnPath.cs new file mode 100644 index 0000000000..3bb3444574 --- /dev/null +++ b/src/studio/modules/Elsa.Studio.Authentication.Abstractions/LocalReturnPath.cs @@ -0,0 +1,60 @@ +namespace Elsa.Studio.Authentication.Abstractions; + +/// +/// Neutralizes open-redirect targets so post-authentication navigation stays inside the Studio host. +/// +public static class LocalReturnPath +{ + private const int MaxDecodePasses = 8; + + /// + /// Returns the original candidate when it is a rooted local path, or / when it is missing, relative, absolute, protocol-relative, or otherwise unsafe. + /// + /// + /// Validation runs on a decoded copy. The returned value is the original string so encoded query values and path segments stay byte-for-byte intact. + /// Only a single leading / (not // or /\) is accepted. Decoding that does not reach a fixed point within fails closed to /. + /// + public static string Normalize(string? candidate) + { + if (string.IsNullOrEmpty(candidate)) + { + return "/"; + } + + var decoded = candidate; + for (var pass = 0; pass < MaxDecodePasses; pass++) + { + var next = Uri.UnescapeDataString(decoded); + if (next == decoded) + { + return IsRootedLocalPath(candidate) && IsRootedLocalPath(decoded) ? candidate : "/"; + } + + decoded = next; + } + + return "/"; + } + + /// + /// Roots a base-relative candidate against 's path, then normalizes. + /// + public static string RootAgainstBase(string? candidate, string baseUri) + { + var basePath = new Uri(baseUri).AbsolutePath; + if (!basePath.EndsWith('/')) + { + basePath += "/"; + } + + var rooted = candidate is { Length: > 0 } && candidate[0] != '/' + ? basePath + candidate + : candidate; + return Normalize(rooted); + } + + private static bool IsRootedLocalPath(string path) => + path.StartsWith('/') && + !path.StartsWith("//", StringComparison.Ordinal) && + !path.Any(c => c == '\\' || char.IsControl(c)); +} diff --git a/src/studio/modules/Elsa.Studio.Authentication.ElsaIdentity.UI/Components/ElsaIdentityLoginMethod.razor b/src/studio/modules/Elsa.Studio.Authentication.ElsaIdentity.UI/Components/ElsaIdentityLoginMethod.razor index 050647c8dc..2e694fd386 100644 --- a/src/studio/modules/Elsa.Studio.Authentication.ElsaIdentity.UI/Components/ElsaIdentityLoginMethod.razor +++ b/src/studio/modules/Elsa.Studio.Authentication.ElsaIdentity.UI/Components/ElsaIdentityLoginMethod.razor @@ -50,6 +50,6 @@ await JwtAccessor.WriteTokenAsync(Elsa.Studio.Authentication.ElsaIdentity.TokenNames.RefreshToken, result.RefreshToken!); if (AuthenticationStateProvider is Elsa.Studio.Authentication.ElsaIdentity.Services.AccessTokenAuthenticationStateProvider stateProvider) stateProvider.NotifyAuthenticationStateChanged(); - Navigation.NavigateTo(Context.ReturnPath, forceLoad: true); + Navigation.NavigateTo(Elsa.Studio.Authentication.Abstractions.LocalReturnPath.RootAgainstBase(Context.ReturnPath, Navigation.BaseUri), forceLoad: true); } } diff --git a/src/studio/modules/Elsa.Studio.Authentication.OpenIdConnect.BlazorServer/Controllers/AuthenticationController.cs b/src/studio/modules/Elsa.Studio.Authentication.OpenIdConnect.BlazorServer/Controllers/AuthenticationController.cs index 7b8f8994c3..8bf038408a 100644 --- a/src/studio/modules/Elsa.Studio.Authentication.OpenIdConnect.BlazorServer/Controllers/AuthenticationController.cs +++ b/src/studio/modules/Elsa.Studio.Authentication.OpenIdConnect.BlazorServer/Controllers/AuthenticationController.cs @@ -1,3 +1,4 @@ +using Elsa.Studio.Authentication.Abstractions; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authentication.OpenIdConnect; @@ -17,7 +18,7 @@ public class AuthenticationController : Controller [HttpGet("login")] public IActionResult Login([FromQuery] string? returnUrl = null) { - return Challenge(new AuthenticationProperties { RedirectUri = NormalizeReturnUrl(returnUrl) }, OpenIdConnectDefaults.AuthenticationScheme); + return Challenge(new AuthenticationProperties { RedirectUri = SafeRedirectUri(returnUrl) }, OpenIdConnectDefaults.AuthenticationScheme); } /// @@ -27,21 +28,19 @@ public IActionResult Login([FromQuery] string? returnUrl = null) public IActionResult Logout([FromQuery] string? returnUrl = null) { return SignOut( - new AuthenticationProperties { RedirectUri = NormalizeReturnUrl(returnUrl) }, + new AuthenticationProperties { RedirectUri = SafeRedirectUri(returnUrl) }, CookieAuthenticationDefaults.AuthenticationScheme, OpenIdConnectDefaults.AuthenticationScheme); } - private static string NormalizeReturnUrl(string? candidate) + private string SafeRedirectUri(string? returnUrl) { - if (string.IsNullOrWhiteSpace(candidate) || - !candidate.StartsWith("/", StringComparison.Ordinal) || - candidate.StartsWith("//", StringComparison.Ordinal) || - candidate.StartsWith("/\\", StringComparison.Ordinal)) + var path = LocalReturnPath.Normalize(returnUrl); + if (!Url.IsLocalUrl(path)) { return "/"; } - return Uri.TryCreate(candidate, UriKind.Relative, out _) ? candidate : "/"; + return path; } } diff --git a/src/studio/modules/Elsa.Studio.Authentication.UI/Components/LoginPanel.razor b/src/studio/modules/Elsa.Studio.Authentication.UI/Components/LoginPanel.razor index e8c2d00eba..2c9f88045b 100644 --- a/src/studio/modules/Elsa.Studio.Authentication.UI/Components/LoginPanel.razor +++ b/src/studio/modules/Elsa.Studio.Authentication.UI/Components/LoginPanel.razor @@ -199,14 +199,6 @@ string.Equals(method.Kind, "local", StringComparison.OrdinalIgnoreCase) || string.Equals(method.Kind, "elsa-identity", StringComparison.OrdinalIgnoreCase); - private static string NormalizeReturnPath(string? candidate) - { - if (string.IsNullOrWhiteSpace(candidate) || - !candidate.StartsWith("/", StringComparison.Ordinal) || - candidate.StartsWith("//", StringComparison.Ordinal) || - candidate.Contains('\\')) - return "/"; - - return Uri.TryCreate(candidate, UriKind.Relative, out _) ? candidate : "/"; - } + private string NormalizeReturnPath(string? candidate) => + LocalReturnPath.RootAgainstBase(candidate, Navigation.BaseUri); } diff --git a/src/studio/modules/Elsa.Studio.Authentication.UI/_Imports.razor b/src/studio/modules/Elsa.Studio.Authentication.UI/_Imports.razor index 7de5c83048..814baa6b01 100644 --- a/src/studio/modules/Elsa.Studio.Authentication.UI/_Imports.razor +++ b/src/studio/modules/Elsa.Studio.Authentication.UI/_Imports.razor @@ -1,3 +1,4 @@ +@using Elsa.Studio.Authentication.Abstractions @using Elsa.Studio.Authentication.Abstractions.Contracts @using Elsa.Studio.Authentication.Abstractions.Models @using Elsa.Studio.Authentication.UI.Components diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.BlazorWasm/Services/ExternalAuthenticationReturnPath.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.BlazorWasm/Services/ExternalAuthenticationReturnPath.cs index 9e09d0f184..9f94f694d0 100644 --- a/src/studio/modules/Elsa.Studio.ExternalAuthentication.BlazorWasm/Services/ExternalAuthenticationReturnPath.cs +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.BlazorWasm/Services/ExternalAuthenticationReturnPath.cs @@ -1,21 +1,10 @@ +using Elsa.Studio.Authentication.Abstractions; + namespace Elsa.Studio.ExternalAuthentication.BlazorWasm.Services; /// Validates targets which remain inside the registered Studio client. public static class ExternalAuthenticationReturnPath { /// Returns a safe client-local path, falling back to the Studio root. - public static string Normalize(string? value) - { - if (string.IsNullOrWhiteSpace(value) || - !value.StartsWith("/", StringComparison.Ordinal) || - value.StartsWith("//", StringComparison.Ordinal) || - value.Contains("\\", StringComparison.Ordinal) || - !Uri.TryCreate(value, UriKind.Relative, out var uri) || - uri.IsAbsoluteUri) - { - return "/"; - } - - return value; - } + public static string Normalize(string? value) => LocalReturnPath.Normalize(value); } diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorServer/ServerBrokerAuthenticationTests.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorServer/ServerBrokerAuthenticationTests.cs index ae02b6a92c..8aa3e0042b 100644 --- a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorServer/ServerBrokerAuthenticationTests.cs +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorServer/ServerBrokerAuthenticationTests.cs @@ -278,6 +278,62 @@ public async Task LocalLoginCallbackError_ReturnsToChooserWithASafeErrorOutcome( Assert.Contains("error=sign_in_failed", redirect.Url); } + [Fact] + public async Task Callback_RelativeReturnPath_LocalRedirectsToRoot() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddAuthentication(ServerExternalAuthenticationStateProvider.Scheme) + .AddCookie(ServerExternalAuthenticationStateProvider.Scheme); + using var serviceProvider = services.BuildServiceProvider(); + using var callbackScope = serviceProvider.CreateScope(); + var context = new DefaultHttpContext { RequestServices = callbackScope.ServiceProvider }; + context.Request.Scheme = "https"; + context.Request.Host = new HostString("studio.example.test"); + var accessor = new HttpContextAccessor { HttpContext = context }; + var options = new ExternalAuthenticationClientOptions { ClientId = "studio-server", ClientSecret = "secret" }; + var broker = new FakeBrokerApi(); + var anonymous = new FakeAnonymousBackendApiClientProvider(broker); + var stateProvider = new ServerExternalAuthenticationStateProvider(accessor, anonymous, new ServerExternalAuthenticationRefreshCoordinator(), options); + var controller = CreateController(context, anonymous, new FakeTransactionStore(new("state", "verifier", "workflows/x", DateTimeOffset.UtcNow.AddMinutes(1))), stateProvider, options); + + var result = await controller.Callback("completion-code", "state", null, CancellationToken.None); + + Assert.Equal("/", Assert.IsType(result).Url); + } + + [Fact] + public async Task Logout_RelativeReturnPath_LocalRedirectsToRoot() + { + var ticket = CreateCurrentTicket(); + var context = CreateAuthenticatedContext(ticket.Principal, new RecordingAuthenticationService(ticket)); + var broker = new FakeBrokerApi { LogoutResult = new(true, null, null) }; + var anonymous = new FakeAnonymousBackendApiClientProvider(broker); + var transactions = new FakeTransactionStore(new("unused", "", "/", DateTimeOffset.UtcNow.AddMinutes(1))); + var state = new ServerExternalAuthenticationStateProvider(new HttpContextAccessor { HttpContext = context }, anonymous, new ServerExternalAuthenticationRefreshCoordinator(), Options()); + + var result = await CreateController(context, anonymous, transactions, state, Options()).Logout("local", "workflows/x", CancellationToken.None); + + Assert.Equal("/", Assert.IsType(result).Url); + } + + [Fact] + public void LogoutCallback_RelativeReturnPath_LocalRedirectsToRoot() + { + var context = new DefaultHttpContext(); + var anonymous = new FakeAnonymousBackendApiClientProvider(); + var transactions = new FakeTransactionStore(new("unused", "", "workflows/x", DateTimeOffset.UtcNow.AddMinutes(1), "logout")); + var state = new ServerExternalAuthenticationStateProvider( + new HttpContextAccessor { HttpContext = context }, + anonymous, + new ServerExternalAuthenticationRefreshCoordinator(), + Options()); + + var result = CreateController(context, anonymous, transactions, state, Options()).LogoutCallback(); + + Assert.Equal("/", Assert.IsType(result).Url); + } + [Fact] public async Task Callback_UsesConfidentialBasicExchangeAndExactState() { diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorWasm/ExternalAuthenticationWasmTests.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorWasm/ExternalAuthenticationWasmTests.cs index ffc25c7a4b..47819809d7 100644 --- a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorWasm/ExternalAuthenticationWasmTests.cs +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/BlazorWasm/ExternalAuthenticationWasmTests.cs @@ -6,6 +6,7 @@ using Elsa.Studio.ExternalAuthentication.BlazorWasm.Services; using Elsa.Studio.ExternalAuthentication.Client; using Elsa.Studio.ExternalAuthentication.Models; +using Elsa.Studio.ExternalAuthentication.Tests.Login; using Microsoft.AspNetCore.Components; using Microsoft.Extensions.DependencyInjection; using Microsoft.JSInterop; @@ -16,12 +17,7 @@ namespace Elsa.Studio.ExternalAuthentication.Tests.BlazorWasm; public class ExternalAuthenticationWasmTests { [Theory] - [InlineData(null, "/")] - [InlineData("", "/")] - [InlineData("/workflows?version=1", "/workflows?version=1")] - [InlineData("https://evil.example/", "/")] - [InlineData("//evil.example/", "/")] - [InlineData("/\\evil", "/")] + [MemberData(nameof(LocalReturnPathCorpus.Cases), MemberType = typeof(LocalReturnPathCorpus))] public void ReturnPathsRemainClientLocal(string? candidate, string expected) => Assert.Equal(expected, ExternalAuthenticationReturnPath.Normalize(candidate)); diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Compatibility/DirectOpenIdConnectLoginTests.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Compatibility/DirectOpenIdConnectLoginTests.cs index e0f5e4beba..61bce4c2e2 100644 --- a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Compatibility/DirectOpenIdConnectLoginTests.cs +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Compatibility/DirectOpenIdConnectLoginTests.cs @@ -1,7 +1,11 @@ using Elsa.Studio.Authentication.OpenIdConnect.BlazorServer.Controllers; using Elsa.Studio.Authentication.OpenIdConnect.BlazorServer.Services; -using Microsoft.AspNetCore.Authentication; +using Elsa.Studio.ExternalAuthentication.Tests.Login; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Routing; +using Microsoft.AspNetCore.Routing; using Xunit; namespace Elsa.Studio.ExternalAuthentication.Tests.Compatibility; @@ -19,15 +23,41 @@ public async Task DirectOpenIdConnectContributesItsLegacyChallengeToTheSharedLog } [Theory] - [InlineData("https://attacker.example", "/")] - [InlineData("//attacker.example", "/")] - [InlineData("/\\attacker.example", "/")] - [InlineData("/workflows?version=1", "/workflows?version=1")] - public void DirectOpenIdConnectChallengeAcceptsOnlyLocalReturnUrls(string returnUrl, string expectedReturnUrl) + [MemberData(nameof(LocalReturnPathCorpus.Cases), MemberType = typeof(LocalReturnPathCorpus))] + public void DirectOpenIdConnectChallengeAcceptsOnlyLocalReturnUrls(string? returnUrl, string expectedReturnUrl) { - var result = Assert.IsType(new AuthenticationController().Login(returnUrl)); + var result = Assert.IsType(CreateController().Login(returnUrl)); Assert.NotNull(result.Properties); Assert.Equal(expectedReturnUrl, result.Properties!.RedirectUri); } + + [Theory] + [MemberData(nameof(LocalReturnPathCorpus.Cases), MemberType = typeof(LocalReturnPathCorpus))] + public void DirectOpenIdConnectSignOutAcceptsOnlyLocalReturnUrls(string? returnUrl, string expectedReturnUrl) + { + var result = Assert.IsType(CreateController().Logout(returnUrl)); + + Assert.NotNull(result.Properties); + Assert.Equal(expectedReturnUrl, result.Properties!.RedirectUri); + } + + [Fact] + public void DirectOpenIdConnectChallengeRejectsBoundControlCharacterReturnUrls() + { + var result = Assert.IsType(CreateController().Login("/\t/evil.com")); + + Assert.NotNull(result.Properties); + Assert.Equal("/", result.Properties!.RedirectUri); + } + + private static AuthenticationController CreateController() + { + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ControllerActionDescriptor()); + return new AuthenticationController + { + ControllerContext = new ControllerContext(actionContext), + Url = new UrlHelper(actionContext) + }; + } } diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathCorpus.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathCorpus.cs new file mode 100644 index 0000000000..0bfdf7e96e --- /dev/null +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathCorpus.cs @@ -0,0 +1,62 @@ +using Xunit; + +namespace Elsa.Studio.ExternalAuthentication.Tests.Login; + +/// +/// Shared table-driven return-path cases used by LocalReturnPath, OIDC, WASM, and chooser tests. +/// +public static class LocalReturnPathCorpus +{ + public static TheoryData Cases { get; } = CreateCases(); + + public static string EncodeTimes(string value, int times) + { + var current = value; + for (var i = 0; i < times; i++) + { + current = Uri.EscapeDataString(current); + } + + return current; + } + + private static TheoryData CreateCases() + { + var cases = new TheoryData + { + { null, "/" }, + { "", "/" }, + { "/%09/evil.com", "/" }, + { "//evil.com", "/" }, + { "/\\evil.com", "/" }, + { "\\\\evil.com", "/" }, + { "/%2F/evil.com", "/" }, + { "http://evil.com", "/" }, + { "https://evil.com/phish", "/" }, + { "javascript:alert(1)", "/" }, + { "http:evil.com", "/" }, + { "javascript%3Aalert(1)", "/" }, + { "%6Aavascript:alert(1)", "/" }, + { "\t//evil.com", "/" }, + { "c|/windows", "/" }, + { "c|evil.com", "/" }, + { " //evil.com", "/" }, + { "workflows/x", "/" }, + { "workflows/definitions?tab=active", "/" }, + { "workflows/instances", "/" }, + { "/workflows/definitions?x=1", "/workflows/definitions?x=1" }, + { "/workflows?version=1", "/workflows?version=1" }, + { "/caf%C3%A9", "/caf%C3%A9" }, + { "/a?x=%26y", "/a?x=%26y" }, + { "/a?ref=https%3A%2F%2Fexample.com", "/a?ref=https%3A%2F%2Fexample.com" }, + { "/a%2Fb", "/a%2Fb" }, + { "/a?ref=https://example.com", "/a?ref=https://example.com" } + }; + + cases.Add(EncodeTimes("//evil.com", 9), "/"); + cases.Add(EncodeTimes("//", 9), "/"); + cases.Add("/" + EncodeTimes("//evil.com", 9), "/"); + + return cases; + } +} diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathTests.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathTests.cs new file mode 100644 index 0000000000..0c0b30e845 --- /dev/null +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathTests.cs @@ -0,0 +1,43 @@ +using Elsa.Studio.Authentication.Abstractions; +using Xunit; + +namespace Elsa.Studio.ExternalAuthentication.Tests.Login; + +public class LocalReturnPathTests +{ + [Theory] + [MemberData(nameof(LocalReturnPathCorpus.Cases), MemberType = typeof(LocalReturnPathCorpus))] + public void Normalize_RejectsOpenRedirectsAndPreservesLocalPaths(string? candidate, string expected) + { + Assert.Equal(expected, LocalReturnPath.Normalize(candidate)); + Assert.Equal(expected, Elsa.Studio.ExternalAuthentication.Services.LocalReturnPath.Normalize(candidate)); + } + + [Fact] + public void Normalize_RejectsDecodedControlCharactersThatCreateProtocolRelativeUrls() + { + Assert.Equal("/", LocalReturnPath.Normalize("/\t/evil.com")); + Assert.Equal("/", LocalReturnPath.Normalize("/\r/evil.com")); + Assert.Equal("/", LocalReturnPath.Normalize("/\n/evil.com")); + } + + [Fact] + public void Normalize_RejectsNineTimesEncodedProtocolRelativeUrls() + { + Assert.Equal("/", LocalReturnPath.Normalize(LocalReturnPathCorpus.EncodeTimes("//evil.com", 9))); + Assert.Equal("/", LocalReturnPath.Normalize("/" + LocalReturnPathCorpus.EncodeTimes("//evil.com", 9))); + } + + [Fact] + public void ElsaLogin_RootsBaseRelativePathsAgainstPathBase() + { + const string studioBase = "https://studio.example/studio/"; + + Assert.Equal("/studio/workflows/x", Elsa.Studio.Login.Pages.Login.Login.ResolveReturnUrl("workflows/x", studioBase)); + Assert.Equal("/studio/workflows/definitions?tab=active", Elsa.Studio.Login.Pages.Login.Login.ResolveReturnUrl("workflows/definitions?tab=active", studioBase)); + Assert.Equal("/studio/c|/windows", Elsa.Studio.Login.Pages.Login.Login.ResolveReturnUrl("c|/windows", studioBase)); + Assert.Equal("/studio/http:evil.com", Elsa.Studio.Login.Pages.Login.Login.ResolveReturnUrl("http:evil.com", studioBase)); + Assert.Equal("/", Elsa.Studio.Login.Pages.Login.Login.ResolveReturnUrl("//evil.com", studioBase)); + Assert.Equal("/workflows/definitions?x=1", Elsa.Studio.Login.Pages.Login.Login.ResolveReturnUrl("/workflows/definitions?x=1", studioBase)); + } +} diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LoginChooserTests.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LoginChooserTests.cs index b3272f360c..69edd14a0a 100644 --- a/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LoginChooserTests.cs +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LoginChooserTests.cs @@ -103,14 +103,10 @@ public void TrustedIconRegistry_UsesOnlyRegistrationsAndFallsBackSafely() Assert.Equal("Identity provider", registry.Resolve("https://untrusted.example/icon.svg").AccessibleName); } - [Fact] - public void InvalidReturnPaths_AreNeverForwarded() - { - Assert.Equal("/", LocalReturnPath.Normalize("https://attacker.example")); - Assert.Equal("/", LocalReturnPath.Normalize("//attacker.example")); - Assert.Equal("/", LocalReturnPath.Normalize("/\\attacker.example")); - Assert.Equal("/workflows?tab=active", LocalReturnPath.Normalize("/workflows?tab=active")); - } + [Theory] + [MemberData(nameof(LocalReturnPathCorpus.Cases), MemberType = typeof(LocalReturnPathCorpus))] + public void InvalidReturnPaths_AreNeverForwarded(string? candidate, string expected) => + Assert.Equal(expected, LocalReturnPath.Normalize(candidate)); [Fact] public void Chooser_RendersTextFirstLocalAndExternalMethods() diff --git a/src/studio/modules/Elsa.Studio.ExternalAuthentication/Services/LoginMethodChooserState.cs b/src/studio/modules/Elsa.Studio.ExternalAuthentication/Services/LoginMethodChooserState.cs index aefc2dc253..b0607ffe74 100644 --- a/src/studio/modules/Elsa.Studio.ExternalAuthentication/Services/LoginMethodChooserState.cs +++ b/src/studio/modules/Elsa.Studio.ExternalAuthentication/Services/LoginMethodChooserState.cs @@ -15,14 +15,6 @@ public static IReadOnlyList Order(IEnumerableGuards local post-authentication paths before a host constructs broker requests. public static class LocalReturnPath { - public static string Normalize(string? candidate) - { - if (string.IsNullOrWhiteSpace(candidate) || - !candidate.StartsWith("/", StringComparison.Ordinal) || - candidate.StartsWith("//", StringComparison.Ordinal) || - candidate.Contains('\\')) - return "/"; - - return Uri.TryCreate(candidate, UriKind.Relative, out _) ? candidate : "/"; - } + public static string Normalize(string? candidate) => + Authentication.Abstractions.LocalReturnPath.Normalize(candidate); } diff --git a/src/studio/modules/Elsa.Studio.Localization.BlazorServer/Controllers/CultureController.cs b/src/studio/modules/Elsa.Studio.Localization.BlazorServer/Controllers/CultureController.cs index 71aa4e7ba0..f0cda1c14a 100644 --- a/src/studio/modules/Elsa.Studio.Localization.BlazorServer/Controllers/CultureController.cs +++ b/src/studio/modules/Elsa.Studio.Localization.BlazorServer/Controllers/CultureController.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Localization; using Microsoft.AspNetCore.Mvc; @@ -27,6 +27,11 @@ public IActionResult Set(string? culture, string redirectUri) new CookieOptions { Secure = true, HttpOnly = true, SameSite = SameSiteMode.Lax }); } + if (!Url.IsLocalUrl(redirectUri)) + { + return LocalRedirect("~/"); + } + return LocalRedirect(redirectUri); } } diff --git a/src/studio/modules/Elsa.Studio.Login/Elsa.Studio.Login.csproj b/src/studio/modules/Elsa.Studio.Login/Elsa.Studio.Login.csproj index 9046a7212d..d95149566f 100644 --- a/src/studio/modules/Elsa.Studio.Login/Elsa.Studio.Login.csproj +++ b/src/studio/modules/Elsa.Studio.Login/Elsa.Studio.Login.csproj @@ -24,4 +24,8 @@ + + + + diff --git a/src/studio/modules/Elsa.Studio.Login/Pages/Login/Login.razor.cs b/src/studio/modules/Elsa.Studio.Login/Pages/Login/Login.razor.cs index 1cf3c4cb57..779064e762 100644 --- a/src/studio/modules/Elsa.Studio.Login/Pages/Login/Login.razor.cs +++ b/src/studio/modules/Elsa.Studio.Login/Pages/Login/Login.razor.cs @@ -1,3 +1,4 @@ +using Elsa.Studio.Authentication.Abstractions; using Elsa.Studio.Contracts; using Elsa.Studio.Login.Contracts; using Elsa.Studio.Login.Services; @@ -49,16 +50,21 @@ private async Task TryLogin(LoginArgs args) ((AccessTokenAuthenticationStateProvider)AuthenticationStateProvider).NotifyAuthenticationStateChanged(); var uri = new Uri(NavigationManager.Uri); - if (QueryHelpers.ParseQuery(uri.Query).TryGetValue("returnUrl", out var returnUrl)) + string? returnUrl = null; + if (QueryHelpers.ParseQuery(uri.Query).TryGetValue("returnUrl", out var values)) { - NavigationManager.NavigateTo(returnUrl.FirstOrDefault() ?? "", true); - } - else - { - NavigationManager.NavigateTo("", true); + returnUrl = values.FirstOrDefault(); } + + NavigationManager.NavigateTo(ResolveReturnUrl(returnUrl, NavigationManager.BaseUri), true); } + /// + /// Roots a legacy ElsaLogin returnUrl against the Studio base path before the shared helper accepts only rooted destinations. + /// + internal static string ResolveReturnUrl(string? returnUrl, string baseUri) => + LocalReturnPath.RootAgainstBase(returnUrl, baseUri); + private async Task ValidateCredentials(string? username, string? password) { if (string.IsNullOrEmpty(username) || string.IsNullOrEmpty(password)) diff --git a/src/studio/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs b/src/studio/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs index 6891b08cf3..8135903488 100644 --- a/src/studio/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs +++ b/src/studio/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs @@ -1,8 +1,9 @@ -using System.Net; +using System.Net; using System.Net.Http.Json; using System.Security.Cryptography; using System.Text; using System.Text.Json; +using Elsa.Studio.Authentication.Abstractions; using Elsa.Studio.Login.Contracts; using Elsa.Studio.Login.Models; using Microsoft.AspNetCore.Components; @@ -32,7 +33,7 @@ public async Task RedirectToAuthorizationServer() url += $"&code_challenge={generated.CodeChallenge}&code_challenge_method={generated.Method}"; } var state = WebEncoders.Base64UrlEncode(RandomNumberGenerator.GetBytes(32)); - var returnPath = new Uri(navigationManager.Uri).PathAndQuery; + var returnPath = LocalReturnPath.Normalize(new Uri(navigationManager.Uri).PathAndQuery); await jsRuntime.InvokeVoidAsync("sessionStorage.setItem", PendingAuthorizationKey, JsonSerializer.Serialize(new PendingAuthorization(state, returnPath, DateTimeOffset.UtcNow))); url += "&state=" + WebUtility.UrlEncode(state); @@ -91,8 +92,7 @@ public async Task ReceiveAuthorizationCode(string code, string? state, Cancellat await jwtAccessor.WriteTokenAsync(TokenNames.AccessToken, tokens.AccessToken ?? ""); await jwtAccessor.WriteTokenAsync(TokenNames.IdToken, tokens.IdToken ?? ""); - var returnPath = pending.ReturnPath; - navigationManager.NavigateTo(returnPath.StartsWith('/') && !returnPath.StartsWith("//", StringComparison.Ordinal) && !returnPath.Contains('\\') ? returnPath : "/", true); + navigationManager.NavigateTo(LocalReturnPath.Normalize(pending.ReturnPath), true); } private static bool StateMatches(string supplied, string expected)