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(core): sign hook payload data #3854

Merged
merged 10 commits into from
May 23, 2023

Conversation

xiaoyijun
Copy link
Contributor

@xiaoyijun xiaoyijun commented May 17, 2023

Summary

This PR contains the following updates:

  • Add a sign util to sign payload data
  • Sign hook payload if the hook's signing key is available

Testing

UT & IT & Test locally.

Test with the go sample and get the correct signature

image

Checklist

  • .changeset
  • unit tests
  • integration tests
  • docs

OR

  • This PR is not applicable for the checklist

@xiaoyijun xiaoyijun requested a review from a team May 17, 2023 03:59
@xiaoyijun xiaoyijun self-assigned this May 17, 2023
@linear
Copy link

linear bot commented May 17, 2023

LOG-6073 Core: Signing Hooks Payload Data

https://www.notion.so/silverhand/Generate-Webhook-Signature-41e0f4989d2944aebab78d640d1f2edb

  • 我们目前允许用户自定义 Request Headers, 如果自定义的 Key 和 我们用的 x-logto-signature-256 一样,要怎么处理

@github-actions
Copy link

github-actions bot commented May 17, 2023

COMPARE TO master

Total Size Diff 📈 +3.85 KB

Diff by File
Name Diff
packages/core/src/libraries/hook.test.ts 📈 +196 Bytes
packages/core/src/libraries/hook.ts 📈 +196 Bytes
packages/core/src/utils/sign.test.ts 📈 +735 Bytes
packages/core/src/utils/sign.ts 📈 +283 Bytes
packages/integration-tests/src/helpers/index.ts 📈 +141 Bytes
packages/integration-tests/src/tests/api/hooks.test.ts 📈 +2.33 KB

@github-actions github-actions bot added feature Cool stuff size/m labels May 17, 2023
@xiaoyijun xiaoyijun marked this pull request as draft May 17, 2023 08:31
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-5959-core-update-hook-api branch 2 times, most recently from defc9fd to c28ff47 Compare May 18, 2023 02:41
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-6073-core-signing-hooks-payload-data branch 2 times, most recently from 5ffa9c9 to aec874a Compare May 18, 2023 03:46
@xiaoyijun xiaoyijun marked this pull request as ready for review May 18, 2023 06:24
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-5959-core-update-hook-api branch from e0bf3c9 to d55a154 Compare May 19, 2023 07:48
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-6073-core-signing-hooks-payload-data branch from aec874a to 278be45 Compare May 19, 2023 08:22
Base automatically changed from xiaoyijun-log-5959-core-update-hook-api to master May 19, 2023 08:48
@github-actions github-actions bot added size/xl and removed size/m labels May 19, 2023
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-6073-core-signing-hooks-payload-data branch from 278be45 to 5a38367 Compare May 19, 2023 09:04
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-6073-core-signing-hooks-payload-data branch from 18c378f to 608ce0c Compare May 22, 2023 03:24
@github-actions github-actions bot added size/s and removed size/l labels May 22, 2023
Copy link
Contributor

@simeng-li simeng-li left a comment

Choose a reason for hiding this comment

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

P.S. should we update the integration test to check the header as well?

@xiaoyijun
Copy link
Contributor Author

xiaoyijun commented May 22, 2023

P.S. should we update the integration test to check the header as well?

A test for webhook signature is added.
@simeng-li

packages/core/src/libraries/hook.ts Outdated Show resolved Hide resolved
packages/core/src/libraries/hook.ts Outdated Show resolved Hide resolved
packages/core/src/utils/signature.ts Outdated Show resolved Hide resolved
@xiaoyijun xiaoyijun force-pushed the xiaoyijun-log-6073-core-signing-hooks-payload-data branch from 3d8f6b6 to e80604c Compare May 22, 2023 11:09
Copy link
Contributor

@simeng-li simeng-li left a comment

Choose a reason for hiding this comment

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

overall LGTM

@github-actions github-actions bot added size/m and removed size/s labels May 22, 2023
@xiaoyijun xiaoyijun merged commit 0c79437 into master May 23, 2023
@xiaoyijun xiaoyijun deleted the xiaoyijun-log-6073-core-signing-hooks-payload-data branch May 23, 2023 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants