From 1933268aad52268b14a863ff60d5312fe1187a08 Mon Sep 17 00:00:00 2001 From: Mathieu Guindon Date: Wed, 5 Feb 2025 17:39:03 -0500 Subject: [PATCH] fix authentication --- .../Api/Admin/AdminController.cs | 4 +- .../Api/Admin/PushWebhookPayload.cs | 32 ++++++++++++++++ .../Api/Admin/WebhookController.cs | 10 +++-- .../Api/Admin/WebhookPayloadType.cs | 1 + .../Admin/WebhookPayloadValidationService.cs | 37 ++++++------------- rubberduckvba.Server/Program.cs | 15 ++++---- .../WebhookSignatureValidationService.cs | 8 +++- 7 files changed, 68 insertions(+), 39 deletions(-) create mode 100644 rubberduckvba.Server/Api/Admin/PushWebhookPayload.cs diff --git a/rubberduckvba.Server/Api/Admin/AdminController.cs b/rubberduckvba.Server/Api/Admin/AdminController.cs index 7ab5284..4993c72 100644 --- a/rubberduckvba.Server/Api/Admin/AdminController.cs +++ b/rubberduckvba.Server/Api/Admin/AdminController.cs @@ -12,7 +12,7 @@ public class AdminController(ConfigurationOptions options, HangfireLauncherServi /// Enqueues a job that updates xmldoc content from the latest release/pre-release tags. /// /// The unique identifier of the enqueued job. - [Authorize("github", AuthenticationSchemes = "github")] + [Authorize("github")] [HttpPost("admin/update/xmldoc")] public IActionResult UpdateXmldocContent() { @@ -24,7 +24,7 @@ public IActionResult UpdateXmldocContent() /// Enqueues a job that gets the latest release/pre-release tags and their respective assets, and updates the installer download stats. /// /// The unique identifier of the enqueued job. - [Authorize("github", AuthenticationSchemes = "github")] + [Authorize("github")] [HttpPost("admin/update/tags")] public IActionResult UpdateTagMetadata() { diff --git a/rubberduckvba.Server/Api/Admin/PushWebhookPayload.cs b/rubberduckvba.Server/Api/Admin/PushWebhookPayload.cs new file mode 100644 index 0000000..154f6a1 --- /dev/null +++ b/rubberduckvba.Server/Api/Admin/PushWebhookPayload.cs @@ -0,0 +1,32 @@ +namespace rubberduckvba.Server.Api.Admin; + +public record class PushWebhookPayload +{ + public string Ref { get; set; } + + public bool Created { get; set; } + public bool Deleted { get; set; } + public bool Forced { get; set; } + + public WebhookPayloadSender Sender { get; set; } + public WebhookPayloadRepository Repository { get; set; } + + public record class WebhookPayloadSender + { + public string Login { get; set; } + } + + public record class WebhookPayloadRepository + { + public int Id { get; set; } + public string Name { get; set; } + public WebhookPayloadRepositoryOwner Owner { get; set; } + + public record class WebhookPayloadRepositoryOwner + { + public int Id { get; set; } + public string Login { get; set; } + public string Type { get; set; } + } + } +} diff --git a/rubberduckvba.Server/Api/Admin/WebhookController.cs b/rubberduckvba.Server/Api/Admin/WebhookController.cs index eae1b6a..a1bb468 100644 --- a/rubberduckvba.Server/Api/Admin/WebhookController.cs +++ b/rubberduckvba.Server/Api/Admin/WebhookController.cs @@ -1,6 +1,5 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; -using Newtonsoft.Json.Linq; namespace rubberduckvba.Server.Api.Admin; @@ -20,9 +19,9 @@ public WebhookController( _hangfire = hangfire; } - [Authorize("webhook", AuthenticationSchemes = "webhook-signature")] + [Authorize("webhook")] [HttpPost("webhook/github")] - public IActionResult GitHub([FromBody] JToken payload) + public IActionResult GitHub([FromBody] PushWebhookPayload payload) { var eventType = _validator.Validate(payload, Request.Headers, out var content, out var gitref); @@ -34,6 +33,11 @@ public IActionResult GitHub([FromBody] JToken payload) Logger.LogInformation(message); return Ok(message); } + else if (eventType == WebhookPayloadType.Ping) + { + Logger.LogInformation("Webhook ping event was accepted; nothing to process."); + return Ok(); + } else if (eventType == WebhookPayloadType.Greeting) { Logger.LogInformation("Webhook push event was accepted; nothing to process. {content}", content); diff --git a/rubberduckvba.Server/Api/Admin/WebhookPayloadType.cs b/rubberduckvba.Server/Api/Admin/WebhookPayloadType.cs index 6b71cb7..741da60 100644 --- a/rubberduckvba.Server/Api/Admin/WebhookPayloadType.cs +++ b/rubberduckvba.Server/Api/Admin/WebhookPayloadType.cs @@ -4,5 +4,6 @@ public enum WebhookPayloadType { Unsupported, Greeting, + Ping, Push } diff --git a/rubberduckvba.Server/Api/Admin/WebhookPayloadValidationService.cs b/rubberduckvba.Server/Api/Admin/WebhookPayloadValidationService.cs index 38526a7..256433d 100644 --- a/rubberduckvba.Server/Api/Admin/WebhookPayloadValidationService.cs +++ b/rubberduckvba.Server/Api/Admin/WebhookPayloadValidationService.cs @@ -1,39 +1,26 @@ -using Newtonsoft.Json.Linq; - -namespace rubberduckvba.Server.Api.Admin; +namespace rubberduckvba.Server.Api.Admin; public class WebhookPayloadValidationService(ConfigurationOptions options) { - public WebhookPayloadType Validate(JToken payload, IHeaderDictionary headers, out string? content, out GitRef? gitref) + public WebhookPayloadType Validate(PushWebhookPayload payload, IHeaderDictionary headers, out string? content, out GitRef? gitref) { content = default; - gitref = default; - if (!IsValidHeaders(headers) || !IsValidSource(payload) || !IsValidEvent(payload)) + gitref = new GitRef(payload.Ref); + if (headers["X-GitHub-Event"].FirstOrDefault() == "ping") { - return WebhookPayloadType.Unsupported; + if (!(payload.Created && gitref.Value.IsTag)) + { + return WebhookPayloadType.Greeting; + } + return WebhookPayloadType.Ping; } - gitref = new GitRef(payload.Value("ref")); - if (!(payload.Value("created") && gitref.HasValue && gitref.Value.IsTag)) + if (headers["X-GitHub-Event"].FirstOrDefault() == "push") { - content = payload.Value("zen"); - return WebhookPayloadType.Greeting; + return WebhookPayloadType.Push; } - return WebhookPayloadType.Push; - } - - private bool IsValidHeaders(IHeaderDictionary headers) => - headers.TryGetValue("X-GitHub-Event", out Microsoft.Extensions.Primitives.StringValues values) && values.Contains("push"); - - private bool IsValidSource(JToken payload) => - payload["repository"].Value("name") == options.GitHubOptions.Value.Rubberduck && - payload["owner"].Value("id") == options.GitHubOptions.Value.RubberduckOrgId; - - private bool IsValidEvent(JToken payload) - { - var ev = payload["hook"]?["events"]?.Values() ?? []; - return ev.Contains("push"); + return WebhookPayloadType.Unsupported; } } diff --git a/rubberduckvba.Server/Program.cs b/rubberduckvba.Server/Program.cs index 1b9c9be..abd4e74 100644 --- a/rubberduckvba.Server/Program.cs +++ b/rubberduckvba.Server/Program.cs @@ -21,7 +21,6 @@ using rubberduckvba.Server.Services.rubberduckdb; using System.Diagnostics; using System.Reflection; -using System.Security.Claims; namespace rubberduckvba.Server; @@ -43,12 +42,12 @@ public static void Main(string[] args) builder.Services.Configure(options => builder.Configuration.GetSection("Api").Bind(options)); builder.Services.Configure(options => builder.Configuration.GetSection("Hangfire").Bind(options)); - builder.Services.AddAuthentication(options => { options.RequireAuthenticatedSignIn = false; + options.DefaultAuthenticateScheme = "github"; - options.DefaultChallengeScheme = "github"; + options.DefaultScheme = "anonymous"; options.AddScheme("github", builder => { @@ -59,18 +58,16 @@ public static void Main(string[] args) builder.HandlerType = typeof(WebhookAuthenticationHandler); }); }); + builder.Services.AddAuthorization(options => { options.AddPolicy("github", builder => { - builder.RequireAuthenticatedUser(); + builder.RequireAuthenticatedUser().AddAuthenticationSchemes("github"); }); options.AddPolicy("webhook", builder => { - builder.RequireAuthenticatedUser() - .RequireClaim(ClaimTypes.Authentication, "webhook-signature") - .RequireClaim(ClaimTypes.Role, "rubberduck-webhook") - .RequireClaim(ClaimTypes.Name, "rubberduck-vba-releasebot"); + builder.RequireAuthenticatedUser().AddAuthenticationSchemes("webhook-signature"); }); }); @@ -164,6 +161,8 @@ private static void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/rubberduckvba.Server/WebhookSignatureValidationService.cs b/rubberduckvba.Server/WebhookSignatureValidationService.cs index 5047df9..c6f94e5 100644 --- a/rubberduckvba.Server/WebhookSignatureValidationService.cs +++ b/rubberduckvba.Server/WebhookSignatureValidationService.cs @@ -24,7 +24,13 @@ string[] xHubSignature256 if (!xGitHubEvent.Contains("push")) { - // only authenticate push events + if (xGitHubEvent.Contains("ping")) + { + // no harm just returning 200-OK on ping + return true; + } + + // only authenticate ping and push events return false; }