octokit / webhooks.net

GitHub webhook events toolset for .NET
MIT License
51 stars 32 forks source link

[FEAT]: Please support fetching the secret asynchronously #486

Closed justinmchase closed 1 week ago

justinmchase commented 5 months ago

Describe the need

I need to fetch the secret from Key Vault and it can change over the lifecycle of the container.

The code you have to verify the signature is to fetch the secret at configure time and not asynchronous...

public static IEndpointConventionBuilder MapGitHubWebhooks(
  this IEndpointRouteBuilder endpoints,
  string path = "/api/github/webhooks",
  string secret = null!
)

At startup we are initializing it like this...

public static WebApplication UseGitHubWebhooks(this WebApplication app)
{
  var githubOptions = app.Services.GetRequiredService<IOptions<GitHubOptions>>().Value;
  var secretClient = app.Services.GetRequiredService<ISecretClient>();
  var secret = secretClient.GetSecret(githubOptions.WebhookSecretKey).ConfigureAwait(false).GetAwaiter().GetResult();
  app.MapGitHubWebhooks($"{ApplicationName.BaseUrl}/v1{githubOptions.WebhookPath}", secret.Value);
  return app;
}

This works but there are a couple of problems with this:

  1. Having to add an async call at this stage of the application startup isn't great.
  2. Theoretically the secret can change in key vault but I'll never be able to update it here without restarting my pods, there is no dynamic fetching of the secret going on.

Now there is a couple of ways that you could solve this but in my opinion simply adding some kind of IWebhookSecret interface which has an async method public Task<string> GetSecret() would work great.

That way I could inject an instance of an object that knows how to reach out to my ISecretClient and dynamically fetch before validating.

var secret = await secretClient.GetSecret();
if (!await VerifySignatureAsync(context, secret, body).ConfigureAwait(false))
{
  Log.SignatureValidationFailed(logger);
  return;
}

SDK Version

Octokit.net 11.0.1

API Version

N/A

Relevant log output

Not relevant to the API

Code of Conduct

github-actions[bot] commented 5 months ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

JamieMagee commented 5 months ago

Thanks for the issue. This sounds like a similar issue to #261 and #262. Could you take a look and let me know what you think?

justinmchase commented 5 months ago

Yes I think #262 would solve my ask exactly. Now I was imagining it was a service rather than just a function but an async function will work too.