workos / workos-dotnet

Official .NET SDK for interacting with the WorkOS API
MIT License
9 stars 9 forks source link

Exception improvements for Webhook header verification #176

Open gcsuk opened 6 days ago

gcsuk commented 6 days ago

When calling VerifyHeader it would be good to know the difference between verification exceptions and any other possible exception. Currently, generic Exceptions are thrown, if we change this to custom exceptions they can be caught and handled specifically.

So we can do:

 try
 {
    ...
    webhookService.VerifyHeader(body, header, secret, 300);
    ....
 }
 catch (WorkOSWebhookException ex)
 {
     // Handle webhook exception
 }
catch (Exception ex)
{
    // Other exception handling
}

This would be a non-breaking change as the custom exception would inherit Exception, so any existing consumer handling these will still catch them.

gcsuk commented 6 days ago

I have this on a branch locally (no permission to push). The change is;

New file Webhooks/Exceptions/WorkOSWebhookException.cs

namespace WorkOS
{
    using System;

    public class WorkOSWebhookException : Exception
    {
        public WorkOSWebhookException(string message)
            : base(message)
        {
        }
    }
}

Updates to WebhookService.cs

if (!this.VerifyTimeTolerance(timeStamp, tolerance))
{
    throw new WorkOSWebhookException("Timestamp outside of the tolerance zone");
}

........

if (!this.SecureCompare(expectedSig, signatureHash))
{
    throw new WorkOSWebhookException("Signature hash does not match the expected signature hash for payload");
}
PaulAsjes commented 5 days ago

Thanks for this! You should be able to open up a PR by first forking this repo, pushing up your changes to your fork and then creating a PR here.