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

Web hook validation in 2.x+ not present anymore? #243

Open
sandromastronardi opened this issue Oct 18, 2022 · 4 comments
Open

Web hook validation in 2.x+ not present anymore? #243

sandromastronardi opened this issue Oct 18, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@sandromastronardi
Copy link

sandromastronardi commented Oct 18, 2022

Related to fix #145

I am trying to update my cloud Events sdk to the newest version with the fix included but I can't make the validation work anymore. Is the fix not even relevant anymore?

I used to have this:

public async Task<HttpResponseMessage> HandleSubscriptionValidationEvent(HttpRequestMessage req)
        {
            if (!await req.IsWebHookValidationRequest())
            {
                throw new NotSupportedException("The request is not a WebHook validation request!");
            }
        // Add missing headers (for azure event grid doesnt send these)
        if (!req.Headers.Contains("WebHook-Request-Rate"))
        {
            req.Headers.Add("WebHook-Request-Rate", "*");
        }
        if (req.Headers.Contains("Webhook-Request-Callback"))
        {
            req.Headers.Remove("Webhook-Request-Callback");
        }
        var response = await req.HandleAsWebHookValidationRequest((origin) =>
        {
            var setting = _settings.WebHookAllowedOrigin ?? Environment.GetEnvironmentVariable("WebHook-Request-Origin");
            if (string.IsNullOrEmpty(setting) || setting == "*")
            {
                return true;
            }
            else
            {
                return setting == origin;
            }
        }, (o) =>
        {
            return _settings.WebHookAllowedRate ?? Environment.GetEnvironmentVariable("WebHook-Request-Rate") ?? "*";
        });
        return response;

But the IsWebHookValidationRequest and HandleAsWebHookValidationRequest are gone, how do i do this now?

Sandro

@jskeet
Copy link
Contributor

jskeet commented Oct 18, 2022

Okay, I've found it - we did indeed remove all webhook code in a91d138, to avoid a later breaking change being needed due to cloudevents/spec#781. This has now been fixed by cloudevents/spec#865, which changes the header from Origin to WebHook-Request-Origin, which appears to be the header the code was already using. So I may be able to just revert that earlier commit and do another release, but I'll need to check it through very carefully - I doubt that it'll be within the next couple of weeks, I'm afraid.

Until then, I suspect you could just copy the deleted code into your own project (with appropriate attribute/licences) and use it - I doubt that it uses any internals from CloudNative.CloudEvents (although I haven't checked in detail).

@jskeet jskeet self-assigned this Oct 18, 2022
@jskeet jskeet added the enhancement New feature or request label Oct 18, 2022
@sandromastronardi
Copy link
Author

Aha, That explains it :)
I could do a workaround for now untill you have a fix, I already had an older version in a local nuget repository with the fix in the past, but wanted to get rid of that now and use the newest version... (hence I found out it wasn't really there in the first place :))
Are the 2.0 sdk's still CloudEvents 1.0 or is there other changes? I mean, azure still only supports 1.0, is it still compatible?

Sandro

@jskeet
Copy link
Contributor

jskeet commented Oct 18, 2022

Yes, the library 2.0 is still CloudEvents 1.0 - there isn't a newer major version of the CloudEvents spec. It's only a new major version because the .NET API took breaking changes (and it's now signed). If Azure only produces genuinely valid CloudEvents (and can consume genuinely valid ones) then you should be fine.

I would suggest updating to the latest library, then putting the fixed code in a separate source file in your own project, using the existing namespaces, so you can just adopt 2.7.0 (or whatever it ends up being) by updating the reference and deleting the file.

@sandromastronardi
Copy link
Author

Ok, great. It adhered the standards mostly (except for some missing headers that I add along they way when they are missing to be able to use the validation).
So I'll move it locally for now.

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

No branches or pull requests

2 participants