Skip to content

Conversation

@rolodato
Copy link
Contributor

@rolodato rolodato commented Mar 10, 2025

Instead of having code snippets in our docs that describe how to validate webhook signatures, we should ship the code directly as part of all server-side SDKs.

There's a potential concern on whether this code belongs as part of this SDK, or if it should belong in a different package or not at all. I believe it does, for several reasons:

  • We're not adding dependencies to this package or meaningfully increasing the package size.
  • If package size was a concern, it would mainly be for client-side applications and not server-side. If this does become a problem, we can have separate artifacts for client-side and server-side applications.
  • We're using a separate namespace from everything else.
  • There's almost no benefit (minus package size) to having to publish a new library just for this type of code.

As an example, I like Auth0's approach for this, where they publish a single dependency with different namespaces for the authentication and management components (analogous to our flags APIs and admin APIs): https://github.com/auth0/node-auth0?tab=readme-ov-file#configure-the-sdk

@github-actions github-actions bot added feature and removed feature labels Mar 10, 2025
@github-actions github-actions bot added feature and removed feature labels Mar 10, 2025
@rolodato rolodato self-assigned this Mar 11, 2025
@rolodato rolodato requested a review from matthewelwell March 11, 2025 14:52
from typing import Union


def generate_signature(
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see any reason that this function should be part of the public interface for this module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could help for generating fake requests in case you want to test webhook listeners without depending on Flagsmith, or if for whatever reason you need to invoke a webhook manually. It could also help troubleshooting if verify_signature fails and you don't understand why (wrong secret, signature or payload).

It's the same as how most JWT libraries have methods for generating and verifying signatures, but the vast majority of users will only be verifying signatures.

In any case I don't feel too strongly about this, I'm happy to remove generate_signature from the public interface and add it later if we need to. Let me know what you prefer!

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough, yep, ok, happy to leave it in.

@rolodato rolodato merged commit 3d8df11 into main Mar 25, 2025
7 checks passed
@rolodato rolodato deleted the feat/webhook-validate branch March 25, 2025 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants