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

Allow multiple signatures to be defined in your configuration file #1

Closed
xantari opened this issue Apr 24, 2020 · 5 comments
Closed

Comments

@xantari
Copy link

xantari commented Apr 24, 2020

Motivation

Webhooks in Kentico Kontent can only define one secret webhook key per URL. As such we have had to make multiple end point definitions in dev / test. In addition due to the issue of no distributed caching ability we have had to make multiple webhook endpoints to identify each node of a load balanced cluster.

This necessitates multiple webhook secrets to be defined in a configuration file.

Here is an example:

image

Proposed solution

We solved this by allowing multiple webhook secrets to be defined as follows:

       public async Task InvokeAsync(HttpContext httpContext, IOptions<ProjectOptionsBase> projectOptions)
        {
            var request = httpContext.Request;
            request.EnableBuffering();

            using var reader = new StreamReader(request.Body, Encoding.UTF8, true, 1024, true);
            var content = await reader.ReadToEndAsync();
            request.Body.Seek(0, SeekOrigin.Begin);

            // Iterates through all secrets to allow us to use multiple Kentico projects content.
            var generatedSignatures = new List<string>();
            foreach (var sig in projectOptions.Value.KenticoKontentWebhookSecrets)
                generatedSignatures.Add(GenerateHash(content, sig));

            var signature = request.Headers["X-KC-Signature"].FirstOrDefault();

            if (!generatedSignatures.Contains(signature))
            {
                httpContext.Response.StatusCode = 401;
                return;
            }

            await _next(httpContext);
        }

Easy fix would be to do the same thing, and make your WebhookOptions model be an array of strings so you could define them in a config file as follows:

  "KenticoKontentWebhookSecrets": [
    ".....", // Matt's ARRT.ORG Ngrok Dev Endpoint
    ".....", // Chris ARRT.ORG NGrok Dev Endpoint
    ".....", //cdevwww2hook.arrt.org
    "....." //tdevwww2hook.arrt.org
  ],
@petrsvihlik
Copy link
Contributor

Wouldn't this better be solved by using multiple per-env configs?

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-3.1

image

@xantari
Copy link
Author

xantari commented Apr 24, 2020

Yeah, that is the other way of doing it. It's just that we would have a lot of extra configurations and each load balanced node would need their own configuration. This would also mean a configuration for each developer for their own local webhook...

@xantari
Copy link
Author

xantari commented Apr 24, 2020

Currently this is the only setting that would necessitate all these extra environment configurations in our current project,.. I'd be more inclined to keep the signature handler we have instead of using the new one to avoid all these extra environment configs.

@petrsvihlik
Copy link
Contributor

Thanks for the explanation!

I understand your arguments but it is the best practice to do it that way. That's exactly what the env-configs are meant for. If it's easier for you to do it differently then go ahead but I don't think it's a good idea to include the aforementioned functionality in this package. Therefore, I'd close the issue now.

If you disagree, feel free to reopen this issue and add more comments.

@xantari
Copy link
Author

xantari commented Apr 24, 2020

Sounds good, thanks for taking a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants