Merge pull request #49 from rubberduck-vba/webhook

fix authentication

Author: retailcoder

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.
     /// </summary>
     /// <returns>The unique identifier of the enqueued job.</returns>
-    [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.
     /// </summary>
     /// <returns>The unique identifier of the enqueued job.</returns>
-    [Authorize("github", AuthenticationSchemes = "github")]
+    [Authorize("github")]
     [HttpPost("admin/update/tags")]
     public IActionResult UpdateTagMetadata()
     {

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

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

rubberduckvba.Server/Api/Admin/WebhookPayloadType.cs

@@ -4,5 +4,6 @@ public enum WebhookPayloadType
 {
     Unsupported,
     Greeting,
+    Ping,
     Push
 }

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<string>("ref"));
-        if (!(payload.Value<bool>("created") && gitref.HasValue && gitref.Value.IsTag))
+        if (headers["X-GitHub-Event"].FirstOrDefault() == "push")
         {
-            content = payload.Value<string>("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<string>("name") == options.GitHubOptions.Value.Rubberduck &&
-        payload["owner"].Value<int>("id") == options.GitHubOptions.Value.RubberduckOrgId;
-
-    private bool IsValidEvent(JToken payload)
-    {
-        var ev = payload["hook"]?["events"]?.Values<string>() ?? [];
-        return ev.Contains("push");
+        return WebhookPayloadType.Unsupported;
     }
 }

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<ApiSettings>(options => builder.Configuration.GetSection("Api").Bind(options));
         builder.Services.Configure<HangfireSettings>(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<IMarkdownFormattingService, MarkdownFormattingService>();
         services.AddSingleton<ISyntaxHighlighterService, SyntaxHighlighterService>();
         services.AddSingleton<WebhookSignatureValidationService>();
+        services.AddSingleton<WebhookPayloadValidationService>();
+        services.AddSingleton<HangfireLauncherService>();
 
         services.AddSingleton<IRubberduckDbService, RubberduckDbService>();
         services.AddSingleton<TagServices>();

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