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

Add webhook signature and timestamp validation methods #113

Merged

Conversation

blairlunceford
Copy link
Contributor

@blairlunceford blairlunceford commented Jul 13, 2021

First SDK pass at adding webhook signature and timestamp validation methods.

I broke up the two methods to give users options, but we could also combine them into one validation method.

I assume that the application will parse the request for the raw POST body and the signature like:

sig_header = request.headers['WorkOS-Signature']
payload = request.body.read

And then enter the parsed information as parameters to the methods, along with the Webhook Secret.

@blairlunceford blairlunceford requested a review from a team as a code owner July 13, 2021 15:55
@linear
Copy link

linear bot commented Jul 13, 2021

@blairlunceford blairlunceford marked this pull request as draft July 28, 2021 20:52
@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2021

Codecov Report

Merging #113 (e76dcf3) into master (4df26ca) will increase coverage by 0.04%.
The diff coverage is 98.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #113      +/-   ##
==========================================
+ Coverage   98.26%   98.30%   +0.04%     
==========================================
  Files          28       31       +3     
  Lines         575      650      +75     
==========================================
+ Hits          565      639      +74     
- Misses         10       11       +1     
Impacted Files Coverage Δ
lib/workos/webhook.rb 94.11% <94.11%> (ø)
lib/workos.rb 100.00% <100.00%> (ø)
lib/workos/errors.rb 96.29% <100.00%> (+0.14%) ⬆️
lib/workos/types.rb 100.00% <100.00%> (ø)
lib/workos/types/webhook_struct.rb 100.00% <100.00%> (ø)
lib/workos/webhooks.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4df26ca...e76dcf3. Read the comment docs.

@blairlunceford blairlunceford marked this pull request as ready for review September 1, 2021 18:37
lib/workos/webhooks.rb Outdated Show resolved Hide resolved
lib/workos/webhooks.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@mthadley mthadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this looks awesome!

Really happy to see that we'll implement this verification process for our users. I think it'll be great to have these security best-practices built-in to our SDK's.

Only thing that I think we need to change is to use a constant time algorithm when comparing the two digests. See the comment I left there for suggestions.

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

Successfully merging this pull request may close these issues.

3 participants