Redirect to the OAuth state parameter after a Slack install

source/src/Slackbot.Net.Endpoints/Hosting/IAppBuilderExtensions.cs

@@ -36,6 +36,9 @@ public static IApplicationBuilder UseSlackbot(
     ///     NB! The path you run this middleware must:
     ///     - match redirect_uri in your 1st redirect to Slack
     ///     - be a valid redirect_uri in your Slack app configuration
+    ///     On a successful install the user is sent to the OAuth `state` parameter when it holds a
+    ///     site-relative path (so an install can return to where it started), and to
+    ///     <see cref="OAuthOptions.SuccessRedirectUri" /> otherwise.
     /// </summary>
     /// <param name="app"></param>
     /// <returns></returns>

source/src/Slackbot.Net.Endpoints/Hosting/ServiceCollectionExtensions.cs

@@ -38,5 +38,11 @@ public class OAuthOptions
 {
     public string CLIENT_ID { get; set; }
     public string CLIENT_SECRET { get; set; }
+
+    /// <summary>
+    ///     Where the user ends up after a successful install. Used as-is unless the install was
+    ///     started with a site-relative path in the OAuth `state` parameter, in which case that path
+    ///     wins - resolved against this uri's origin when it is absolute.
+    /// </summary>
     public string SuccessRedirectUri { get; set; } = "/success?default=1";
 }

source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotCodeTokenExchangeMiddleware.cs

@@ -39,7 +39,8 @@ await installationHandler.Install(new Workspace
                 response.Access_Token
             ));
 
-            ctx.Response.Redirect(options.Value.SuccessRedirectUri);
+            var state = ctx.Request.Query["state"].FirstOrDefault();
+            ctx.Response.Redirect(ResolveRedirectUri(state, options.Value.SuccessRedirectUri));
         }
         else
         {
@@ -48,4 +49,46 @@ await installationHandler.Install(new Workspace
             await ctx.Response.WriteAsync(response.Error);
         }
     }
+
+    /// <summary>
+    ///     Slack round-trips the OAuth `state` parameter untouched, so an app can use it to carry
+    ///     the page the install was started from. Only site-relative paths are honored, and they are
+    ///     resolved against <see cref="OAuthOptions.SuccessRedirectUri" />'s origin so the user stays
+    ///     on the same site. Anything else falls back to <see cref="OAuthOptions.SuccessRedirectUri" />.
+    /// </summary>
+    private static string ResolveRedirectUri(string state, string successRedirectUri)
+    {
+        if (!IsLocalUrl(state))
+        {
+            return successRedirectUri;
+        }
+
+        if (Uri.TryCreate(successRedirectUri, UriKind.Absolute, out var successUri) &&
+            (successUri.Scheme == Uri.UriSchemeHttp || successUri.Scheme == Uri.UriSchemeHttps))
+        {
+            return new Uri(successUri, state).AbsoluteUri;
+        }
+
+        return state;
+    }
+
+    /// <summary>
+    ///     Accepts site-relative urls like `/foo`, but not absolute (`https://host/foo`) or
+    ///     protocol-relative (`//host`, `/\host`) ones, which would turn the callback into an open
+    ///     redirect.
+    /// </summary>
+    private static bool IsLocalUrl(string url)
+    {
+        if (string.IsNullOrEmpty(url) || url[0] != '/')
+        {
+            return false;
+        }
+
+        if (url.Length > 1 && (url[1] == '/' || url[1] == '\\'))
+        {
+            return false;
+        }
+
+        return !url.Any(char.IsControl);
+    }
 }

source/test/Slackbot.Net.SlackClients.Http.Tests/DistributionRedirectTests.cs

