Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 51 additions & 37 deletions src/Octokit.Webhooks.AspNetCore/GitHubWebhookExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 +12,63 @@
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

/// <summary>
/// A class containing extension methods for <see cref="IEndpointRouteBuilder"/>
/// for adding an HTTP endpoint for processing GitHub webhook payloads.
/// </summary>
public static partial class GitHubWebhookExtensions
{
public static IEndpointConventionBuilder MapGitHubWebhooks(this IEndpointRouteBuilder endpoints, string path = "/api/github/webhooks", string secret = null!) =>
endpoints.MapPost(path, async context =>
{
var logger = context.RequestServices.GetRequiredService<ILogger<WebhookEventProcessor>>();

// Verify content type
if (!VerifyContentType(context, MediaTypeNames.Application.Json))
{
Log.IncorrectContentType(logger);
return;
}

// Get body
var body = await GetBodyAsync(context).ConfigureAwait(false);

// Verify signature
if (!await VerifySignatureAsync(context, secret, body).ConfigureAwait(false))
{
Log.SignatureValidationFailed(logger);
return;
}

// Process body
try
{
var service = context.RequestServices.GetRequiredService<WebhookEventProcessor>();
await service.ProcessWebhookAsync(context.Request.Headers, body)
.ConfigureAwait(false);
context.Response.StatusCode = 200;
}
catch (Exception ex)
public static IEndpointConventionBuilder MapGitHubWebhooks(
this IEndpointRouteBuilder endpoints,
string path = "/api/github/webhooks",
string? secret = null)
{
var options = endpoints.ServiceProvider.GetService<IOptionsMonitor<GitHubWebhookOptions>>();
return endpoints.MapPost(
path,
async context =>
{
Log.ProcessingFailed(logger, ex);
context.Response.StatusCode = 500;
}
});
var logger = context.RequestServices.GetRequiredService<ILogger<WebhookEventProcessor>>();

// Verify content type
if (!VerifyContentType(context, MediaTypeNames.Application.Json))
{
Log.IncorrectContentType(logger);
return;
}

// Get body
var body = await GetBodyAsync(context).ConfigureAwait(false);

if (secret is null && options is not null)
{
secret = options.CurrentValue.Secret;
}

// Verify signature
if (!await VerifySignatureAsync(context, secret, body).ConfigureAwait(false))
{
Log.SignatureValidationFailed(logger);
return;
}

// Process body
try
{
var service = context.RequestServices.GetRequiredService<WebhookEventProcessor>();
await service.ProcessWebhookAsync(context.Request.Headers, body)
.ConfigureAwait(false);
context.Response.StatusCode = 200;
}
catch (Exception ex)
{
Log.ProcessingFailed(logger, ex);
context.Response.StatusCode = 500;
}
});
}

private static bool VerifyContentType(HttpContext context, string expectedContentType)
{
Expand All @@ -79,7 +93,7 @@ private static async Task<string> GetBodyAsync(HttpContext context)
return await reader.ReadToEndAsync().ConfigureAwait(false);
}

private static async Task<bool> VerifySignatureAsync(HttpContext context, string secret, string body)
private static async Task<bool> VerifySignatureAsync(HttpContext context, string? secret, string body)
{
_ = context.Request.Headers.TryGetValue("X-Hub-Signature-256", out var signatureSha256);

Expand All @@ -106,7 +120,7 @@ await context.Response.WriteAsync("Payload includes a secret, so the webhook rec
return false;
}

var keyBytes = Encoding.UTF8.GetBytes(secret);
var keyBytes = Encoding.UTF8.GetBytes(secret!);
var bodyBytes = Encoding.UTF8.GetBytes(body);

var hash = HMACSHA256.HashData(keyBytes, bodyBytes);
Expand Down
6 changes: 6 additions & 0 deletions src/Octokit.Webhooks.AspNetCore/GitHubWebhookOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace Octokit.Webhooks.AspNetCore;

public sealed record GitHubWebhookOptions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be sealed? I can imagine a use case (at least personally) where I would like to be able to also lump unrelated configuration to webhooks (like my client secret or app id) and not have to create a separate class for webhooks and GitHub app configurations.

Copy link
Member Author

@JamieMagee JamieMagee Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sealed it mostly because I seal all of the classes in this library. The performance benefits of the webhook object classes is probably more important than this object. Webhook objects will be constructed on each event, whereas this will be constructed once, on startup.

I can send a follow-up PR to unseal it. I'm still waiting on #568 to be merged before I cut a release.

{
public string? Secret { get; set; }
}