Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
justinmchase opened this issue Apr 19, 2024 · 3 comments
Open
1 task done

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

justinmchase opened this issue Apr 19, 2024 · 3 comments
Labels
Type: Feature New feature or request

Comments

@justinmchase
Copy link

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

  • I agree to follow this project's Code of Conduct
@justinmchase justinmchase added Status: Triage This is being looked at and prioritized Type: Feature New feature or request labels Apr 19, 2024
Copy link

👋 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
Copy link
Contributor

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
Copy link
Author

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.

@kfcampbell kfcampbell removed the Status: Triage This is being looked at and prioritized label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
Status: 🔥 Backlog
Development

No branches or pull requests

3 participants