From 9ece67f9bf2d29a64bc2a8764637f597073a7986 Mon Sep 17 00:00:00 2001 From: John Korsnes Date: Wed, 6 Dec 2023 16:51:09 +0100 Subject: [PATCH] Chore: Skip auth instead of fail (#12) * Skip auth instead of Fail auth on missing slack headers * Update editorconfig (end-of-file, final newline) --- source/.editorconfig | 2 +- ...EventsAuthenticationAuthenticationHandler.cs | 17 ++++++++++++----- .../Middlewares/SlackbotEventAuthMiddleware.cs | 8 ++++---- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/source/.editorconfig b/source/.editorconfig index 750fc2f..e99c22e 100644 --- a/source/.editorconfig +++ b/source/.editorconfig @@ -16,7 +16,7 @@ tab_width = 4 # New line preferences end_of_line = crlf -insert_final_newline = false +insert_final_newline = true #### .NET Coding Conventions #### diff --git a/source/src/Slackbot.Net.Endpoints/Authentication/SlackbotEventsAuthenticationAuthenticationHandler.cs b/source/src/Slackbot.Net.Endpoints/Authentication/SlackbotEventsAuthenticationAuthenticationHandler.cs index 6aa72ad..eb798f2 100644 --- a/source/src/Slackbot.Net.Endpoints/Authentication/SlackbotEventsAuthenticationAuthenticationHandler.cs +++ b/source/src/Slackbot.Net.Endpoints/Authentication/SlackbotEventsAuthenticationAuthenticationHandler.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; namespace Slackbot.Net.Endpoints.Authentication; @@ -31,22 +32,28 @@ protected override async Task HandleAuthenticateAsync() string timestamp = headers[TimestampHeaderName].FirstOrDefault(); string signature = headers[SignatureHeaderName].FirstOrDefault(); - + var failures = new StringBuilder(); if (timestamp == null) { - return HandleRequestResult.Fail($"Missing header {TimestampHeaderName}"); + failures.Append($"Missing header {TimestampHeaderName}"); } if (signature == null) { - return HandleRequestResult.Fail($"Missing header {SignatureHeaderName}"); + failures.Append($"Missing header {TimestampHeaderName}"); + } + + if (timestamp is null || signature == null) + { + Logger.LogDebug($"Skipping handler: {failures}"); + return HandleRequestResult.SkipHandler(); } bool isNumber = long.TryParse(timestamp, out long timestampAsLong); if (!isNumber) { - return HandleRequestResult.Fail($"Invalid header. Header {TimestampHeaderName} not a number"); + return HandleRequestResult.Fail($"Invalid formatted headers. {TimestampHeaderName} is not a number. "); } Request.EnableBuffering(); @@ -59,7 +66,7 @@ protected override async Task HandleAuthenticateAsync() return HandleRequestResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), SlackbotEventsAuthenticationConstants.AuthenticationScheme)); } - return HandleRequestResult.Fail("Verification of Slack request failed."); + return HandleRequestResult.Fail("Slack request failed signature verification."); } diff --git a/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotEventAuthMiddleware.cs b/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotEventAuthMiddleware.cs index 2ad2364..8d83939 100644 --- a/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotEventAuthMiddleware.cs +++ b/source/src/Slackbot.Net.Endpoints/Middlewares/SlackbotEventAuthMiddleware.cs @@ -16,23 +16,23 @@ public SlackbotEventAuthMiddleware(RequestDelegate next) public async Task Invoke(HttpContext ctx, ILogger logger) { - bool success = false; + AuthenticateResult res; try { - var res = await ctx.AuthenticateAsync(SlackbotEventsAuthenticationConstants.AuthenticationScheme); - success = res.Succeeded; + res = await ctx.AuthenticateAsync(SlackbotEventsAuthenticationConstants.AuthenticationScheme); } catch (InvalidOperationException ioe) { throw new InvalidOperationException("Did you forget to call services.AddAuthentication().AddSlackbotEvents()?", ioe); } - if (success) + if (res.Succeeded) { await _next(ctx); } else { + logger.LogWarning($"Unauthorized callback from Slack"); ctx.Response.StatusCode = StatusCodes.Status401Unauthorized; await ctx.Response.WriteAsync("UNAUTHORIZED"); }