diff --git a/source/src/Slackbot.Net.Endpoints/Hosting/ServiceCollectionExtensions.cs b/source/src/Slackbot.Net.Endpoints/Hosting/ServiceCollectionExtensions.cs index 1e5754c..a8cc959 100644 --- a/source/src/Slackbot.Net.Endpoints/Hosting/ServiceCollectionExtensions.cs +++ b/source/src/Slackbot.Net.Endpoints/Hosting/ServiceCollectionExtensions.cs @@ -38,5 +38,10 @@ public class OAuthOptions { public string CLIENT_ID { get; set; } public string CLIENT_SECRET { get; set; } + /// + /// Where the user ends up after a successful install. If the install was started with an + /// OAuth state parameter, that value is appended here as ?state= so the page + /// can pick it up — this library never interprets it. + /// public string SuccessRedirectUri { get; set; } = "/success?default=1"; } diff --git a/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotCodeTokenExchangeMiddleware.cs b/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotCodeTokenExchangeMiddleware.cs index 8a8a592..f9a4333 100644 --- a/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotCodeTokenExchangeMiddleware.cs +++ b/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotCodeTokenExchangeMiddleware.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Slackbot.Net.Abstractions.Hosting; @@ -39,7 +40,8 @@ await installationHandler.Install(new Workspace response.Access_Token )); - ctx.Response.Redirect(options.Value.SuccessRedirectUri); + var stateTheAppSent = ctx.Request.Query["state"].FirstOrDefault(); + ctx.Response.Redirect(SuccessRedirect(options.Value.SuccessRedirectUri, stateTheAppSent)); } else { @@ -48,4 +50,11 @@ await installationHandler.Install(new Workspace await ctx.Response.WriteAsync(response.Error); } } + + // `state` is opaque to this library — only the app that sent it knows what it means, so it + // rides back to that app's own success page untouched rather than being interpreted here. + internal static string SuccessRedirect(string successRedirectUri, string state) => + string.IsNullOrEmpty(state) + ? successRedirectUri + : QueryHelpers.AddQueryString(successRedirectUri, "state", state); } diff --git a/source/test/Slackbot.Net.SlackClients.Http.Tests/DistributionRedirectTests.cs b/source/test/Slackbot.Net.SlackClients.Http.Tests/DistributionRedirectTests.cs new file mode 100644 index 0000000..aee8b4f --- /dev/null +++ b/source/test/Slackbot.Net.SlackClients.Http.Tests/DistributionRedirectTests.cs @@ -0,0 +1,97 @@ +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 WithoutState_TheWorkspaceIsInstalledAndTheSuccessPageUsedUntouched() + { + var (ctx, handler) = await Install(state: null, successRedirectUri: "/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("/success?default=1", ctx.Response.Headers.Location.ToString()); + } + + [Fact] + public async Task StateRidesBackToTheSuccessPageAsAQueryParameter() + { + var (ctx, _) = await Install("/admin/slack", "/success?default=1"); + + Assert.Equal("/success?default=1&state=%2Fadmin%2Fslack", ctx.Response.Headers.Location.ToString()); + } + + [Theory] + [InlineData("/admin/slack")] + [InlineData("https://evil.example/steal")] + [InlineData("//evil.example")] + [InlineData("eyJyZXR1cm5UbyI6Ii9hZG1pbiJ9")] + public async Task StateIsNeverTheRedirectTarget_WhateverItHolds(string state) + { + var (ctx, _) = await Install(state, "https://example.com/success"); + + Assert.StartsWith("https://example.com/success?state=", 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(); + services.AddSlackbotDistribution(o => + { + o.CLIENT_ID = "client-id"; + o.CLIENT_SECRET = "client-secret"; + o.SuccessRedirectUri = successRedirectUri; + }); + 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(); + return (ctx, handler); + } + + private sealed class RecordingInstallationHandler : IWorkspaceInstallationHandler + { + public List Installed { get; } = []; + + public Task Install(Workspace workspace) + { + Installed.Add(workspace); + return Task.CompletedTask; + } + + public Task Uninstall(string teamId) => Task.CompletedTask; + } +}