fix(auth): prevent open redirect on OIDC login returnUrl (port to main)

src/studio/modules/Elsa.Studio.Authentication.Abstractions/LocalReturnPath.cs

@@ -0,0 +1,60 @@
+namespace Elsa.Studio.Authentication.Abstractions;
+
+/// <summary>
+/// Neutralizes open-redirect targets so post-authentication navigation stays inside the Studio host.
+/// </summary>
+public static class LocalReturnPath
+{
+    private const int MaxDecodePasses = 8;
+
+    /// <summary>
+    /// Returns the original candidate when it is a rooted local path, or <c>/</c> when it is missing, relative, absolute, protocol-relative, or otherwise unsafe.
+    /// </summary>
+    /// <remarks>
+    /// 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 <c>/</c> (not <c>//</c> or <c>/\</c>) is accepted. Decoding that does not reach a fixed point within <see cref="MaxDecodePasses"/> fails closed to <c>/</c>.
+    /// </remarks>
+    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 "/";
+    }
+
+    /// <summary>
+    /// Roots a base-relative candidate against <paramref name="baseUri"/>'s path, then normalizes.
+    /// </summary>
+    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));
+}

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);
     }
 }

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);
     }
 
     /// <summary>
@@ -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;
     }
 }

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);
 }

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

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;
 
 /// <summary>Validates targets which remain inside the registered Studio client.</summary>
 public static class ExternalAuthenticationReturnPath
 {
     /// <summary>Returns a safe client-local path, falling back to the Studio root.</summary>
-    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);
 }

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<LocalRedirectResult>(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<LocalRedirectResult>(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<LocalRedirectResult>(result).Url);
+    }
+
     [Fact]
     public async Task Callback_UsesConfidentialBasicExchangeAndExactState()
     {

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));
 

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<ChallengeResult>(new AuthenticationController().Login(returnUrl));
+        var result = Assert.IsType<ChallengeResult>(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<SignOutResult>(CreateController().Logout(returnUrl));
+
+        Assert.NotNull(result.Properties);
+        Assert.Equal(expectedReturnUrl, result.Properties!.RedirectUri);
+    }
+
+    [Fact]
+    public void DirectOpenIdConnectChallengeRejectsBoundControlCharacterReturnUrls()
+    {
+        var result = Assert.IsType<ChallengeResult>(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)
+        };
+    }
 }

src/studio/modules/Elsa.Studio.ExternalAuthentication.Tests/Login/LocalReturnPathCorpus.cs

@@ -0,0 +1,62 @@
+using Xunit;
+
+namespace Elsa.Studio.ExternalAuthentication.Tests.Login;
+
+/// <summary>
+/// Shared table-driven return-path cases used by LocalReturnPath, OIDC, WASM, and chooser tests.
+/// </summary>
+public static class LocalReturnPathCorpus
+{
+    public static TheoryData<string?, string> 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<string?, string> CreateCases()
+    {
+        var cases = new TheoryData<string?, string>
+        {
+            { 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;
+    }
+}