@@ -0,0 +1,103 @@
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.DependencyInjection;
+using Slackbot.Net.Abstractions.Hosting;
+using Slackbot.Net.Endpoints.Hosting;
+using Slackbot.Net.Tests.Helpers;
+
+namespace Slackbot.Net.Tests;
+
+public class DistributionRedirectTests
+{
+    private const string OauthAccessResponse =
+        """{"ok":true,"access_token":"xoxb-token","scope":"chat:write","team":{"id":"T1","name":"Team"},"app_id":"A1"}""";
+
+    [Fact]
+    public async Task InstallsWorkspaceAndRedirectsToSuccessRedirectUri()
+    {
+        var (ctx, handler) = await Install(null, "/success?default=1");
+
+        var workspace = Assert.Single(handler.Installed);
+        Assert.Equal("T1", workspace.TeamId);
+        Assert.Equal("Team", workspace.TeamName);
+        Assert.Equal("xoxb-token", workspace.Token);
+        Assert.Equal(StatusCodes.Status302Found, ctx.Response.StatusCode);
+        Assert.Equal("/success?default=1", ctx.Response.Headers.Location.ToString());
+    }
+
+    [Theory]
+    // A site-relative state keeps the origin of an absolute SuccessRedirectUri
+    [InlineData("/admin/slack?installed=1", "https://example.com/success?default=1",
+        "https://example.com/admin/slack?installed=1")]
+    [InlineData("/", "https://example.com/success", "https://example.com/")]
+    // ... and is used as-is when SuccessRedirectUri is relative too
+    [InlineData("/admin/slack", "/success?default=1", "/admin/slack")]
+    // Anything that could leave the site is ignored
+    [InlineData("https://evil.example/steal", "https://example.com/success", "https://example.com/success")]
+    [InlineData("//evil.example", "https://example.com/success", "https://example.com/success")]
+    [InlineData("/\\evil.example", "https://example.com/success", "https://example.com/success")]
+    [InlineData("admin/slack", "https://example.com/success", "https://example.com/success")]
+    [InlineData("", "https://example.com/success", "https://example.com/success")]
+    public async Task RedirectsToStateWhenItIsSiteRelative(string state, string successRedirectUri,
+        string expectedLocation)
+    {
+        var (ctx, _) = await Install(state, successRedirectUri);
+
+        Assert.Equal(expectedLocation, ctx.Response.Headers.Location.ToString());
+    }
+
+    private static async Task<(HttpContext Context, RecordingInstallationHandler Handler)> Install(string state,
+        string successRedirectUri)
+    {
+        var services = new ServiceCollection();
+        services.AddLogging();
+        services.AddSlackBotEvents<RecordingInstallationHandler>();
+        services.AddSlackbotDistribution(o =>
+        {
+            o.CLIENT_ID = "client-id";
+            o.CLIENT_SECRET = "client-secret";
+            o.SuccessRedirectUri = successRedirectUri;
+        });
+        // Serves a canned oauth.v2.access response, so no call goes to Slack
+        services.ConfigureHttpClientDefaults(b =>
+            b.ConfigurePrimaryHttpMessageHandler(() => new StubHttpMessageHandler(OauthAccessResponse)));
+
+        await using var provider = services.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true });
+        await using var scope = provider.CreateAsyncScope();
+
+        var pipeline = new ApplicationBuilder(provider).UseSlackbotDistribution().Build();
+
+        var query = QueryString.Create("code", "the-code");
+        if (state != null)
+        {
+            query = query.Add(QueryString.Create("state", state));
+        }
+
+        var ctx = new DefaultHttpContext { RequestServices = scope.ServiceProvider };
+        ctx.Request.Scheme = "https";
+        ctx.Request.Host = new HostString("example.com");
+        ctx.Request.QueryString = query;
+
+        await pipeline(ctx);
+
+        var handler = (RecordingInstallationHandler)scope.ServiceProvider
+            .GetRequiredService<IWorkspaceInstallationHandler>();
+        return (ctx, handler);
+    }
+
+    private sealed class RecordingInstallationHandler : IWorkspaceInstallationHandler
+    {
+        public List<Workspace> Installed { get; } = [];
+
+        public Task Install(Workspace workspace)
+        {
+            Installed.Add(workspace);
+            return Task.CompletedTask;
+        }
+
+        public Task Uninstall(string teamId)
+        {
+            return Task.CompletedTask;
+        }
+    }
+}