diff --git a/cmd/e2e/standard_mode_test.go b/cmd/e2e/standard_mode_test.go new file mode 100644 index 000000000..76c9a99ec --- /dev/null +++ b/cmd/e2e/standard_mode_test.go @@ -0,0 +1,122 @@ +package e2e_test + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "net/http" + "strconv" + "strings" + "testing" + + "github.com/hookdeck/outpost/cmd/e2e/configs" + "github.com/hookdeck/outpost/internal/config" + "github.com/hookdeck/outpost/internal/idgen" + "github.com/hookdeck/outpost/internal/util/testinfra" + standardwebhooks "github.com/standard-webhooks/standard-webhooks/libraries/go" + "github.com/stretchr/testify/suite" +) + +// TestE2E_StandardMode boots Outpost in "standard" webhook mode with a compat +// signature in Outpost's default format, then checks each delivery from the +// receiver's side: the primary headers verify with the official Standard +// Webhooks SDK and the compat header with the "Verify Webhook Signatures" +// guide's algorithm. +func TestE2E_StandardMode(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping e2e test") + } + suite.Run(t, &standardModeSuite{}) +} + +type standardModeSuite struct { + suite.Suite + base *basicSuite + receiver *captureServer +} + +func (s *standardModeSuite) SetupSuite() { + s.receiver = newCaptureServer() + + s.base = &basicSuite{ + logStorageType: configs.LogStorageTypeClickHouse, + redisConfig: testinfra.NewDragonflyStackConfig(s.T()), + configure: func(cfg *config.Config) { + cfg.Destinations.Webhook.Mode = "standard" + // Header name alone: the rest defaults to Outpost's default scheme. + cfg.Destinations.Webhook.Compat.SignatureHeaderName = "x-outpost-signature" + }, + } + s.base.SetT(s.T()) + s.base.SetupSuite() +} + +func (s *standardModeSuite) SetupTest() { + s.base.SetT(s.T()) +} + +func (s *standardModeSuite) TearDownSuite() { + s.receiver.Close() + s.base.TearDownSuite() +} + +func (s *standardModeSuite) TestSDKVerifiesPrimaryAndCompatVerifiesDefault() { + tenant := s.base.createTenant() + secret := newStandardWebhooksSecret() + dest := s.createDestination(tenant.ID, map[string]any{"secret": secret}) + + event := s.base.publish(tenant.ID, "user.created", map[string]any{"hello": "standard"}) + req := s.receiver.wait(s.T(), dest) + + s.assertStandardWebhooks(secret, req) + s.Equal(event.ID, req.Header.Get("webhook-id")) + _, err := strconv.ParseInt(req.Header.Get("webhook-timestamp"), 10, 64) + s.NoError(err, "webhook-timestamp %q should be unix seconds", req.Header.Get("webhook-timestamp")) + s.Equal("user.created", req.Header.Get("webhook-topic")) + + // Compat with raw secret encoding: the stored string is the HMAC key. + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write(req.Body) + s.Equal("v0="+hex.EncodeToString(mac.Sum(nil)), req.Header.Get("x-outpost-signature")) +} + +func (s *standardModeSuite) TestGeneratedSecretIsStandardFormat() { + tenant := s.base.createTenant() + dest := s.createDestination(tenant.ID, nil) + + var got struct { + Credentials map[string]string `json:"credentials"` + } + status := s.base.doJSON(http.MethodGet, s.base.apiURL("/tenants/"+tenant.ID+"/destinations/"+dest), nil, &got) + s.Require().Equal(http.StatusOK, status) + secret := got.Credentials["secret"] + s.Require().True(strings.HasPrefix(secret, "whsec_"), "generated secret %q", secret) + + s.base.publish(tenant.ID, "user.created", map[string]any{"hello": "generated"}) + s.assertStandardWebhooks(secret, s.receiver.wait(s.T(), dest)) +} + +func (s *standardModeSuite) assertStandardWebhooks(secret string, req capturedRequest) { + s.T().Helper() + wh, err := standardwebhooks.NewWebhook(secret) + s.Require().NoError(err) + s.NoError(wh.Verify(req.Body, req.Header), "Standard Webhooks SDK should verify the primary signature") +} + +func (s *standardModeSuite) createDestination(tenantID string, credentials map[string]any) string { + s.T().Helper() + id := idgen.Destination() + body := map[string]any{ + "id": id, + "type": "webhook", + "topics": []string{"*"}, + "config": map[string]any{"url": s.receiver.url(id)}, + } + if credentials != nil { + body["credentials"] = credentials + } + status := s.base.doJSON(http.MethodPost, s.base.apiURL("/tenants/"+tenantID+"/destinations"), body, nil) + s.Require().Equal(http.StatusCreated, status, "failed to create destination %s", id) + return id +} diff --git a/docs/content/destinations/webhook.mdoc b/docs/content/destinations/webhook.mdoc index 09c974813..9a47ed1f6 100644 --- a/docs/content/destinations/webhook.mdoc +++ b/docs/content/destinations/webhook.mdoc @@ -298,7 +298,7 @@ Set `DESTINATIONS_WEBHOOK_PROXY_URL` to your proxy URL (basic auth supported). S {% tabs tabGroup="deployment" %} {% tab label="Managed" %} -Configure webhook operator behavior using these keys in the Config API or in [Hookdeck Destinations settings](https://dashboard.hookdeck.com/settings/project/destinations): `DESTINATIONS_WEBHOOK_HEADER_PREFIX`, `DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME`, `DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME`, `DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME`, `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME`, `DESTINATIONS_WEBHOOK_MODE`, `DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM`, `DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING`, `DESTINATIONS_WEBHOOK_SIGNATURE_CONTENT_TEMPLATE`, `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_TEMPLATE`, and `DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES`. +Configure webhook operator behavior using these keys in the Config API or in [Hookdeck Destinations settings](https://dashboard.hookdeck.com/settings/project/destinations): `DESTINATIONS_WEBHOOK_HEADER_PREFIX`, `DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME`, `DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME`, `DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME`, `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME`, `DESTINATIONS_WEBHOOK_TIMESTAMP_FORMAT`, `DESTINATIONS_WEBHOOK_MODE`, `DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM`, `DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING`, `DESTINATIONS_WEBHOOK_SIGNATURE_CONTENT_TEMPLATE`, `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_TEMPLATE`, and `DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES`. {% /tab %} {% tab label="Self-Hosted" %} ### Header Settings @@ -306,10 +306,11 @@ Configure webhook operator behavior using these keys in the Config API or in [Ho | Variable | Default | Description | |----------|---------|-------------| | `DESTINATIONS_WEBHOOK_HEADER_PREFIX` | `x-outpost-` / `webhook-` | Prefix for system webhook headers (event id, topic, timestamp, signature). Unless overridden, defaults to **`x-outpost-`** when `DESTINATIONS_WEBHOOK_MODE` is `default` and **`webhook-`** when `standard`. | -| `DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME` | — | Complete name of the event ID header. Unset uses the default `event-id`; an explicit value pins that exact name; an empty string disables the header. | -| `DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME` | — | Complete name of the timestamp header. Unset uses the default `timestamp`; an explicit value pins that exact name; an empty string disables the header. | +| `DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME` | — | Complete name of the event ID header. Unset uses the default `event-id`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME` | — | Complete name of the timestamp header. Unset uses the default `timestamp`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | | `DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME` | — | Complete name of the topic header. Unset uses the default `topic`; an explicit value pins that exact name; an empty string disables the header. | -| `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME` | — | Complete name of the signature header. Unset uses the default `signature`; an explicit value pins that exact name; an empty string disables the header. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME` | — | Complete name of the signature header. Unset uses the default `signature`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_TIMESTAMP_FORMAT` | `rfc3339` / `unix` | Format of the timestamp header: `rfc3339` or `unix` (seconds since the epoch). Only applies to `default` mode. | {% callout type="warning" %} The `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_*_HEADER` flags are deprecated and will be removed in a future version. Disable a header by setting its corresponding `*_HEADER_NAME` variable to an empty string instead. @@ -324,12 +325,12 @@ Header names are matched case-insensitively. Outpost rejects a configuration whe | Variable | Default | Description | |----------|---------|-------------| | `DESTINATIONS_WEBHOOK_MODE` | `default` | Set to `standard` for Standard Webhooks compliance | -| `DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM` | `hmac-sha256` | Signature algorithm | -| `DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING` | `hex` | Encoding: `hex` or `base64` | -| `DESTINATIONS_WEBHOOK_SIGNATURE_CONTENT_TEMPLATE` | `{{.Body}}` | Template for signed content in default mode | -| `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_TEMPLATE` | `v0={{.Signatures \| join ","}}` | Template for signature header value in default mode | +| `DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM` | `hmac-sha256` | Signature algorithm. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING` | `hex` | Encoding: `hex` or `base64`. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_CONTENT_TEMPLATE` | `{{.Body}}` | Template for signed content. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_TEMPLATE` | `v0={{.Signatures \| join ","}}` | Template for signature header value. Only applies to `default` mode. | | `DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_ENCODING` | `raw` | How the HMAC key is derived from the destination secret: `raw` uses the secret string as-is, `base64` and `hex` decode it. Only applies to `default` mode. | -| `DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_PREFIX` | — | Prefix stripped from the destination secret before decoding, e.g. `whsec_`. Ignored when the secret encoding is `raw`. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_PREFIX` | — | Prefix stripped from the destination secret before decoding, e.g. `whsec_`. Ignored when the secret encoding is `raw`. Only applies to `default` mode. | ### Compat Signature diff --git a/docs/content/guides/change-webhook-signature-scheme.mdoc b/docs/content/guides/change-webhook-signature-scheme.mdoc index 14d86345a..bf4776002 100644 --- a/docs/content/guides/change-webhook-signature-scheme.mdoc +++ b/docs/content/guides/change-webhook-signature-scheme.mdoc @@ -14,10 +14,6 @@ Use it when you are: - moving to Outpost from a webhook system you built yourself, or from another provider, and your receivers already verify that system's signature, or - changing the signature scheme of a running Outpost deployment. -{% callout type="info" %} -The compat signature applies to the `default` webhook mode. -{% /callout %} - ## How it works The compat signature takes the same options as the primary signature, under `DESTINATIONS_WEBHOOK_COMPAT_*`. It is enabled by setting `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_HEADER_NAME`. diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index 8e0cabb45..34c5dbe80 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -128,12 +128,13 @@ Choose one for event log persistence: | `DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME` | — | Complete name of the event ID header. Unset uses the default `event-id`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | | `DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME` | — | Complete name of the signature header. Unset uses the default `signature`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | | `DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME` | — | Complete name of the timestamp header. Unset uses the default `timestamp`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | -| `DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME` | — | Complete name of the topic header. Unset uses the default `topic`; an explicit value pins that exact name; an empty string disables the header. Only applies to `default` mode. | -| `DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM` | `hmac-sha256` | Signature algorithm | -| `DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING` | `hex` | Encoding: `hex` or `base64` | +| `DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME` | — | Complete name of the topic header. Unset uses the default `topic`; an explicit value pins that exact name; an empty string disables the header. | +| `DESTINATIONS_WEBHOOK_TIMESTAMP_FORMAT` | `rfc3339` / `unix` | Format of the timestamp header: `rfc3339` or `unix` (seconds since the epoch). Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM` | `hmac-sha256` | Signature algorithm. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING` | `hex` | Encoding: `hex` or `base64`. Only applies to `default` mode. | | `DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_ENCODING` | `raw` | How the HMAC key is derived from the destination secret: `raw` uses the secret string as-is, `base64` and `hex` decode it. Only applies to `default` mode. | -| `DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_PREFIX` | — | Prefix stripped from the destination secret before decoding, e.g. `whsec_`. Ignored when the secret encoding is `raw`. | -| `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_HEADER_NAME` | — | Name of the compat signature header. Setting it enables the compat signature: a second signature in your previous format, sent alongside the primary one while receivers migrate. See [Change Webhook Signature Scheme](/docs/outpost/guides/change-webhook-signature-scheme). Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_PREFIX` | — | Prefix stripped from the destination secret before decoding, e.g. `whsec_`. Ignored when the secret encoding is `raw`. Only applies to `default` mode. | +| `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_HEADER_NAME` | — | Name of the compat signature header. Setting it enables the compat signature: a second signature in your previous format, sent alongside the primary one while receivers migrate. See [Change Webhook Signature Scheme](/docs/outpost/guides/change-webhook-signature-scheme). | | `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_CONTENT_TEMPLATE` | `{{.Body}}` | Template for the content signed by the compat signature | | `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_HEADER_TEMPLATE` | `v0={{.Signatures \| join ","}}` | Template for the compat signature header value | | `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_ENCODING` | `hex` | Compat signature encoding: `hex` or `base64` | diff --git a/internal/apirouter/destination_credentials_test.go b/internal/apirouter/destination_credentials_test.go index c1a4ef516..9266a3e90 100644 --- a/internal/apirouter/destination_credentials_test.go +++ b/internal/apirouter/destination_credentials_test.go @@ -9,21 +9,32 @@ import ( "github.com/hookdeck/outpost/internal/destregistry" destregistrydefault "github.com/hookdeck/outpost/internal/destregistry/providers" + "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" "github.com/hookdeck/outpost/internal/util/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// webhookStandardRegistry creates a registry with the real webhook standard -// provider registered as "webhook". This is needed because testutil.Registry -// uses the default (non-standard) webhook provider. +// webhookStandardRegistry creates a registry with the webhook provider in +// standard mode, as config resolves it. testutil.Registry uses the default +// mode. func webhookStandardRegistry(t *testing.T) destregistry.Registry { t.Helper() logger := testutil.CreateTestLogger(t) reg := destregistry.NewRegistry(&destregistry.Config{}, logger) err := destregistrydefault.RegisterDefault(reg, destregistrydefault.RegisterDefaultDestinationOptions{ Webhook: &destregistrydefault.DestWebhookConfig{ - Mode: "standard", + MetadataName: destwebhook.StandardMetadataName, + HeaderPrefix: destwebhook.StandardHeaderPrefix, + EventIDHeader: destregistrydefault.WebhookHeaderConfig{Name: "webhook-id"}, + TimestampFormat: destwebhook.StandardTimestampFormat, + SignatureContentTemplate: destwebhook.StandardSignatureContentTmpl, + SignatureHeaderTemplate: destwebhook.StandardSignatureHeaderTmpl, + SignatureEncoding: destwebhook.StandardEncoding, + SignatureAlgorithm: destwebhook.DefaultAlgorithm, + SignatureSecretEncoding: destwebhook.StandardSecretEncoding, + SignatureSecretPrefix: destwebhook.StandardSecretPrefix, + SigningSecretTemplate: destwebhook.StandardSigningSecretTmpl, }, }) require.NoError(t, err) diff --git a/internal/config/config.go b/internal/config/config.go index 2aa5f2ad9..c1a234bef 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -119,17 +119,16 @@ type Config struct { } var ( - ErrMismatchedServiceType = errors.New("config validation error: service type mismatch") - ErrInvalidServiceType = errors.New("config validation error: invalid service type") - ErrMissingRedis = errors.New("config validation error: redis configuration is required") - ErrMissingLogStorage = errors.New("config validation error: log storage must be provided") - ErrMissingMQs = errors.New("config validation error: message queue configuration is required") - ErrMissingAESSecret = errors.New("config validation error: AES encryption secret is required") - ErrInvalidPortalProxyURL = errors.New("config validation error: invalid portal proxy url") - ErrInvalidWebhookProxyURL = errors.New("config validation error: invalid webhook proxy url") - ErrInvalidWebhookCompatSignature = errors.New("config validation error: invalid webhook compat signature") - ErrInvalidDeploymentID = errors.New("config validation error: deployment_id must contain only alphanumeric characters, hyphens, and underscores (max 64 characters)") - ErrInvalidRedisPoolSize = errors.New("config validation error: redis pool_size must be >= 0") + ErrMismatchedServiceType = errors.New("config validation error: service type mismatch") + ErrInvalidServiceType = errors.New("config validation error: invalid service type") + ErrMissingRedis = errors.New("config validation error: redis configuration is required") + ErrMissingLogStorage = errors.New("config validation error: log storage must be provided") + ErrMissingMQs = errors.New("config validation error: message queue configuration is required") + ErrMissingAESSecret = errors.New("config validation error: AES encryption secret is required") + ErrInvalidPortalProxyURL = errors.New("config validation error: invalid portal proxy url") + ErrInvalidWebhookProxyURL = errors.New("config validation error: invalid webhook proxy url") + ErrInvalidDeploymentID = errors.New("config validation error: deployment_id must contain only alphanumeric characters, hyphens, and underscores (max 64 characters)") + ErrInvalidRedisPoolSize = errors.New("config validation error: redis pool_size must be >= 0") ) func (c *Config) InitDefaults() { @@ -592,10 +591,10 @@ func resolveAlertCount(raw OptionalString, defaultValue, min int) (resolvedAlert return resolvedAlertCount{enabled: true, value: n}, nil } -// DeprecationWarnings returns human-readable warnings for deprecated config -// options that are actively in use, so callers can surface them at startup. +// DeprecationWarnings returns human-readable warnings for config options that +// are set but deprecated or ignored, so callers can surface them at startup. func (c *Config) DeprecationWarnings() []string { - return c.Destinations.Webhook.deprecationWarnings() + return append(c.Destinations.Webhook.deprecationWarnings(), c.Destinations.Webhook.standardModeWarnings()...) } // ConfigFilePath returns the path of the config file that was used diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 061b323f6..062a4eade 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config_test import ( "fmt" "os" + "strings" "testing" "github.com/hookdeck/outpost/internal/config" @@ -573,6 +574,113 @@ func TestDestinationWebhookHeaderNamesAllHeaders(t *testing.T) { assert.Equal(t, destregistrydefault.WebhookHeaderConfig{Disabled: true}, opts.Webhook.TopicHeader) } +func TestDestinationWebhookTimestampFormat(t *testing.T) { + mockOS := &mockOS{ + files: map[string][]byte{}, + envVars: map[string]string{"DESTINATIONS_WEBHOOK_TIMESTAMP_FORMAT": "unix"}, + } + + cfg, err := config.ParseWithoutValidation(config.Flags{}, mockOS) + require.NoError(t, err) + + assert.Equal(t, "unix", cfg.Destinations.ToConfig(cfg).Webhook.TimestampFormat) +} + +func TestDestinationWebhookStandardMode(t *testing.T) { + t.Run("sets the Standard Webhooks values", func(t *testing.T) { + mockOS := &mockOS{ + files: map[string][]byte{}, + envVars: map[string]string{"DESTINATIONS_WEBHOOK_MODE": "standard"}, + } + + cfg, err := config.ParseWithoutValidation(config.Flags{}, mockOS) + require.NoError(t, err) + + webhook := cfg.Destinations.ToConfig(cfg).Webhook + assert.Equal(t, "webhook_standard", webhook.MetadataName) + assert.Equal(t, "webhook-", webhook.HeaderPrefix) + assert.Equal(t, destregistrydefault.WebhookHeaderConfig{Name: "webhook-id"}, webhook.EventIDHeader) + assert.Equal(t, destregistrydefault.WebhookHeaderConfig{}, webhook.TimestampHeader) + assert.Equal(t, "unix", webhook.TimestampFormat) + assert.Equal(t, "{{.EventID}}.{{.Timestamp.Unix}}.{{.Body}}", webhook.SignatureContentTemplate) + assert.Equal(t, "v1,{{index .Signatures 0}}{{range slice .Signatures 1}} v1,{{.}}{{end}}", webhook.SignatureHeaderTemplate) + assert.Equal(t, "base64", webhook.SignatureEncoding) + assert.Equal(t, "hmac-sha256", webhook.SignatureAlgorithm) + assert.Equal(t, "base64", webhook.SignatureSecretEncoding) + assert.Equal(t, "whsec_", webhook.SignatureSecretPrefix) + assert.Equal(t, "whsec_{{.RandomBase64}}", webhook.SigningSecretTemplate) + }) + + t.Run("format options are ignored and warned about", func(t *testing.T) { + mockOS := &mockOS{ + files: map[string][]byte{}, + envVars: map[string]string{ + "DESTINATIONS_WEBHOOK_MODE": "standard", + "DESTINATIONS_WEBHOOK_HEADER_PREFIX": "x-acme-", + "DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME": "x-acme-kind", + "DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME": "x-acme-message", + "DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME": "", + "DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM": "hmac-sha1", + "DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_ENCODING": "raw", + "DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_HEADER_NAME": "x-legacy-signature", + }, + } + + cfg, err := config.ParseWithoutValidation(config.Flags{}, mockOS) + require.NoError(t, err) + + webhook := cfg.Destinations.ToConfig(cfg).Webhook + // Prefix, topic header and compat still apply. + assert.Equal(t, "x-acme-", webhook.HeaderPrefix) + assert.Equal(t, destregistrydefault.WebhookHeaderConfig{Name: "x-acme-kind"}, webhook.TopicHeader) + require.NotNil(t, webhook.Compat) + assert.Equal(t, "x-legacy-signature", webhook.Compat.SignatureHeaderName) + // The format options are fixed. + assert.Equal(t, destregistrydefault.WebhookHeaderConfig{Name: "x-acme-id"}, webhook.EventIDHeader) + assert.Equal(t, destregistrydefault.WebhookHeaderConfig{}, webhook.SignatureHeader) + assert.Equal(t, "hmac-sha256", webhook.SignatureAlgorithm) + assert.Equal(t, "base64", webhook.SignatureSecretEncoding) + + warnings := cfg.DeprecationWarnings() + assert.Len(t, warnings, 4) + for _, env := range []string{ + "DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME", + "DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME", + "DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM", + "DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_ENCODING", + } { + assert.Contains(t, strings.Join(warnings, "\n"), env+" is ignored in standard webhook mode") + } + }) + + t.Run("no warnings when only mode is set", func(t *testing.T) { + mockOS := &mockOS{ + files: map[string][]byte{}, + envVars: map[string]string{"DESTINATIONS_WEBHOOK_MODE": "standard", "DESTINATIONS_WEBHOOK_HEADER_PREFIX": "acme-"}, + } + + cfg, err := config.ParseWithoutValidation(config.Flags{}, mockOS) + require.NoError(t, err) + + assert.Empty(t, cfg.DeprecationWarnings()) + }) + + t.Run("whitespace prefix gives bare header names", func(t *testing.T) { + mockOS := &mockOS{ + files: map[string][]byte{}, + envVars: map[string]string{ + "DESTINATIONS_WEBHOOK_MODE": "standard", + "DESTINATIONS_WEBHOOK_HEADER_PREFIX": " ", + }, + } + + cfg, err := config.ParseWithoutValidation(config.Flags{}, mockOS) + require.NoError(t, err) + + assert.Equal(t, destregistrydefault.WebhookHeaderConfig{Name: "id"}, cfg.Destinations.ToConfig(cfg).Webhook.EventIDHeader) + }) +} + func TestDestinationWebhookCompatSignature(t *testing.T) { t.Run("unset leaves compat off", func(t *testing.T) { mockOS := &mockOS{files: map[string][]byte{}, envVars: map[string]string{}} diff --git a/internal/config/destinations.go b/internal/config/destinations.go index 3bc1caa6c..f40a9457a 100644 --- a/internal/config/destinations.go +++ b/internal/config/destinations.go @@ -53,17 +53,17 @@ type DestinationWebhookConfig struct { // ProxyURL may contain authentication credentials (e.g., http://user:pass@proxy:8080) // and should be treated as sensitive. // TODO: Implement sensitive value handling - https://github.com/hookdeck/outpost/issues/480 - Mode string `yaml:"mode" env:"DESTINATIONS_WEBHOOK_MODE" desc:"Webhook mode: 'default' for customizable webhooks or 'standard' for Standard Webhooks specification compliance. Defaults to 'default'." required:"N"` + Mode string `yaml:"mode" env:"DESTINATIONS_WEBHOOK_MODE" desc:"Webhook mode: 'default' or 'standard'. 'standard' uses the Standard Webhooks format and ignores the options marked default-only. Defaults to 'default'." required:"N"` ProxyURL string `yaml:"proxy_url" env:"DESTINATIONS_WEBHOOK_PROXY_URL" desc:"Forward proxy for outgoing webhook requests (HTTP or HTTPS, basic auth supported). Multiple whitespace-separated URLs are tunneled in order." required:"N"` HeaderPrefix string `yaml:"header_prefix" env:"DESTINATIONS_WEBHOOK_HEADER_PREFIX" desc:"Prefix for metadata headers added to webhook requests. Defaults to 'x-outpost-' in 'default' mode and 'webhook-' in 'standard' mode. Set to whitespace (e.g. ' ') to disable the prefix entirely." required:"N"` // Header name configs. Each is three-state: unset uses the default // '' + key, an explicit value pins that exact header name, and an - // empty string disables the header entirely. Only applies to 'default' mode. + // empty string disables the header entirely. EventIDHeaderName OptionalString `yaml:"event_id_header_name" env:"DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME" desc:"Complete name of the event ID header. Unset uses the default 'event-id'; an explicit value pins that exact name; an empty string disables the header. Only applies to 'default' mode." required:"N"` SignatureHeaderName OptionalString `yaml:"signature_header_name" env:"DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME" desc:"Complete name of the signature header. Unset uses the default 'signature'; an explicit value pins that exact name; an empty string disables the header. Only applies to 'default' mode." required:"N"` TimestampHeaderName OptionalString `yaml:"timestamp_header_name" env:"DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME" desc:"Complete name of the timestamp header. Unset uses the default 'timestamp'; an explicit value pins that exact name; an empty string disables the header. Only applies to 'default' mode." required:"N"` - TopicHeaderName OptionalString `yaml:"topic_header_name" env:"DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME" desc:"Complete name of the topic header. Unset uses the default 'topic'; an explicit value pins that exact name; an empty string disables the header. Only applies to 'default' mode." required:"N"` + TopicHeaderName OptionalString `yaml:"topic_header_name" env:"DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME" desc:"Complete name of the topic header. Unset uses the default 'topic'; an explicit value pins that exact name; an empty string disables the header." required:"N"` // Deprecated: replaced by the *_HEADER_NAME configs above. Setting one of // these to true still disables the corresponding header (an empty @@ -72,7 +72,9 @@ type DestinationWebhookConfig struct { DisableDefaultEventIDHeader bool `yaml:"disable_default_event_id_header" env:"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_EVENT_ID_HEADER" desc:"Deprecated: set DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME to an empty string to disable the event ID header instead. Only applies to 'default' mode." required:"N"` DisableDefaultSignatureHeader bool `yaml:"disable_default_signature_header" env:"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_SIGNATURE_HEADER" desc:"Deprecated: set DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME to an empty string to disable the signature header instead. Only applies to 'default' mode." required:"N"` DisableDefaultTimestampHeader bool `yaml:"disable_default_timestamp_header" env:"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TIMESTAMP_HEADER" desc:"Deprecated: set DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME to an empty string to disable the timestamp header instead. Only applies to 'default' mode." required:"N"` - DisableDefaultTopicHeader bool `yaml:"disable_default_topic_header" env:"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TOPIC_HEADER" desc:"Deprecated: set DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME to an empty string to disable the topic header instead. Only applies to 'default' mode." required:"N"` + DisableDefaultTopicHeader bool `yaml:"disable_default_topic_header" env:"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TOPIC_HEADER" desc:"Deprecated: set DESTINATIONS_WEBHOOK_TOPIC_HEADER_NAME to an empty string to disable the topic header instead." required:"N"` + + TimestampFormat string `yaml:"timestamp_format" env:"DESTINATIONS_WEBHOOK_TIMESTAMP_FORMAT" desc:"Format of the timestamp header: 'rfc3339' or 'unix' (seconds since the epoch). Defaults to 'rfc3339'. Only applies to 'default' mode." required:"N"` SignatureContentTemplate string `yaml:"signature_content_template" env:"DESTINATIONS_WEBHOOK_SIGNATURE_CONTENT_TEMPLATE" desc:"Go template for constructing the content to be signed for webhook requests. Only applies to 'default' mode." required:"N"` SignatureHeaderTemplate string `yaml:"signature_header_template" env:"DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_TEMPLATE" desc:"Go template for the value of the signature header. Only applies to 'default' mode." required:"N"` @@ -83,7 +85,7 @@ type DestinationWebhookConfig struct { SigningSecretTemplate string `yaml:"signing_secret_template" env:"DESTINATIONS_WEBHOOK_SIGNING_SECRET_TEMPLATE" desc:"Go template for generating webhook signing secrets. Available variables: {{.RandomHex}} (64-char hex), {{.RandomBase64}} (base64-encoded), {{.RandomAlphanumeric}} (32-char alphanumeric). Defaults to 'whsec_{{.RandomHex}}'. Only applies to 'default' mode." required:"N"` MaxResponseBodyBytes int `yaml:"max_response_body_bytes" env:"DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES" desc:"Maximum size in bytes of a destination's response body stored on the delivery attempt. Responses larger than this are replaced with a placeholder so the attempt log stays under the event queue's per-message size limit (oversized log messages fail to publish and retry indefinitely). Default: 131072 (128 KiB). Set to 0 to disable the cap." required:"N"` - Compat DestinationWebhookCompatConfig `yaml:"compat" desc:"A second signature sent alongside the primary one, so receivers verifying an older scheme keep working while they migrate. Only applies to 'default' mode."` + Compat DestinationWebhookCompatConfig `yaml:"compat" desc:"A second signature sent alongside the primary one, so receivers verifying an older scheme keep working while they migrate."` } // DestinationWebhookCompatConfig mirrors the primary signature options. It is @@ -146,25 +148,23 @@ func (c *DestinationWebhookConfig) toConfig() *destregistrydefault.DestWebhookCo if headerPrefix == "" { // Apply mode-specific default only when truly empty (not whitespace) if c.Mode == "standard" { - headerPrefix = "webhook-" + headerPrefix = destwebhook.StandardHeaderPrefix } else { - headerPrefix = "x-outpost-" + headerPrefix = destwebhook.DefaultHeaderPrefix } } - compat := c.Compat.toProviderConfig() - - return &destregistrydefault.DestWebhookConfig{ - Compat: compat, + cfg := &destregistrydefault.DestWebhookConfig{ + Compat: c.Compat.toProviderConfig(), SignatureSecretEncoding: c.SignatureSecretEncoding, SignatureSecretPrefix: c.SignatureSecretPrefix, - Mode: c.Mode, ProxyURL: c.ProxyURL, HeaderPrefix: headerPrefix, EventIDHeader: resolveWebhookHeaderName(c.EventIDHeaderName, c.DisableDefaultEventIDHeader), SignatureHeader: resolveWebhookHeaderName(c.SignatureHeaderName, c.DisableDefaultSignatureHeader), TimestampHeader: resolveWebhookHeaderName(c.TimestampHeaderName, c.DisableDefaultTimestampHeader), TopicHeader: resolveWebhookHeaderName(c.TopicHeaderName, c.DisableDefaultTopicHeader), + TimestampFormat: c.TimestampFormat, SignatureContentTemplate: c.SignatureContentTemplate, SignatureHeaderTemplate: c.SignatureHeaderTemplate, SignatureEncoding: c.SignatureEncoding, @@ -172,6 +172,64 @@ func (c *DestinationWebhookConfig) toConfig() *destregistrydefault.DestWebhookCo SigningSecretTemplate: c.SigningSecretTemplate, MaxResponseBodyBytes: c.MaxResponseBodyBytes, } + if c.Mode == "standard" { + applyStandardWebhooks(cfg) + } + return cfg +} + +// applyStandardWebhooks is standard mode. The provider has no notion of +// modes: config sets the options that define the Standard Webhooks format +// here, and the configured values for them are ignored (see +// standardModeWarnings). The prefix, topic header, compat signature, proxy +// and response cap still apply. +func applyStandardWebhooks(cfg *destregistrydefault.DestWebhookConfig) { + cfg.MetadataName = destwebhook.StandardMetadataName + cfg.SignatureContentTemplate = destwebhook.StandardSignatureContentTmpl + cfg.SignatureHeaderTemplate = destwebhook.StandardSignatureHeaderTmpl + cfg.SignatureEncoding = destwebhook.StandardEncoding + cfg.SignatureAlgorithm = destwebhook.DefaultAlgorithm + cfg.SigningSecretTemplate = destwebhook.StandardSigningSecretTmpl + cfg.SignatureSecretEncoding = destwebhook.StandardSecretEncoding + cfg.SignatureSecretPrefix = destwebhook.StandardSecretPrefix + cfg.TimestampFormat = destwebhook.StandardTimestampFormat + cfg.EventIDHeader = destregistrydefault.WebhookHeaderConfig{Name: strings.TrimSpace(cfg.HeaderPrefix) + destwebhook.StandardEventIDHeaderKey} + cfg.SignatureHeader = destregistrydefault.WebhookHeaderConfig{} + cfg.TimestampHeader = destregistrydefault.WebhookHeaderConfig{} +} + +// standardModeWarnings names each option set to a non-default value that +// standard mode ignores. +func (c *DestinationWebhookConfig) standardModeWarnings() []string { + if c.Mode != "standard" { + return nil + } + isSet := func(o OptionalString) bool { _, set := o.Get(); return set } + var ignored []string + for _, o := range []struct { + env string + isSet bool + }{ + {"DESTINATIONS_WEBHOOK_EVENT_ID_HEADER_NAME", isSet(c.EventIDHeaderName)}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_NAME", isSet(c.SignatureHeaderName)}, + {"DESTINATIONS_WEBHOOK_TIMESTAMP_HEADER_NAME", isSet(c.TimestampHeaderName)}, + {"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_EVENT_ID_HEADER", c.DisableDefaultEventIDHeader}, + {"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_SIGNATURE_HEADER", c.DisableDefaultSignatureHeader}, + {"DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TIMESTAMP_HEADER", c.DisableDefaultTimestampHeader}, + {"DESTINATIONS_WEBHOOK_TIMESTAMP_FORMAT", c.TimestampFormat != ""}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_CONTENT_TEMPLATE", c.SignatureContentTemplate != destwebhook.DefaultSignatureContentTmpl}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_HEADER_TEMPLATE", c.SignatureHeaderTemplate != destwebhook.DefaultSignatureHeaderTmpl}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_ENCODING", c.SignatureEncoding != destwebhook.DefaultEncoding}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_ALGORITHM", c.SignatureAlgorithm != destwebhook.DefaultAlgorithm}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_ENCODING", c.SignatureSecretEncoding != ""}, + {"DESTINATIONS_WEBHOOK_SIGNATURE_SECRET_PREFIX", c.SignatureSecretPrefix != ""}, + {"DESTINATIONS_WEBHOOK_SIGNING_SECRET_TEMPLATE", c.SigningSecretTemplate != destwebhook.DefaultSigningSecretTmpl}, + } { + if o.isSet { + ignored = append(ignored, fmt.Sprintf("%s is ignored in standard webhook mode.", o.env)) + } + } + return ignored } // resolveWebhookHeaderName applies the three-state rule for a webhook header diff --git a/internal/config/validation.go b/internal/config/validation.go index 35fc7cde3..141fa0a18 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -179,9 +179,6 @@ func (c *Config) validateDestinations() error { if _, err := destregistry.ParseProxyURL(c.Destinations.Webhook.ProxyURL); err != nil { return fmt.Errorf("%w: %w", ErrInvalidWebhookProxyURL, err) } - if c.Destinations.Webhook.Compat.toProviderConfig() != nil && c.Destinations.Webhook.Mode == "standard" { - return fmt.Errorf("%w: only supported in 'default' webhook mode", ErrInvalidWebhookCompatSignature) - } return nil } diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go index 4ac165444..a26f75cb3 100644 --- a/internal/config/validation_test.go +++ b/internal/config/validation_test.go @@ -317,14 +317,14 @@ func TestMisc(t *testing.T) { wantErr: nil, }, { - name: "webhook compat signature is rejected in standard mode", + name: "webhook compat signature is valid in standard mode", config: func() *config.Config { c := validConfig() c.Destinations.Webhook.Mode = "standard" c.Destinations.Webhook.Compat.SignatureHeaderName = "x-legacy-signature" return c }(), - wantErr: config.ErrInvalidWebhookCompatSignature, + wantErr: nil, }, { name: "webhook compat options are ignored without a signature header name", diff --git a/internal/destregistry/providers/default.go b/internal/destregistry/providers/default.go index cd34b7bfe..4fbe48263 100644 --- a/internal/destregistry/providers/default.go +++ b/internal/destregistry/providers/default.go @@ -15,7 +15,6 @@ import ( "github.com/hookdeck/outpost/internal/destregistry/providers/destkafka" "github.com/hookdeck/outpost/internal/destregistry/providers/destrabbitmq" "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" - "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard" "github.com/hookdeck/outpost/internal/emetrics" ) @@ -29,13 +28,16 @@ type WebhookHeaderConfig struct { } type DestWebhookConfig struct { - Mode string + // MetadataName selects the metadata/providers entry describing the + // provider; empty means "webhook". + MetadataName string ProxyURL string HeaderPrefix string EventIDHeader WebhookHeaderConfig SignatureHeader WebhookHeaderConfig TimestampHeader WebhookHeaderConfig TopicHeader WebhookHeaderConfig + TimestampFormat string SignatureContentTemplate string SignatureHeaderTemplate string SignatureEncoding string @@ -98,53 +100,38 @@ func RegisterDefault(registry destregistry.Registry, opts RegisterDefaultDestina } } - // Register webhook provider based on mode - if opts.Webhook != nil && opts.Webhook.Mode == "standard" { - // Standard Webhooks mode - register webhook_standard as "webhook" - webhookStandardOpts := []destwebhookstandard.Option{ - destwebhookstandard.WithUserAgent(opts.UserAgent), - destwebhookstandard.WithProxy(proxy), - destwebhookstandard.WithHeaderPrefix(opts.Webhook.HeaderPrefix), - destwebhookstandard.WithMaxResponseBodyBytes(opts.Webhook.MaxResponseBodyBytes), - destwebhookstandard.WithConnectionPool(fanOutPool), - destwebhookstandard.WithConnectionObserver(connObserver("webhook")), - } - webhookStandard, err := destwebhookstandard.New(loader, basePublisherOpts, webhookStandardOpts...) - if err != nil { - return err - } - registry.RegisterProvider("webhook", webhookStandard) - } else { - // Default mode - register customizable webhook as "webhook" - webhookOpts := []destwebhook.Option{ - destwebhook.WithUserAgent(opts.UserAgent), - destwebhook.WithConnectionPool(fanOutPool), - destwebhook.WithConnectionObserver(connObserver("webhook")), - } - if opts.Webhook != nil { - webhookOpts = append(webhookOpts, - destwebhook.WithProxy(proxy), - destwebhook.WithHeaderPrefix(opts.Webhook.HeaderPrefix), - destwebhook.WithEventIDHeader(opts.Webhook.EventIDHeader.Name, opts.Webhook.EventIDHeader.Disabled), - destwebhook.WithSignatureHeader(opts.Webhook.SignatureHeader.Name, opts.Webhook.SignatureHeader.Disabled), - destwebhook.WithTimestampHeader(opts.Webhook.TimestampHeader.Name, opts.Webhook.TimestampHeader.Disabled), - destwebhook.WithTopicHeader(opts.Webhook.TopicHeader.Name, opts.Webhook.TopicHeader.Disabled), - destwebhook.WithSignatureContentTemplate(opts.Webhook.SignatureContentTemplate), - destwebhook.WithSignatureHeaderTemplate(opts.Webhook.SignatureHeaderTemplate), - destwebhook.WithSignatureEncoding(opts.Webhook.SignatureEncoding), - destwebhook.WithSignatureAlgorithm(opts.Webhook.SignatureAlgorithm), - destwebhook.WithSigningSecretTemplate(opts.Webhook.SigningSecretTemplate), - destwebhook.WithMaxResponseBodyBytes(opts.Webhook.MaxResponseBodyBytes), - destwebhook.WithSecretEncoding(opts.Webhook.SignatureSecretEncoding, opts.Webhook.SignatureSecretPrefix), - destwebhook.WithCompatSignature(opts.Webhook.Compat), - ) - } - webhook, err := destwebhook.New(loader, basePublisherOpts, webhookOpts...) - if err != nil { - return err + webhookOpts := []destwebhook.Option{ + destwebhook.WithUserAgent(opts.UserAgent), + destwebhook.WithConnectionPool(fanOutPool), + destwebhook.WithConnectionObserver(connObserver("webhook")), + } + if opts.Webhook != nil { + webhookOpts = append(webhookOpts, + destwebhook.WithProxy(proxy), + destwebhook.WithHeaderPrefix(opts.Webhook.HeaderPrefix), + destwebhook.WithEventIDHeader(opts.Webhook.EventIDHeader.Name, opts.Webhook.EventIDHeader.Disabled), + destwebhook.WithSignatureHeader(opts.Webhook.SignatureHeader.Name, opts.Webhook.SignatureHeader.Disabled), + destwebhook.WithTimestampHeader(opts.Webhook.TimestampHeader.Name, opts.Webhook.TimestampHeader.Disabled), + destwebhook.WithTopicHeader(opts.Webhook.TopicHeader.Name, opts.Webhook.TopicHeader.Disabled), + destwebhook.WithTimestampFormat(opts.Webhook.TimestampFormat), + destwebhook.WithSignatureContentTemplate(opts.Webhook.SignatureContentTemplate), + destwebhook.WithSignatureHeaderTemplate(opts.Webhook.SignatureHeaderTemplate), + destwebhook.WithSignatureEncoding(opts.Webhook.SignatureEncoding), + destwebhook.WithSignatureAlgorithm(opts.Webhook.SignatureAlgorithm), + destwebhook.WithSigningSecretTemplate(opts.Webhook.SigningSecretTemplate), + destwebhook.WithMaxResponseBodyBytes(opts.Webhook.MaxResponseBodyBytes), + destwebhook.WithSecretEncoding(opts.Webhook.SignatureSecretEncoding, opts.Webhook.SignatureSecretPrefix), + destwebhook.WithCompatSignature(opts.Webhook.Compat), + ) + if opts.Webhook.MetadataName != "" { + webhookOpts = append(webhookOpts, destwebhook.WithMetadataName(opts.Webhook.MetadataName)) } - registry.RegisterProvider("webhook", webhook) } + webhook, err := destwebhook.New(loader, basePublisherOpts, webhookOpts...) + if err != nil { + return err + } + registry.RegisterProvider("webhook", webhook) hookdeck, err := desthookdeck.New(loader, basePublisherOpts, desthookdeck.WithUserAgent(opts.UserAgent), diff --git a/internal/destregistry/providers/default_test.go b/internal/destregistry/providers/default_test.go index adecc6478..1fa61abd6 100644 --- a/internal/destregistry/providers/default_test.go +++ b/internal/destregistry/providers/default_test.go @@ -6,43 +6,52 @@ import ( "github.com/hookdeck/outpost/internal/destregistry" destregistrydefault "github.com/hookdeck/outpost/internal/destregistry/providers" "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" + "github.com/hookdeck/outpost/internal/models" "github.com/hookdeck/outpost/internal/util/testutil" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Signature template validation lives in destwebhook.New, so registration is -// where a bad template fails startup — and only in default mode. In 'standard' -// mode the templates are fixed by the Standard Webhooks spec, the configured -// ones are never parsed, and registration must succeed regardless of their -// contents. +// where a bad template fails startup. func TestRegisterDefault_WebhookSignatureTemplates(t *testing.T) { - webhookConfig := func(mode string) *destregistrydefault.DestWebhookConfig { - return &destregistrydefault.DestWebhookConfig{ - Mode: mode, + registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) + err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{ + Webhook: &destregistrydefault.DestWebhookConfig{ HeaderPrefix: destwebhook.DefaultHeaderPrefix, SignatureContentTemplate: destwebhook.DefaultSignatureContentTmpl, SignatureHeaderTemplate: "v0={{.Body}}", // header templates have no .Body — invalid at render SignatureEncoding: destwebhook.DefaultEncoding, SignatureAlgorithm: destwebhook.DefaultAlgorithm, SigningSecretTemplate: destwebhook.DefaultSigningSecretTmpl, - } - } - - t.Run("default mode rejects an invalid template", func(t *testing.T) { - registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) - err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{ - Webhook: webhookConfig(""), - }) - assert.ErrorContains(t, err, "can't evaluate field Body") + }, }) + assert.ErrorContains(t, err, "can't evaluate field Body") +} - t.Run("standard mode ignores the configured templates", func(t *testing.T) { - registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) - err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{ - Webhook: webhookConfig("standard"), - }) - assert.NoError(t, err) +// Standard mode swaps the provider's metadata for the entry that carries the +// Standard Webhooks verification instructions. +func TestRegisterDefault_WebhookStandardMetadata(t *testing.T) { + registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) + err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{ + Webhook: &destregistrydefault.DestWebhookConfig{ + MetadataName: destwebhook.StandardMetadataName, + HeaderPrefix: destwebhook.StandardHeaderPrefix, + SignatureContentTemplate: destwebhook.StandardSignatureContentTmpl, + SignatureHeaderTemplate: destwebhook.StandardSignatureHeaderTmpl, + SignatureEncoding: destwebhook.StandardEncoding, + SignatureAlgorithm: destwebhook.DefaultAlgorithm, + SigningSecretTemplate: destwebhook.StandardSigningSecretTmpl, + SignatureSecretEncoding: destwebhook.StandardSecretEncoding, + SignatureSecretPrefix: destwebhook.StandardSecretPrefix, + }, }) + require.NoError(t, err) + + provider, err := registry.ResolveProvider(&models.Destination{Type: "webhook"}) + require.NoError(t, err) + instructions := provider.Metadata().Instructions + assert.Contains(t, instructions, "webhook-signature") } func TestRegisterDefault_WebhookCompatSignature(t *testing.T) { diff --git a/internal/destregistry/providers/destwebhook/destwebhook.go b/internal/destregistry/providers/destwebhook/destwebhook.go index 26bb1495a..33d67d60f 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook.go +++ b/internal/destregistry/providers/destwebhook/destwebhook.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "regexp" + "strconv" "strings" "text/template" "time" @@ -31,6 +32,12 @@ const ( DefaultSigningSecretTmpl = "whsec_{{.RandomHex}}" ) +// Timestamp header formats. +const ( + TimestampFormatRFC3339 = "rfc3339" + TimestampFormatUnix = "unix" // seconds since the epoch, as Standard Webhooks requires +) + // Reserved headers that cannot be set via custom_headers var reservedHeaders = map[string]bool{ "content-type": true, @@ -94,6 +101,7 @@ type headerConfig struct { type WebhookDestination struct { *destregistry.BaseProvider + metadataName string headerPrefix string userAgent string proxy []*url.URL @@ -103,6 +111,7 @@ type WebhookDestination struct { signatureHeader headerConfig timestampHeader headerConfig topicHeader headerConfig + timestampFormat string encoding string algorithm string secretEncoding string @@ -174,6 +183,14 @@ func WithUserAgent(userAgent string) Option { } } +// WithMetadataName selects the metadata/providers entry (schema and +// instructions) the provider is described by. Defaults to "webhook". +func WithMetadataName(name string) Option { + return func(w *WebhookDestination) { + w.metadataName = name + } +} + // WithProxy routes every request through the given forward proxies, // nearest hop first. See destregistry.ParseProxyURL. func WithProxy(hops []*url.URL) Option { @@ -242,6 +259,14 @@ func WithTopicHeader(name string, disabled bool) Option { } } +// WithTimestampFormat sets how the timestamp header renders the delivery time: +// TimestampFormatRFC3339 (the default) or TimestampFormatUnix. +func WithTimestampFormat(format string) Option { + return func(w *WebhookDestination) { + w.timestampFormat = format + } +} + func WithSignatureContentTemplate(template string) Option { return func(w *WebhookDestination) { w.signatureContentTemplate = template @@ -280,16 +305,15 @@ type signingSecretTemplateData struct { } func New(loader metadata.MetadataLoader, basePublisherOpts []destregistry.BasePublisherOption, opts ...Option) (*WebhookDestination, error) { - base, err := destregistry.NewBaseProvider(loader, "webhook", basePublisherOpts...) - if err != nil { - return nil, err - } - destination := &WebhookDestination{ - BaseProvider: base, - } + destination := &WebhookDestination{metadataName: "webhook"} for _, opt := range opts { opt(destination) } + base, err := destregistry.NewBaseProvider(loader, destination.metadataName, basePublisherOpts...) + if err != nil { + return nil, err + } + destination.BaseProvider = base // Validate all required configuration is provided // Config is responsible for setting defaults - provider requires explicit values @@ -297,6 +321,11 @@ func New(loader metadata.MetadataLoader, basePublisherOpts []destregistry.BasePu if destination.rawSigningSecretTemplate == "" { return nil, fmt.Errorf("signing secret template is required") } + switch destination.timestampFormat { + case "", TimestampFormatRFC3339, TimestampFormatUnix: + default: + return nil, fmt.Errorf("invalid timestamp format %q: must be one of %s, %s", destination.timestampFormat, TimestampFormatRFC3339, TimestampFormatUnix) + } destination.scheme, err = newSignatureScheme(signatureSchemeConfig{ ContentTemplate: destination.signatureContentTemplate, HeaderTemplate: destination.signatureHeaderTemplate, @@ -515,6 +544,7 @@ func (d *WebhookDestination) CreatePublisher(ctx context.Context, destination *m signatureHeader: d.signatureHeader, timestampHeader: d.timestampHeader, topicHeader: d.topicHeader, + timestampFormat: d.timestampFormat, secrets: secrets, sm: sm, primary: d.primary, @@ -799,6 +829,7 @@ type WebhookPublisher struct { signatureHeader headerConfig timestampHeader headerConfig topicHeader headerConfig + timestampFormat string secrets []WebhookSecret sm *SignatureManager primary headerSet @@ -871,6 +902,11 @@ func (p *WebhookPublisher) Format(ctx context.Context, event *models.Event) (*ht if !ok { continue } + // Delivery and event metadata may override the system timestamp; only + // the system value is reformatted. + if key == "timestamp" && p.timestampFormat == TimestampFormatUnix && value == now.UTC().Format(time.RFC3339) { + value = strconv.FormatInt(now.Unix(), 10) + } req.Header.Set(name, value) } diff --git a/internal/destregistry/providers/destwebhook/standard.go b/internal/destregistry/providers/destwebhook/standard.go new file mode 100644 index 000000000..6fcdfce25 --- /dev/null +++ b/internal/destregistry/providers/destwebhook/standard.go @@ -0,0 +1,17 @@ +package destwebhook + +// Standard Webhooks (https://www.standardwebhooks.com) expressed as values of +// the webhook options. "standard" mode is resolved in config, which sets +// these; the provider itself has no modes. +const ( + StandardHeaderPrefix = "webhook-" + StandardEventIDHeaderKey = "id" // "id" rather than "event-id" + StandardTimestampFormat = TimestampFormatUnix + StandardSignatureContentTmpl = "{{.EventID}}.{{.Timestamp.Unix}}.{{.Body}}" + StandardSignatureHeaderTmpl = "v1,{{index .Signatures 0}}{{range slice .Signatures 1}} v1,{{.}}{{end}}" + StandardEncoding = "base64" + StandardSecretEncoding = SecretEncodingBase64 + StandardSecretPrefix = "whsec_" + StandardSigningSecretTmpl = "whsec_{{.RandomBase64}}" + StandardMetadataName = "webhook_standard" // metadata/providers dir with the verification instructions +) diff --git a/internal/destregistry/providers/destwebhook/standard_test.go b/internal/destregistry/providers/destwebhook/standard_test.go new file mode 100644 index 000000000..9c7b83d44 --- /dev/null +++ b/internal/destregistry/providers/destwebhook/standard_test.go @@ -0,0 +1,277 @@ +package destwebhook_test + +import ( + "context" + "crypto/hmac" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "net/http" + "strconv" + "strings" + "testing" + "time" + + "github.com/hookdeck/outpost/internal/destregistry" + "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" + "github.com/hookdeck/outpost/internal/models" + "github.com/hookdeck/outpost/internal/util/testutil" + standardwebhooks "github.com/standard-webhooks/standard-webhooks/libraries/go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The Standard Webhooks preset, as config resolves "standard" mode: the +// provider gets the Standard* values and the event ID header pinned to +// "id". +func standardOptions(prefix string) []destwebhook.Option { + return []destwebhook.Option{ + destwebhook.WithHeaderPrefix(prefix), + destwebhook.WithEventIDHeader(strings.TrimSpace(prefix)+destwebhook.StandardEventIDHeaderKey, false), + destwebhook.WithTimestampFormat(destwebhook.StandardTimestampFormat), + destwebhook.WithSignatureContentTemplate(destwebhook.StandardSignatureContentTmpl), + destwebhook.WithSignatureHeaderTemplate(destwebhook.StandardSignatureHeaderTmpl), + destwebhook.WithSignatureEncoding(destwebhook.StandardEncoding), + destwebhook.WithSecretEncoding(destwebhook.StandardSecretEncoding, destwebhook.StandardSecretPrefix), + destwebhook.WithSigningSecretTemplate(destwebhook.StandardSigningSecretTmpl), + } +} + +func newStandardProvider(t *testing.T, opts ...destwebhook.Option) *destwebhook.WebhookDestination { + t.Helper() + return NewTestProvider(t, append(standardOptions(destwebhook.StandardHeaderPrefix), opts...)...) +} + +const ( + standardSecret = "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw" + standardPreviousSecret = "whsec_T2xkU2VjcmV0U3RyaW5nMTIz" +) + +func standardDestination(credentials map[string]string, opts ...func(*models.Destination)) models.Destination { + base := []func(*models.Destination){ + testutil.DestinationFactory.WithType("webhook"), + testutil.DestinationFactory.WithConfig(map[string]string{"url": "http://example.com/webhook"}), + testutil.DestinationFactory.WithCredentials(credentials), + } + return testutil.DestinationFactory.Any(append(base, opts...)...) +} + +func formatStandard(t *testing.T, provider *destwebhook.WebhookDestination, destination models.Destination, event models.Event) *http.Request { + t.Helper() + publisher, err := provider.CreatePublisher(context.Background(), &destination) + require.NoError(t, err) + defer publisher.Close() + req, err := publisher.(*destwebhook.WebhookPublisher).Format(context.Background(), &event) + require.NoError(t, err) + return req +} + +func requestBody(t *testing.T, req *http.Request) []byte { + t.Helper() + body, err := req.GetBody() + require.NoError(t, err) + data := make([]byte, 0, 256) + buf := make([]byte, 256) + for { + n, err := body.Read(buf) + data = append(data, buf[:n]...) + if err != nil { + break + } + } + return data +} + +// assertStandardWebhooksSDK verifies the request with the official SDK, the +// way a receiver would. +func assertStandardWebhooksSDK(t *testing.T, secret string, req *http.Request) { + t.Helper() + wh, err := standardwebhooks.NewWebhook(secret) + require.NoError(t, err) + assert.NoError(t, wh.Verify(requestBody(t, req), req.Header), "Standard Webhooks SDK should verify with %s", secret) +} + +func TestStandardPreset_VerifiesWithSDK(t *testing.T) { + t.Parallel() + + provider := newStandardProvider(t) + event := testutil.EventFactory.Any( + testutil.EventFactory.WithID("msg_2KWPBgLlAfxdpx2AI54pPJ85f4W"), + testutil.EventFactory.WithTopic("user.created"), + testutil.EventFactory.WithDataMap(map[string]interface{}{"hello": "world"}), + ) + + t.Run("single secret", func(t *testing.T) { + before := time.Now() + req := formatStandard(t, provider, standardDestination(map[string]string{"secret": standardSecret}), event) + + assert.Equal(t, "msg_2KWPBgLlAfxdpx2AI54pPJ85f4W", req.Header.Get("webhook-id")) + assert.Empty(t, req.Header.Get("webhook-event-id")) + seconds, err := strconv.ParseInt(req.Header.Get("webhook-timestamp"), 10, 64) + require.NoError(t, err, "webhook-timestamp must be unix seconds") + assert.InDelta(t, before.Unix(), seconds, 2) + assert.Equal(t, "user.created", req.Header.Get("webhook-topic")) + + signature := req.Header.Get("webhook-signature") + require.True(t, strings.HasPrefix(signature, "v1,"), "signature %q", signature) + raw, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(signature, "v1,")) + require.NoError(t, err) + assert.Len(t, raw, sha256.Size) + + assertStandardWebhooksSDK(t, standardSecret, req) + }) + + t.Run("rotation signs with both secrets", func(t *testing.T) { + req := formatStandard(t, provider, standardDestination(map[string]string{ + "secret": standardSecret, + "previous_secret": standardPreviousSecret, + "previous_secret_invalid_at": time.Now().Add(time.Hour).Format(time.RFC3339), + }), event) + + signatures := strings.Split(req.Header.Get("webhook-signature"), " ") + assert.Len(t, signatures, 2) + for _, sig := range signatures { + assert.True(t, strings.HasPrefix(sig, "v1,"), "signature %q", sig) + } + assertStandardWebhooksSDK(t, standardSecret, req) + assertStandardWebhooksSDK(t, standardPreviousSecret, req) + }) + + t.Run("expired previous secret is dropped", func(t *testing.T) { + req := formatStandard(t, provider, standardDestination(map[string]string{ + "secret": standardSecret, + "previous_secret": standardPreviousSecret, + "previous_secret_invalid_at": time.Now().Add(-time.Hour).Format(time.RFC3339), + }), event) + + assert.Len(t, strings.Split(req.Header.Get("webhook-signature"), " "), 1) + assertStandardWebhooksSDK(t, standardSecret, req) + }) +} + +func TestStandardPreset_HeaderPrefix(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + prefix string + want string + }{ + {"default prefix", "webhook-", "webhook-"}, + {"custom prefix", "x-custom-", "x-custom-"}, + {"whitespace disables the prefix", " ", ""}, + } { + t.Run(tt.name, func(t *testing.T) { + provider := NewTestProvider(t, standardOptions(tt.prefix)...) + event := testutil.EventFactory.Any( + testutil.EventFactory.WithID("msg_test123"), + testutil.EventFactory.WithTopic("user.created"), + testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"}), + ) + req := formatStandard(t, provider, standardDestination(map[string]string{"secret": standardSecret}), event) + + assert.Equal(t, "msg_test123", req.Header.Get(tt.want+"id")) + assert.NotEmpty(t, req.Header.Get(tt.want+"timestamp")) + assert.NotEmpty(t, req.Header.Get(tt.want+"signature")) + assert.Equal(t, "user.created", req.Header.Get(tt.want+"topic")) + if tt.want != "webhook-" { + assert.Empty(t, req.Header.Get("webhook-id")) + assert.Empty(t, req.Header.Get("webhook-signature")) + } + }) + } +} + +func TestStandardPreset_MetadataHeadersArePrefixed(t *testing.T) { + t.Parallel() + + provider := newStandardProvider(t) + event := testutil.EventFactory.Any( + testutil.EventFactory.WithMetadata(map[string]string{"source": "crm"}), + testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"}), + ) + req := formatStandard(t, provider, standardDestination(map[string]string{"secret": standardSecret}), event) + + assert.Equal(t, "crm", req.Header.Get("webhook-source")) + assert.Empty(t, req.Header.Get("source"), "event metadata is sent under the prefix only") +} + +func TestStandardPreset_SecretValidation(t *testing.T) { + t.Parallel() + + provider := newStandardProvider(t) + + assertInvalid := func(t *testing.T, credentials map[string]string, field string) { + t.Helper() + destination := standardDestination(credentials) + err := provider.Validate(context.Background(), &destination) + var validationErr *destregistry.ErrDestinationValidation + require.ErrorAs(t, err, &validationErr) + assert.Equal(t, field, validationErr.Errors[0].Field) + assert.Equal(t, "invalid", validationErr.Errors[0].Type) + } + + t.Run("secret that is not base64", func(t *testing.T) { + assertInvalid(t, map[string]string{"secret": "whsec_not-valid-base64!!!"}, "credentials.secret") + }) + + t.Run("previous secret that is not base64", func(t *testing.T) { + assertInvalid(t, map[string]string{ + "secret": standardSecret, + "previous_secret": "not-a-whsec-secret", + "previous_secret_invalid_at": time.Now().Add(time.Hour).Format(time.RFC3339), + }, "credentials.previous_secret") + }) + + t.Run("valid secrets", func(t *testing.T) { + destination := standardDestination(map[string]string{ + "secret": standardSecret, + "previous_secret": standardPreviousSecret, + "previous_secret_invalid_at": "2024-01-02T00:00:00Z", + }) + assert.NoError(t, provider.Validate(context.Background(), &destination)) + }) +} + +func TestStandardPreset_GeneratedSecret(t *testing.T) { + t.Parallel() + + provider := newStandardProvider(t) + destination := testutil.DestinationFactory.Any( + testutil.DestinationFactory.WithType("webhook"), + testutil.DestinationFactory.WithConfig(map[string]string{"url": "https://example.com"}), + ) + require.NoError(t, provider.Preprocess(&destination, nil, &destregistry.PreprocessDestinationOpts{Role: "tenant"})) + + secret := destination.Credentials["secret"] + require.True(t, strings.HasPrefix(secret, "whsec_"), "secret %q", secret) + raw, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(secret, "whsec_")) + require.NoError(t, err) + assert.Len(t, raw, 32) + assert.NoError(t, provider.Validate(context.Background(), &destination)) + + event := testutil.EventFactory.Any(testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"})) + assertStandardWebhooksSDK(t, secret, formatStandard(t, provider, destination, event)) +} + +// The compat signature is a second header set, so it works in standard mode +// the same way: here the previous scheme is Outpost's default one. +func TestStandardPreset_CompatSignature(t *testing.T) { + t.Parallel() + + provider := newStandardProvider(t, destwebhook.WithCompatSignature(&destwebhook.CompatSignatureConfig{ + SignatureHeaderName: "x-outpost-signature", + SignatureContentTemplate: destwebhook.DefaultSignatureContentTmpl, + SignatureHeaderTemplate: destwebhook.DefaultSignatureHeaderTmpl, + SignatureEncoding: destwebhook.DefaultEncoding, + SignatureAlgorithm: destwebhook.DefaultAlgorithm, + })) + event := testutil.EventFactory.Any(testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"})) + req := formatStandard(t, provider, standardDestination(map[string]string{"secret": standardSecret}), event) + + assertStandardWebhooksSDK(t, standardSecret, req) + + mac := hmac.New(sha256.New, []byte(standardSecret)) // raw secret encoding: the stored string is the key + mac.Write(requestBody(t, req)) + assert.Equal(t, "v0="+hex.EncodeToString(mac.Sum(nil)), req.Header.Get("x-outpost-signature")) +} diff --git a/internal/destregistry/providers/destwebhook/timestamp_format_test.go b/internal/destregistry/providers/destwebhook/timestamp_format_test.go new file mode 100644 index 000000000..b1f77e072 --- /dev/null +++ b/internal/destregistry/providers/destwebhook/timestamp_format_test.go @@ -0,0 +1,62 @@ +package destwebhook_test + +import ( + "context" + "strconv" + "testing" + "time" + + "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" + "github.com/hookdeck/outpost/internal/models" + "github.com/hookdeck/outpost/internal/util/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWebhookPublisher_TimestampFormat(t *testing.T) { + t.Parallel() + + format := func(t *testing.T, provider *destwebhook.WebhookDestination, deliveryMetadata map[string]string) (time.Time, string) { + t.Helper() + opts := []func(*models.Destination){ + testutil.DestinationFactory.WithType("webhook"), + testutil.DestinationFactory.WithConfig(map[string]string{"url": "http://example.com/webhook"}), + testutil.DestinationFactory.WithCredentials(map[string]string{"secret": "test-secret"}), + } + if deliveryMetadata != nil { + opts = append(opts, testutil.DestinationFactory.WithDeliveryMetadata(deliveryMetadata)) + } + destination := testutil.DestinationFactory.Any(opts...) + publisher, err := provider.CreatePublisher(context.Background(), &destination) + require.NoError(t, err) + event := testutil.EventFactory.Any(testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"})) + before := time.Now() + req, err := publisher.(*destwebhook.WebhookPublisher).Format(context.Background(), &event) + require.NoError(t, err) + return before, req.Header.Get("x-outpost-timestamp") + } + + t.Run("rfc3339 by default", func(t *testing.T) { + _, value := format(t, NewTestProvider(t), nil) + _, err := time.Parse(time.RFC3339, value) + assert.NoError(t, err, "timestamp %q", value) + }) + + t.Run("unix renders seconds", func(t *testing.T) { + before, value := format(t, NewTestProvider(t, destwebhook.WithTimestampFormat(destwebhook.TimestampFormatUnix)), nil) + seconds, err := strconv.ParseInt(value, 10, 64) + require.NoError(t, err, "timestamp %q", value) + assert.InDelta(t, before.Unix(), seconds, 2) + }) + + t.Run("unix leaves a metadata override alone", func(t *testing.T) { + _, value := format(t, NewTestProvider(t, destwebhook.WithTimestampFormat(destwebhook.TimestampFormatUnix)), + map[string]string{"timestamp": "custom"}) + assert.Equal(t, "custom", value) + }) + + t.Run("unknown format fails construction", func(t *testing.T) { + _, err := newTestProvider(destwebhook.WithTimestampFormat("millis")) + assert.ErrorContains(t, err, `invalid timestamp format "millis"`) + }) +} diff --git a/internal/destregistry/providers/destwebhookstandard/assert_test.go b/internal/destregistry/providers/destwebhookstandard/assert_test.go deleted file mode 100644 index c82cbe203..000000000 --- a/internal/destregistry/providers/destwebhookstandard/assert_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package destwebhookstandard_test - -import ( - "crypto/hmac" - "crypto/sha256" - "encoding/base64" - "fmt" - "net/http" - "strings" - - testsuite "github.com/hookdeck/outpost/internal/destregistry/testing" - standardwebhooks "github.com/standard-webhooks/standard-webhooks/libraries/go" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// assertValidStandardWebhookSignature verifies a Standard Webhooks signature -// using both our manual verification AND the official Standard Webhooks SDK -func assertValidStandardWebhookSignature(t testsuite.TestingT, secret, msgID, timestamp string, body []byte, signatureHeader string) { - t.Helper() - - // First, verify using the official Standard Webhooks SDK - // This ensures our implementation is compatible with the official library - wh, err := standardwebhooks.NewWebhook(secret) - require.NoError(t, err, "failed to create webhook verifier with official SDK") - - headers := http.Header{} - headers.Set("webhook-id", msgID) - headers.Set("webhook-timestamp", timestamp) - headers.Set("webhook-signature", signatureHeader) - - err = wh.Verify(body, headers) - assert.NoError(t, err, "official Standard Webhooks SDK should verify our signature") - - // Also verify manually to ensure we understand the signature format - encodedPart := strings.TrimPrefix(secret, "whsec_") - decodedSecret, err := base64.StdEncoding.DecodeString(encodedPart) - require.NoError(t, err, "secret should decode successfully") - - // Construct signed content: msg_id.timestamp.body - signedContent := fmt.Sprintf("%s.%s.%s", msgID, timestamp, string(body)) - - // Generate expected signature - mac := hmac.New(sha256.New, decodedSecret) - mac.Write([]byte(signedContent)) - expectedSig := base64.StdEncoding.EncodeToString(mac.Sum(nil)) - - // Check if any signature in header matches - signatures := strings.Split(signatureHeader, " ") - found := false - for _, sig := range signatures { - sigPart := strings.TrimPrefix(sig, "v1,") - if hmac.Equal([]byte(sigPart), []byte(expectedSig)) { - found = true - break - } - } - - assert.True(t, found, "no valid signature found in header (manual verification)") -} - -// assertSignatureFormat verifies the signature header format -func assertSignatureFormat(t testsuite.TestingT, signatureHeader string, expectedCount int) { - t.Helper() - - signatures := strings.Split(signatureHeader, " ") - assert.Equal(t, expectedCount, len(signatures), "signature count mismatch") - - for i, sig := range signatures { - assert.True(t, strings.HasPrefix(sig, "v1,"), "signature %d should have v1, prefix", i) - - // Verify it's valid base64 - sigPart := strings.TrimPrefix(sig, "v1,") - _, err := base64.StdEncoding.DecodeString(sigPart) - assert.NoError(t, err, "signature %d should be valid base64", i) - } -} diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go deleted file mode 100644 index 7aaf08299..000000000 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard.go +++ /dev/null @@ -1,681 +0,0 @@ -package destwebhookstandard - -/* -Standard Webhooks Destination Provider - -This implementation is based on the destwebhook provider and reuses its core -signature management infrastructure (SignatureManager). The key differences are: - -1. Secret Format: - - destwebhook: Flexible format (any string, typically hex-encoded) - - destwebhookstandard: Strict "whsec_" format per Standard Webhooks spec - -2. Header Names: - - destwebhook: Customizable via templates - - destwebhookstandard: Uses configurable prefix for all headers: "id", "timestamp", "signature", - and metadata headers (topic, etc.) - - Prefix defaults to "webhook-" in standard mode, "x-outpost-" in default mode - - Examples: "webhook-id", "webhook-timestamp" OR "x-custom-id", "x-custom-timestamp" - -3. Signature Format: - - destwebhook: Customizable template - - destwebhookstandard: Fixed "${webhook-id}.${timestamp}.${body}" signed content - and "v1," signature format - -ARCHITECTURE NOTES: - -- We import and use destwebhook.SignatureManager for signature generation -- We use destwebhook.WebhookSecret for secret storage -- Secret validation, parsing, and rotation logic is currently duplicated here - -FUTURE REFACTORING CONSIDERATIONS: - -If this pattern proves useful, consider extracting shared logic into a common package: -- Secret validation and parsing helpers -- Secret rotation logic (with configurable format validators) -- Common credential preprocessing patterns -- Shared validation error construction - -This would allow both destwebhook and destwebhookstandard to share the same -underlying infrastructure while maintaining their specific requirements. - -For now, we keep them separate to: -1. Avoid breaking changes to the existing destwebhook implementation -2. Allow independent evolution of the two providers -3. Keep the Standard Webhooks implementation self-contained for easier review - -Related files to consider for refactoring: -- internal/destregistry/providers/destwebhook/signature.go (SignatureManager) -- internal/destregistry/providers/destwebhook/destwebhook.go (rotation logic) -*/ - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "net/http" - "net/url" - "strconv" - "strings" - "time" - - "github.com/hookdeck/outpost/internal/destregistry" - "github.com/hookdeck/outpost/internal/destregistry/metadata" - "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" - "github.com/hookdeck/outpost/internal/models" -) - -// Signature templates fixed by the Standard Webhooks spec. -const ( - signatureContentTemplate = "{{.EventID}}.{{.Timestamp.Unix}}.{{.Body}}" - signatureHeaderTemplate = "v1,{{index .Signatures 0}}{{range slice .Signatures 1}} v1,{{.}}{{end}}" -) - -type StandardWebhookDestination struct { - *destregistry.BaseProvider - userAgent string - proxy []*url.URL - headerPrefix string // Prefix for metadata headers (defaults to "webhook-") - maxResponseBodyBytes int - - // httpClient is shared by every publisher this provider creates — see the - // note on destwebhook.WebhookDestination.httpClient. - httpClient *http.Client - pool destregistry.PoolSizing - onConnection func(reused bool) - // Standard Webhooks templates are fixed by the spec, so the formatters are - // built once and shared by every publisher rather than per destination. - signatureFormatter destwebhook.SignatureFormatter - headerFormatter destwebhook.HeaderFormatter -} - -type StandardWebhookDestinationConfig struct { - URL string `json:"url"` - CustomHeaders map[string]string `json:"custom_headers,omitempty"` -} - -type StandardWebhookDestinationCredentials struct { - Secret string `json:"secret"` - PreviousSecret string `json:"previous_secret,omitempty"` - PreviousSecretInvalidAt *time.Time `json:"previous_secret_invalid_at,omitempty"` -} - -var _ destregistry.Provider = (*StandardWebhookDestination)(nil) - -// Option is a functional option for configuring StandardWebhookDestination -type Option func(*StandardWebhookDestination) - -// WithUserAgent sets the user agent for the webhook request -func WithUserAgent(userAgent string) Option { - return func(d *StandardWebhookDestination) { - d.userAgent = userAgent - } -} - -// WithProxy routes every request through the given forward proxies, -// nearest hop first. See destregistry.ParseProxyURL. -func WithProxy(hops []*url.URL) Option { - return func(d *StandardWebhookDestination) { - d.proxy = hops - } -} - -// WithMaxResponseBodyBytes caps how much of the destination response body is -// stored on the attempt. 0 (default) disables the cap. -func WithMaxResponseBodyBytes(maxBytes int) Option { - return func(d *StandardWebhookDestination) { - d.maxResponseBodyBytes = maxBytes - } -} - -// WithConnectionPool sizes the shared client's idle connection pool. The zero -// value leaves Go's defaults in place. -func WithConnectionPool(pool destregistry.PoolSizing) Option { - return func(d *StandardWebhookDestination) { - d.pool = pool - } -} - -// WithConnectionObserver registers a callback invoked once per request with -// whether the underlying connection was reused. -func WithConnectionObserver(fn func(reused bool)) Option { - return func(d *StandardWebhookDestination) { - d.onConnection = fn - } -} - -// WithHeaderPrefix sets the prefix for metadata headers. -// The prefix is trimmed of whitespace. An empty string disables the prefix entirely. -// Config is responsible for providing the appropriate default ("webhook-" for standard mode). -func WithHeaderPrefix(prefix string) Option { - return func(d *StandardWebhookDestination) { - d.headerPrefix = strings.TrimSpace(prefix) - } -} - -func New(loader metadata.MetadataLoader, basePublisherOpts []destregistry.BasePublisherOption, opts ...Option) (*StandardWebhookDestination, error) { - base, err := destregistry.NewBaseProvider(loader, "webhook_standard", basePublisherOpts...) - if err != nil { - return nil, err - } - destination := &StandardWebhookDestination{ - BaseProvider: base, - } - for _, opt := range opts { - opt(destination) - } - // headerPrefix may be empty (after trimming) to disable prefix entirely — that's valid. - // But the caller must have explicitly set it via WithHeaderPrefix. - // Config is responsible for providing the appropriate default ("webhook-"). - - httpClient, err := destregistry.NewHTTPClient(destregistry.HTTPClientConfig{ - UserAgent: &destination.userAgent, - Proxy: destination.proxy, - WrapTransport: destwebhook.WrapTransport, - Pool: destination.pool, - OnConnection: destination.onConnection, - }) - if err != nil { - return nil, err - } - destination.httpClient = httpClient - - destination.signatureFormatter, err = destwebhook.NewSignatureFormatter(signatureContentTemplate) - if err != nil { - return nil, err - } - destination.headerFormatter, err = destwebhook.NewHeaderFormatter(signatureHeaderTemplate) - if err != nil { - return nil, err - } - - return destination, nil -} - -func (d *StandardWebhookDestination) ComputeTarget(destination *models.Destination) destregistry.DestinationTarget { - return destregistry.DestinationTarget{ - Target: destination.Config["url"], - TargetURL: "", - } -} - -func (d *StandardWebhookDestination) ObfuscateDestination(destination *models.Destination) *models.Destination { - result := *destination // shallow copy - result.Config = make(map[string]string, len(destination.Config)) - result.Credentials = make(map[string]string, len(destination.Credentials)) - - // Copy config values - for key, value := range destination.Config { - result.Config[key] = value - } - - // Check if previous_secret has expired - skipPreviousSecret := false - if invalidAtStr := destination.Credentials["previous_secret_invalid_at"]; invalidAtStr != "" { - if invalidAt, err := time.Parse(time.RFC3339, invalidAtStr); err == nil { - if time.Now().After(invalidAt) { - skipPreviousSecret = true - } - } - } - - // Copy credentials, omitting expired previous_secret fields - // NOTE: Secrets are intentionally not obfuscated for now because: - // 1. They're needed for secret rotation logic - // 2. They're less security-critical than other provider credentials - for key, value := range destination.Credentials { - if skipPreviousSecret && (key == "previous_secret" || key == "previous_secret_invalid_at") { - continue - } - result.Credentials[key] = value - } - - return &result -} - -func (d *StandardWebhookDestination) Validate(ctx context.Context, destination *models.Destination) error { - if _, _, err := d.resolveConfig(ctx, destination); err != nil { - return err - } - return nil -} - -func (d *StandardWebhookDestination) CreatePublisher(ctx context.Context, destination *models.Destination) (destregistry.Publisher, error) { - config, creds, err := d.resolveConfig(ctx, destination) - if err != nil { - return nil, err - } - - // Parse and validate secrets - now := time.Now() - var secrets []destwebhook.WebhookSecret - - // Parse current secret - parsedSecret, err := parseSecret(creds.Secret) - if err != nil { - return nil, fmt.Errorf("failed to parse secret: %w", err) - } - secrets = append(secrets, destwebhook.WebhookSecret{ - Key: parsedSecret, - CreatedAt: now, - }) - - // Parse previous secret if present - if creds.PreviousSecret != "" { - parsedPrevSecret, err := parseSecret(creds.PreviousSecret) - if err != nil { - return nil, fmt.Errorf("failed to parse previous_secret: %w", err) - } - secrets = append(secrets, destwebhook.WebhookSecret{ - Key: parsedPrevSecret, - CreatedAt: now.Add(-1 * time.Hour), // Set to 1 hour before current secret - InvalidAt: creds.PreviousSecretInvalidAt, - }) - } - - // Create SignatureManager with the shared Standard Webhooks formatters - sm := destwebhook.NewSignatureManager( - secrets, - destwebhook.WithSignatureFormatter(d.signatureFormatter), - destwebhook.WithHeaderFormatter(d.headerFormatter), - destwebhook.WithEncoder(destwebhook.GetEncoder("base64")), - destwebhook.WithAlgorithm(destwebhook.GetAlgorithm("hmac-sha256")), - ) - - return &StandardWebhookPublisher{ - BasePublisher: d.BaseProvider.NewPublisher(destregistry.WithDeliveryMetadata(destination.DeliveryMetadata)), - httpClient: d.httpClient, - url: config.URL, - secrets: secrets, - sm: sm, - headerPrefix: d.headerPrefix, - customHeaders: config.CustomHeaders, - maxResponseBodyBytes: d.maxResponseBodyBytes, - }, nil -} - -func (d *StandardWebhookDestination) resolveConfig(ctx context.Context, destination *models.Destination) (*StandardWebhookDestinationConfig, *StandardWebhookDestinationCredentials, error) { - if err := d.BaseProvider.Validate(ctx, destination); err != nil { - return nil, nil, err - } - - config := &StandardWebhookDestinationConfig{ - URL: destination.Config["url"], - } - - // Parse custom headers from config - if headersJSON, ok := destination.Config["custom_headers"]; ok && headersJSON != "" { - if err := json.Unmarshal([]byte(headersJSON), &config.CustomHeaders); err != nil { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "config.custom_headers", - Type: "invalid", - }}) - } - if len(config.CustomHeaders) == 0 { - config.CustomHeaders = nil - } else if err := destwebhook.ValidateCustomHeaders(config.CustomHeaders); err != nil { - return nil, nil, err - } - } - - // Parse credentials - creds := &StandardWebhookDestinationCredentials{ - Secret: destination.Credentials["secret"], - PreviousSecret: destination.Credentials["previous_secret"], - } - - // Skip validation if no relevant credentials are passed - if destination.Credentials["secret"] == "" && - destination.Credentials["previous_secret"] == "" && - destination.Credentials["previous_secret_invalid_at"] == "" { - return config, creds, nil - } - - // If any credentials are passed, secret is required - if creds.Secret == "" { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "credentials.secret", - Type: "required", - }}) - } - - // Validate secret format - if err := validateSecret(creds.Secret); err != nil { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "credentials.secret", - Type: "pattern", - }}) - } - - // Parse previous_secret_invalid_at if present - if invalidAtStr := destination.Credentials["previous_secret_invalid_at"]; invalidAtStr != "" { - invalidAt, err := time.Parse(time.RFC3339, invalidAtStr) - if err != nil { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "credentials.previous_secret_invalid_at", - Type: "pattern", - }}) - } - creds.PreviousSecretInvalidAt = &invalidAt - } - - // Validate previous_secret if provided - if creds.PreviousSecret != "" { - if err := validateSecret(creds.PreviousSecret); err != nil { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "credentials.previous_secret", - Type: "pattern", - }}) - } - - // Require invalidation time if previous secret is provided - if creds.PreviousSecretInvalidAt == nil { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "credentials.previous_secret_invalid_at", - Type: "required", - }}) - } - } - - // If previous_secret_invalid_at is provided, validate previous_secret - if creds.PreviousSecretInvalidAt != nil && creds.PreviousSecret == "" { - return nil, nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ - Field: "credentials.previous_secret", - Type: "required", - }}) - } - - return config, creds, nil -} - -// rotateSecret handles secret rotation and returns clean credentials -func (d *StandardWebhookDestination) rotateSecret(origDest *models.Destination, opts *destregistry.PreprocessDestinationOpts) (map[string]string, error) { - if origDest == nil { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.rotate_secret", - Type: "invalid", - }, - }) - } - - if origDest.Credentials["secret"] == "" { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.secret", - Type: "required", - }, - }) - } - - creds := make(map[string]string) - - // Store the current secret as the previous secret - creds["previous_secret"] = origDest.Credentials["secret"] - - // Generate a new secret - secret, err := generateStandardSecret() - if err != nil { - return nil, err - } - creds["secret"] = secret - - // Keep custom invalidation time if provided, otherwise set default. - // The merged credentials can't tell us whether the caller sent the - // field — the raw request credentials can. - if invalidAt := opts.Request.Credentials["previous_secret_invalid_at"]; invalidAt != "" { - creds["previous_secret_invalid_at"] = invalidAt - } else { - creds["previous_secret_invalid_at"] = time.Now().Add(24 * time.Hour).Format(time.RFC3339) - } - - return creds, nil -} - -// updateSecret handles non-rotation updates and returns clean credentials -func (d *StandardWebhookDestination) updateSecret(newDest, origDest *models.Destination, opts *destregistry.PreprocessDestinationOpts) (map[string]string, error) { - creds := make(map[string]string) - - if opts.Role != "admin" { - // For tenants, first check if they're trying to modify any credential fields - if origDest != nil && origDest.Credentials != nil { - // Updating existing destination - must match original values - if newDest.Credentials["secret"] != "" && newDest.Credentials["secret"] != origDest.Credentials["secret"] { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.secret", - Type: "forbidden", - }, - }) - } - if newDest.Credentials["previous_secret"] != "" && newDest.Credentials["previous_secret"] != origDest.Credentials["previous_secret"] { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.previous_secret", - Type: "forbidden", - }, - }) - } - if newDest.Credentials["previous_secret_invalid_at"] != "" && newDest.Credentials["previous_secret_invalid_at"] != origDest.Credentials["previous_secret_invalid_at"] { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.previous_secret_invalid_at", - Type: "forbidden", - }, - }) - } - // Copy original values - for _, key := range []string{"secret", "previous_secret", "previous_secret_invalid_at"} { - if value := origDest.Credentials[key]; value != "" { - creds[key] = value - } - } - } else { - // First time creation - can't set any credentials - if newDest.Credentials["secret"] != "" { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.secret", - Type: "forbidden", - }, - }) - } - if newDest.Credentials["previous_secret"] != "" { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.previous_secret", - Type: "forbidden", - }, - }) - } - if newDest.Credentials["previous_secret_invalid_at"] != "" { - return nil, destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{ - { - Field: "credentials.previous_secret_invalid_at", - Type: "forbidden", - }, - }) - } - } - } else { - // Admin can set any values - for _, key := range []string{"secret", "previous_secret", "previous_secret_invalid_at"} { - if value := newDest.Credentials[key]; value != "" { - creds[key] = value - } - } - } - - return creds, nil -} - -// ensureInitializedCredentials ensures credentials are initialized for new destinations -func (d *StandardWebhookDestination) ensureInitializedCredentials(creds map[string]string) (map[string]string, error) { - // If there are any credentials already, return them as is - if creds["secret"] != "" || creds["previous_secret"] != "" || creds["previous_secret_invalid_at"] != "" { - return creds, nil - } - - // Otherwise generate a new secret - secret, err := generateStandardSecret() - if err != nil { - return nil, err - } - return map[string]string{ - "secret": secret, - }, nil -} - -// validateAndSanitizeCredentials performs final validation and cleanup -func (d *StandardWebhookDestination) validateAndSanitizeCredentials(creds map[string]string) (map[string]string, error) { - // Set default previous_secret_invalid_at if previous_secret is set but invalid_at is not - if creds["previous_secret"] != "" && creds["previous_secret_invalid_at"] == "" { - creds["previous_secret_invalid_at"] = time.Now().Add(24 * time.Hour).Format(time.RFC3339) - } - - // Clean up any extra fields - cleanCreds := make(map[string]string) - for _, key := range []string{"secret", "previous_secret", "previous_secret_invalid_at"} { - if value := creds[key]; value != "" { - cleanCreds[key] = value - } - } - - return cleanCreds, nil -} - -// Preprocess sets a default secret if one isn't provided and handles secret rotation -func (d *StandardWebhookDestination) Preprocess(newDestination *models.Destination, originalDestination *models.Destination, opts *destregistry.PreprocessDestinationOpts) error { - // Initialize credentials if nil - if newDestination.Credentials == nil { - newDestination.Credentials = make(map[string]string) - } - - // Get clean credentials based on operation type - var cleanCredentials map[string]string - var err error - if isTruthy(newDestination.Credentials["rotate_secret"]) { - cleanCredentials, err = d.rotateSecret(originalDestination, opts) - } else { - cleanCredentials, err = d.updateSecret(newDestination, originalDestination, opts) - // For new destinations, ensure credentials are initialized if needed - if err == nil && originalDestination == nil { - cleanCredentials, err = d.ensureInitializedCredentials(cleanCredentials) - } - } - if err != nil { - return err - } - - // Final validation and sanitization - cleanCredentials, err = d.validateAndSanitizeCredentials(cleanCredentials) - if err != nil { - return err - } - - newDestination.Credentials = cleanCredentials - return nil -} - -type StandardWebhookPublisher struct { - *destregistry.BasePublisher - httpClient *http.Client - url string - secrets []destwebhook.WebhookSecret - sm *destwebhook.SignatureManager - headerPrefix string - customHeaders map[string]string - maxResponseBodyBytes int -} - -func (p *StandardWebhookPublisher) Close() error { - p.BasePublisher.StartClose() - return nil -} - -func (p *StandardWebhookPublisher) Publish(ctx context.Context, event *models.Event) (*destregistry.Delivery, error) { - if err := p.BasePublisher.StartPublish(); err != nil { - return nil, err - } - defer p.BasePublisher.FinishPublish() - - httpReq, err := p.Format(ctx, event) - if err != nil { - return destregistry.NewFormatError("webhook_standard", "", err) - } - - result := destwebhook.ExecuteHTTPRequest(ctx, p.httpClient, httpReq, "webhook_standard", p.maxResponseBodyBytes) - return result.Delivery, result.Error -} - -// Format creates an HTTP request formatted according to Standard Webhooks specification -func (p *StandardWebhookPublisher) Format(ctx context.Context, event *models.Event) (*http.Request, error) { - now := time.Now() - rawBody := []byte(event.Data) - - req, err := http.NewRequestWithContext(ctx, "POST", p.url, bytes.NewBuffer(rawBody)) - if err != nil { - return nil, err - } - - req.Header.Set("Content-Type", "application/json") - - // Add custom headers FIRST (so metadata can override if there's a conflict) - for key, value := range p.customHeaders { - req.Header.Set(key, value) - } - - // Use event ID directly as the message ID - // This ensures the same message ID is used across retry attempts - // TODO: Support configurable ID generator/template (e.g., "msg_" prefix) - messageID := event.ID - - // Set Standard Webhooks headers with configurable prefix - req.Header.Set(p.headerPrefix+"id", messageID) - req.Header.Set(p.headerPrefix+"timestamp", strconv.FormatInt(now.Unix(), 10)) - - // Generate and set signature header - signatureHeader, err := p.sm.GenerateSignatureHeader(destwebhook.SignaturePayload{ - EventID: messageID, - Topic: event.Topic, - Timestamp: now, - Body: string(rawBody), - }) - if err != nil { - return nil, err - } - if signatureHeader != "" { - req.Header.Set(p.headerPrefix+"signature", signatureHeader) - } - - // Add event metadata as custom headers - // Get merged metadata (system + event metadata) using BasePublisher - metadata := p.BasePublisher.MakeMetadata(event, now) - for key, value := range metadata { - // Skip system metadata that's already handled by Standard Webhooks headers - // (webhook-id replaces event-id, webhook-timestamp replaces timestamp) - if key == "event-id" || key == "timestamp" { - continue - } - // Add with configured prefix (defaults to "webhook-") - req.Header.Set(p.headerPrefix+key, value) - } - - // Also add custom event metadata without prefix (user-defined metadata) - for key, value := range event.Metadata { - req.Header.Set(key, value) - } - - return req, nil -} - -// isTruthy checks if a string value represents a truthy value -func isTruthy(value string) bool { - switch strings.ToLower(value) { - case "true", "1", "on", "yes": - return true - default: - return false - } -} diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go deleted file mode 100644 index b11377055..000000000 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_config_test.go +++ /dev/null @@ -1,154 +0,0 @@ -package destwebhookstandard_test - -import ( - "context" - "net/url" - "testing" - - "github.com/hookdeck/outpost/internal/destregistry" - "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard" - "github.com/hookdeck/outpost/internal/util/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestStandardWebhookDestination_CustomHeadersConfig(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - t.Run("should parse config with valid custom_headers", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{"x-api-key":"secret123","x-tenant-id":"tenant-abc"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.NoError(t, err) - }) - - t.Run("should accept empty custom_headers object", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.NoError(t, err) - }) - - t.Run("should parse config without custom_headers field (backward compatibility)", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.NoError(t, err) - }) - - t.Run("should fail on invalid custom_headers JSON", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{invalid json}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.Error(t, err) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "config.custom_headers", validationErr.Errors[0].Field) - assert.Equal(t, "invalid", validationErr.Errors[0].Type) - }) -} - -func TestNew(t *testing.T) { - t.Parallel() - - t.Run("creates provider with no options", func(t *testing.T) { - t.Parallel() - provider, err := destwebhookstandard.New(testutil.Registry.MetadataLoader(), nil) - require.NoError(t, err) - assert.NotNil(t, provider) - }) - - t.Run("creates provider with user agent option", func(t *testing.T) { - t.Parallel() - provider, err := destwebhookstandard.New( - testutil.Registry.MetadataLoader(), - nil, - destwebhookstandard.WithUserAgent("test-agent"), - ) - require.NoError(t, err) - assert.NotNil(t, provider) - }) - - t.Run("creates provider with proxy chain option", func(t *testing.T) { - t.Parallel() - provider, err := destwebhookstandard.New( - testutil.Registry.MetadataLoader(), - nil, - destwebhookstandard.WithProxy(mustProxy(t, "http://proxy.example.com")), - ) - require.NoError(t, err) - assert.NotNil(t, provider) - }) - - t.Run("creates provider with header prefix option", func(t *testing.T) { - t.Parallel() - provider, err := destwebhookstandard.New( - testutil.Registry.MetadataLoader(), - nil, - destwebhookstandard.WithHeaderPrefix("x-custom-"), - ) - require.NoError(t, err) - assert.NotNil(t, provider) - }) - - t.Run("creates provider with multiple options", func(t *testing.T) { - t.Parallel() - provider, err := destwebhookstandard.New( - testutil.Registry.MetadataLoader(), - nil, - destwebhookstandard.WithUserAgent("test-agent"), - destwebhookstandard.WithProxy(mustProxy(t, "http://proxy.example.com")), - destwebhookstandard.WithHeaderPrefix("x-outpost-"), - ) - require.NoError(t, err) - assert.NotNil(t, provider) - }) -} - -func mustProxy(t *testing.T, s string) []*url.URL { - t.Helper() - hops, err := destregistry.ParseProxyURL(s) - require.NoError(t, err) - return hops -} diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go deleted file mode 100644 index 7d0fa491d..000000000 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_publish_test.go +++ /dev/null @@ -1,699 +0,0 @@ -package destwebhookstandard_test - -import ( - "context" - "encoding/base64" - "encoding/json" - "io" - "net/http" - "net/http/httptest" - "strconv" - "strings" - "sync" - "testing" - "time" - - "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard" - testsuite "github.com/hookdeck/outpost/internal/destregistry/testing" - "github.com/hookdeck/outpost/internal/models" - "github.com/hookdeck/outpost/internal/util/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" -) - -// StandardWebhookConsumer implements testsuite.MessageConsumer -type StandardWebhookConsumer struct { - server *httptest.Server - messages chan testsuite.Message - wg sync.WaitGroup -} - -func NewStandardWebhookConsumer() *StandardWebhookConsumer { - consumer := &StandardWebhookConsumer{ - messages: make(chan testsuite.Message, 100), - } - - consumer.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - consumer.wg.Add(1) - defer consumer.wg.Done() - - body, err := io.ReadAll(r.Body) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return - } - - // Extract all headers as metadata - metadata := make(map[string]string) - for k, v := range r.Header { - if len(v) > 0 { - headerKey := strings.ToLower(k) - // Map Standard Webhooks headers to expected metadata keys - // Standard Webhooks prefixes all headers with "webhook-", so we need to strip it - if strings.HasPrefix(headerKey, "webhook-") { - // Remove "webhook-" prefix to get the actual metadata key - metadataKey := strings.TrimPrefix(headerKey, "webhook-") - // Map to expected keys - switch metadataKey { - case "id": - metadata["event-id"] = v[0] - case "timestamp": - metadata["timestamp"] = v[0] - default: - metadata[metadataKey] = v[0] - } - } else { - // Keep all non-prefixed headers as-is (user-defined metadata) - metadata[headerKey] = v[0] - } - } - } - - consumer.messages <- testsuite.Message{ - Data: body, - Metadata: metadata, - Raw: r, // Store raw request for detailed assertions - } - - w.WriteHeader(http.StatusOK) - })) - - return consumer -} - -func (c *StandardWebhookConsumer) Consume() <-chan testsuite.Message { - return c.messages -} - -func (c *StandardWebhookConsumer) Close() error { - c.wg.Wait() - c.server.Close() - close(c.messages) - return nil -} - -// StandardWebhookAsserter implements testsuite.MessageAsserter -type StandardWebhookAsserter struct { - secret string - expectedSignatures int - headerPrefix string // Defaults to "webhook-" -} - -func (a *StandardWebhookAsserter) AssertMessage(t testsuite.TestingT, msg testsuite.Message, event models.Event) { - req := msg.Raw.(*http.Request) - - // Verify HTTP properties - assert.Equal(t, "POST", req.Method) - assert.Equal(t, "application/json", req.Header.Get("Content-Type")) - - // Use configured prefix or default to "webhook-" - prefix := a.headerPrefix - if prefix == "" { - prefix = "webhook-" - } - - // Verify Standard Webhooks headers with configured prefix - webhookID := req.Header.Get(prefix + "id") - assert.NotEmpty(t, webhookID, prefix+"id should be present") - // Note: webhook-id format depends on event.ID format (user-provided) - - webhookTimestamp := req.Header.Get(prefix + "timestamp") - assert.NotEmpty(t, webhookTimestamp, prefix+"timestamp should be present") - // Standard Webhooks spec requires Unix seconds for the webhook-timestamp header - _, err := strconv.ParseInt(webhookTimestamp, 10, 64) - assert.NoError(t, err, "webhook-timestamp should be a valid Unix timestamp integer") - - webhookSignature := req.Header.Get(prefix + "signature") - assert.NotEmpty(t, webhookSignature, prefix+"signature should be present") - - // Verify signature format and count - assertSignatureFormat(t, webhookSignature, a.expectedSignatures) - - // Verify signature with known secret (if provided) - if a.secret != "" { - assertValidStandardWebhookSignature(t, a.secret, webhookID, webhookTimestamp, msg.Data, webhookSignature) - } -} - -// StandardWebhookPublishSuite is the test suite -type StandardWebhookPublishSuite struct { - testsuite.PublisherSuite - consumer *StandardWebhookConsumer - setupFn func(*StandardWebhookPublishSuite) -} - -func (s *StandardWebhookPublishSuite) SetupSuite() { - s.setupFn(s) -} - -func (s *StandardWebhookPublishSuite) TearDownSuite() { - if s.consumer != nil { - s.consumer.Close() - } -} - -// Basic publish test configuration -func (s *StandardWebhookPublishSuite) setupBasicSuite() { - consumer := NewStandardWebhookConsumer() - - provider := newTestProvider(s.T()) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - s.InitSuite(testsuite.Config{ - Provider: provider, - Dest: &dest, - Consumer: consumer, - Asserter: &StandardWebhookAsserter{ - secret: "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - expectedSignatures: 1, - }, - }) - - s.consumer = consumer -} - -// Multiple secrets test configuration -func (s *StandardWebhookPublishSuite) setupMultipleSecretsSuite() { - consumer := NewStandardWebhookConsumer() - - provider := newTestProvider(s.T()) - - now := time.Now() - invalidAt := now.Add(24 * time.Hour) - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_TmV3U2VjcmV0QmFzZTY0RW5jb2RlZFN0cmluZzEyMw==", - "previous_secret": "whsec_T2xkU2VjcmV0QmFzZTY0RW5jb2RlZFN0cmluZzEyMw==", - "previous_secret_invalid_at": invalidAt.Format(time.RFC3339), - }), - ) - - s.InitSuite(testsuite.Config{ - Provider: provider, - Dest: &dest, - Consumer: consumer, - Asserter: &StandardWebhookAsserter{ - secret: "whsec_TmV3U2VjcmV0QmFzZTY0RW5jb2RlZFN0cmluZzEyMw==", - expectedSignatures: 2, - }, - }) - - s.consumer = consumer -} - -// Expired secrets test configuration -func (s *StandardWebhookPublishSuite) setupExpiredSecretsSuite() { - consumer := NewStandardWebhookConsumer() - - provider := newTestProvider(s.T()) - - now := time.Now() - invalidAt := now.Add(-1 * time.Hour) // Previous secret is already invalid - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_QWN0aXZlU2VjcmV0QmFzZTY0RW5jb2RlZFN0cmluZzEyMw==", - "previous_secret": "whsec_RXhwaXJlZFNlY3JldEJhc2U2NEVuY29kZWRTdHJpbmcxMjM=", - "previous_secret_invalid_at": invalidAt.Format(time.RFC3339), - }), - ) - - s.InitSuite(testsuite.Config{ - Provider: provider, - Dest: &dest, - Consumer: consumer, - Asserter: &StandardWebhookAsserter{ - secret: "whsec_QWN0aXZlU2VjcmV0QmFzZTY0RW5jb2RlZFN0cmluZzEyMw==", - expectedSignatures: 1, // Only expect signature from active secret - }, - }) - - s.consumer = consumer -} - -func TestStandardWebhookPublish(t *testing.T) { - t.Parallel() - - // Run basic publish tests - t.Run("Basic", func(t *testing.T) { - t.Parallel() - suite.Run(t, &StandardWebhookPublishSuite{ - setupFn: (*StandardWebhookPublishSuite).setupBasicSuite, - }) - }) - - // Run multiple secrets tests - t.Run("MultipleSecrets", func(t *testing.T) { - t.Parallel() - suite.Run(t, &StandardWebhookPublishSuite{ - setupFn: (*StandardWebhookPublishSuite).setupMultipleSecretsSuite, - }) - }) - - // Run expired secrets tests - t.Run("ExpiredSecrets", func(t *testing.T) { - t.Parallel() - suite.Run(t, &StandardWebhookPublishSuite{ - setupFn: (*StandardWebhookPublishSuite).setupExpiredSecretsSuite, - }) - }) -} - -func TestStandardWebhookPublisher_SignatureFormat(t *testing.T) { - t.Parallel() - - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - - provider := newTestProvider(t) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithID("msg_2KWPBgLlAfxdpx2AI54pPJ85f4W"), - testutil.EventFactory.WithDataMap(map[string]interface{}{"hello": "world"}), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - // Get the message - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - - // Verify signature format is "v1," - signatureHeader := req.Header.Get("webhook-signature") - assert.True(t, strings.HasPrefix(signatureHeader, "v1,")) - - // Verify base64 - sigPart := strings.TrimPrefix(signatureHeader, "v1,") - decoded, err := base64.StdEncoding.DecodeString(sigPart) - assert.NoError(t, err) - assert.Equal(t, 32, len(decoded)) // HMAC-SHA256 produces 32 bytes - - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } -} - -func TestStandardWebhookPublisher_MessageIDFormat(t *testing.T) { - t.Parallel() - - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - - provider := newTestProvider(t) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithID("msg_2KWPBgLlAfxdpx2AI54pPJ85f4W"), - testutil.EventFactory.WithDataMap(map[string]interface{}{"test": "data"}), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - // Get the message - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - - // Verify webhook-id uses event ID directly and has msg_ prefix - webhookID := req.Header.Get("webhook-id") - assert.NotEmpty(t, webhookID) - assert.Equal(t, event.ID, webhookID) - assert.True(t, strings.HasPrefix(webhookID, "msg_"), "webhook-id should have msg_ prefix, got: %s", webhookID) - - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } -} - -func TestStandardWebhookPublisher_CustomHeaderPrefix(t *testing.T) { - t.Parallel() - - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - - // Create provider with custom header prefix - provider, err := destwebhookstandard.New( - testutil.Registry.MetadataLoader(), - nil, - destwebhookstandard.WithHeaderPrefix("x-custom-"), - ) - require.NoError(t, err) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithID("msg_2KWPBgLlAfxdpx2AI54pPJ85f4W"), - testutil.EventFactory.WithDataMap(map[string]interface{}{"test": "data"}), - testutil.EventFactory.WithTopic("user.created"), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - // Get the message - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - - // Verify ALL headers use custom prefix (including Standard Webhooks headers) - assert.NotEmpty(t, req.Header.Get("x-custom-id"), "should have x-custom-id header") - assert.NotEmpty(t, req.Header.Get("x-custom-timestamp"), "should have x-custom-timestamp header") - assert.NotEmpty(t, req.Header.Get("x-custom-signature"), "should have x-custom-signature header") - assert.NotEmpty(t, req.Header.Get("x-custom-topic"), "should have x-custom-topic header") - assert.Equal(t, "user.created", req.Header.Get("x-custom-topic")) - - // Verify default prefix is NOT used - assert.Empty(t, req.Header.Get("webhook-id"), "should not have webhook-id header") - assert.Empty(t, req.Header.Get("webhook-timestamp"), "should not have webhook-timestamp header") - assert.Empty(t, req.Header.Get("webhook-signature"), "should not have webhook-signature header") - assert.Empty(t, req.Header.Get("webhook-topic"), "should not have webhook-topic header") - - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } -} - -func TestStandardWebhookPublisher_EmptyHeaderPrefix(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - prefix string - setPrefix bool - want string // expected prefix on headers - }{ - { - name: "no prefix option gives empty", - setPrefix: false, - want: "", - }, - { - name: "explicit default prefix", - prefix: "webhook-", - setPrefix: true, - want: "webhook-", - }, - { - name: "empty string disables prefix", - prefix: "", - setPrefix: true, - want: "", - }, - { - name: "whitespace-only disables prefix", - prefix: " ", - setPrefix: true, - want: "", - }, - { - name: "custom prefix is applied", - prefix: "x-custom-", - setPrefix: true, - want: "x-custom-", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - opts := []destwebhookstandard.Option{} - if tt.setPrefix { - opts = append(opts, destwebhookstandard.WithHeaderPrefix(tt.prefix)) - } - - provider, err := destwebhookstandard.New(testutil.Registry.MetadataLoader(), nil, opts...) - require.NoError(t, err) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "http://example.com/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithID("msg_test123"), - testutil.EventFactory.WithTopic("user.created"), - testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"}), - ) - - req, err := publisher.(*destwebhookstandard.StandardWebhookPublisher).Format(context.Background(), &event) - require.NoError(t, err) - - // Verify headers use the expected prefix - assert.Equal(t, "msg_test123", req.Header.Get(tt.want+"id")) - assert.NotEmpty(t, req.Header.Get(tt.want+"timestamp")) - assert.NotEmpty(t, req.Header.Get(tt.want+"signature")) - assert.Equal(t, "user.created", req.Header.Get(tt.want+"topic")) - }) - } -} - -func TestStandardWebhookPublisher_CustomHeaders(t *testing.T) { - t.Parallel() - - t.Run("should include custom headers in request", func(t *testing.T) { - t.Parallel() - - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - - provider := newTestProvider(t) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - "custom_headers": `{"x-api-key":"secret123","x-tenant-id":"tenant-abc"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"}), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - assert.Equal(t, "secret123", req.Header.Get("x-api-key")) - assert.Equal(t, "tenant-abc", req.Header.Get("x-tenant-id")) - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } - }) - - t.Run("should allow metadata to override custom headers", func(t *testing.T) { - t.Parallel() - - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - - provider := newTestProvider(t) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - "custom_headers": `{"webhook-source":"custom-value"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - testutil.DestinationFactory.WithDeliveryMetadata(map[string]string{ - "source": "delivery-metadata-value", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"}), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - // Metadata should override custom headers (metadata adds prefix webhook-) - assert.Equal(t, "delivery-metadata-value", req.Header.Get("webhook-source")) - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } - }) - - t.Run("should accept CreatePublisher when custom_headers is empty object", func(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "http://example.com/webhook", - "custom_headers": `{}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - }) - - t.Run("should work without custom_headers field", func(t *testing.T) { - t.Parallel() - - consumer := NewStandardWebhookConsumer() - defer consumer.Close() - - provider := newTestProvider(t) - - dest := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": consumer.server.URL + "/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &dest) - require.NoError(t, err) - defer publisher.Close() - - event := testutil.EventFactory.Any( - testutil.EventFactory.WithDataMap(map[string]interface{}{"key": "value"}), - ) - - _, err = publisher.Publish(context.Background(), &event) - require.NoError(t, err) - - select { - case msg := <-consumer.Consume(): - req := msg.Raw.(*http.Request) - // Should still have standard headers - assert.NotEmpty(t, req.Header.Get("webhook-id")) - assert.NotEmpty(t, req.Header.Get("webhook-timestamp")) - case <-time.After(5 * time.Second): - t.Fatal("timeout waiting for message") - } - }) -} - -// TestStandardWebhookPublisher_PreservesKeyOrder verifies that Format() sends -// the original JSON key order in the HTTP request body. -func TestStandardWebhookPublisher_PreservesKeyOrder(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "http://example.com/webhook", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - publisher, err := provider.CreatePublisher(context.Background(), &destination) - require.NoError(t, err) - - rawData := json.RawMessage(`{"z":1,"a":2,"m":3}`) - event := testutil.EventFactory.Any( - testutil.EventFactory.WithData(rawData), - ) - - req, err := publisher.(*destwebhookstandard.StandardWebhookPublisher).Format(context.Background(), &event) - require.NoError(t, err) - - body, err := io.ReadAll(req.Body) - require.NoError(t, err) - - // Key order must match the original raw JSON — not alphabetised. - assert.Equal(t, `{"z":1,"a":2,"m":3}`, string(body)) -} diff --git a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_validate_test.go b/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_validate_test.go deleted file mode 100644 index 115a47191..000000000 --- a/internal/destregistry/providers/destwebhookstandard/destwebhookstandard_validate_test.go +++ /dev/null @@ -1,772 +0,0 @@ -package destwebhookstandard_test - -import ( - "context" - "strings" - "testing" - "time" - - "github.com/hookdeck/outpost/internal/destregistry" - "github.com/hookdeck/outpost/internal/util/maputil" - "github.com/hookdeck/outpost/internal/util/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestStandardWebhookDestination_Validate(t *testing.T) { - t.Parallel() - - validDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - provider := newTestProvider(t) - - t.Run("should validate valid destination", func(t *testing.T) { - t.Parallel() - assert.NoError(t, provider.Validate(context.Background(), &validDestination)) - }) - - t.Run("should validate invalid type", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Type = "invalid" - err := provider.Validate(context.Background(), &invalidDestination) - assert.Error(t, err) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "type", validationErr.Errors[0].Field) - assert.Equal(t, "invalid_type", validationErr.Errors[0].Type) - }) - - t.Run("should validate missing url", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Config = map[string]string{} - err := provider.Validate(context.Background(), &invalidDestination) - - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "config.url", validationErr.Errors[0].Field) - assert.Equal(t, "required", validationErr.Errors[0].Type) - }) - - t.Run("should validate malformed url", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Config = map[string]string{ - "url": "not-a-valid-url", - } - err := provider.Validate(context.Background(), &invalidDestination) - - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "config.url", validationErr.Errors[0].Field) - assert.Equal(t, "pattern", validationErr.Errors[0].Type) - }) - - t.Run("should accept valid URLs", func(t *testing.T) { - t.Parallel() - validURLs := []string{ - // Standard URLs - "https://example.com", - "http://example.com", - "https://example.com/path", - "https://example.com:8080/path", - "https://example.com/path?query=value", - "https://example.com/path#fragment", - "https://sub.example.com/path", - "http://localhost:3000/webhook", - // Basic Auth URLs - "https://user:pass@example.com", - "https://user:pass@example.com/path", - "https://user:pass@example.com:8080/path", - "https://token@example.com/webhook", - "https://sam:123444@example.com/api/message", - // Percent-encoded URLs (Azure Logic Apps, etc.) - "https://example.com/path?param=%2Fencoded%2Fslash", - "https://example.com/path%2Fwith%2Fencoded", - "https://logic.azure.com/workflows/abc123/triggers/manual?api-version=2016&sp=%2Ftriggers%2Fmanual%2Frun", - // IP addresses - "http://192.168.1.1:8080/webhook", - "http://127.0.0.1/webhook", - } - for _, url := range validURLs { - t.Run(url, func(t *testing.T) { - t.Parallel() - dest := validDestination - dest.Config = map[string]string{"url": url} - assert.NoError(t, provider.Validate(context.Background(), &dest)) - }) - } - }) - - t.Run("should reject invalid URLs", func(t *testing.T) { - t.Parallel() - invalidURLs := []string{ - "not-a-url", - "ftp://example.com", - "://missing-scheme.com", - "https://", - "", - "example.com", - } - for _, url := range invalidURLs { - t.Run(url, func(t *testing.T) { - t.Parallel() - dest := validDestination - dest.Config = map[string]string{"url": url} - err := provider.Validate(context.Background(), &dest) - assert.Error(t, err) - }) - } - }) - - t.Run("should validate secret without whsec prefix", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Credentials = map[string]string{ - "secret": "not-a-whsec-secret", - } - err := provider.Validate(context.Background(), &invalidDestination) - - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "credentials.secret", validationErr.Errors[0].Field) - assert.Equal(t, "pattern", validationErr.Errors[0].Type) - }) - - t.Run("should validate secret with invalid base64", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Credentials = map[string]string{ - "secret": "whsec_not-valid-base64!!!", - } - err := provider.Validate(context.Background(), &invalidDestination) - - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "credentials.secret", validationErr.Errors[0].Field) - assert.Equal(t, "pattern", validationErr.Errors[0].Type) - }) - - t.Run("should validate previous_secret without whsec prefix", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Credentials = map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - "previous_secret": "not-a-whsec-secret", - "previous_secret_invalid_at": time.Now().Add(24 * time.Hour).Format(time.RFC3339), - } - err := provider.Validate(context.Background(), &invalidDestination) - - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "credentials.previous_secret", validationErr.Errors[0].Field) - assert.Equal(t, "pattern", validationErr.Errors[0].Type) - }) - - t.Run("should validate previous secret without invalid_at", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Credentials = map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - } - err := provider.Validate(context.Background(), &invalidDestination) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "credentials.previous_secret_invalid_at", validationErr.Errors[0].Field) - assert.Equal(t, "required", validationErr.Errors[0].Type) - }) - - t.Run("should validate malformed previous_secret_invalid_at", func(t *testing.T) { - t.Parallel() - invalidDestination := validDestination - invalidDestination.Credentials = map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - "previous_secret_invalid_at": "not-a-timestamp", - } - err := provider.Validate(context.Background(), &invalidDestination) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "credentials.previous_secret_invalid_at", validationErr.Errors[0].Field) - assert.Equal(t, "pattern", validationErr.Errors[0].Type) - }) - - t.Run("should validate valid destination with previous secret", func(t *testing.T) { - t.Parallel() - validDestWithPrevious := validDestination - validDestWithPrevious.Credentials = map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - "previous_secret_invalid_at": "2024-01-02T00:00:00Z", - } - assert.NoError(t, provider.Validate(context.Background(), &validDestWithPrevious)) - }) -} - -func TestStandardWebhookDestination_ValidateCustomHeaders(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - t.Run("should accept valid header names", func(t *testing.T) { - t.Parallel() - validHeaders := []string{ - "x-api-key", - "X-Custom-Header", - "Authorization", - "x-tenant-id", - "X_Custom_Header", - "x123-header", - } - for _, header := range validHeaders { - t.Run(header, func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{"` + header + `":"value"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.NoError(t, err, "header name %q should be valid", header) - }) - } - }) - - t.Run("should reject invalid header names", func(t *testing.T) { - t.Parallel() - invalidHeaders := []struct { - name string - expectedType string - }{ - {"header with space", "pattern"}, - {"header:colon", "pattern"}, - {"-starts-with-dash", "pattern"}, - {"_starts_with_underscore", "pattern"}, - } - for _, tc := range invalidHeaders { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{"` + tc.name + `":"value"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.Error(t, err, "header name %q should be invalid", tc.name) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, tc.expectedType, validationErr.Errors[0].Type) - }) - } - }) - - t.Run("should reject reserved header names", func(t *testing.T) { - t.Parallel() - reservedHeaders := []string{ - "Content-Type", - "content-type", - "Content-Length", - "Host", - "Connection", - "User-Agent", - } - for _, header := range reservedHeaders { - t.Run(header, func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{"` + header + `":"value"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.Error(t, err, "reserved header %q should be rejected", header) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "forbidden", validationErr.Errors[0].Type) - }) - } - }) - - t.Run("should reject empty header values", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{"x-api-key":""}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.Error(t, err) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "config.custom_headers.x-api-key", validationErr.Errors[0].Field) - assert.Equal(t, "required", validationErr.Errors[0].Type) - }) - - t.Run("should collect multiple validation errors", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - "custom_headers": `{"Content-Type":"application/xml","x-valid":"ok","Host":"example.com"}`, - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - }), - ) - - err := provider.Validate(context.Background(), &destination) - assert.Error(t, err) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - // Should have errors for both Content-Type and Host (reserved headers) - assert.GreaterOrEqual(t, len(validationErr.Errors), 2) - }) -} - -func TestStandardWebhookDestination_ComputeTarget(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - t.Run("should return url as target", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/webhook", - }), - ) - target := provider.ComputeTarget(&destination) - assert.Equal(t, "https://example.com/webhook", target.Target) - }) -} - -func TestStandardWebhookDestination_Preprocess(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - t.Run("should generate default whsec secret if not provided", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - ) - - err := provider.Preprocess(&destination, nil, &destregistry.PreprocessDestinationOpts{Role: "tenant"}) - require.NoError(t, err) - - // Verify that a whsec_ secret was generated - assert.True(t, strings.HasPrefix(destination.Credentials["secret"], "whsec_")) - - // Verify it's valid - assert.NoError(t, provider.Validate(context.Background(), &destination)) - }) - - t.Run("should preserve existing secret for admin", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_CustomSecretBase64EncodedString", - }), - ) - - err := provider.Preprocess(&destination, nil, &destregistry.PreprocessDestinationOpts{Role: "admin"}) - require.NoError(t, err) - - // Verify that the custom secret was preserved - assert.Equal(t, "whsec_CustomSecretBase64EncodedString", destination.Credentials["secret"]) - }) - - t.Run("tenant should not be able to override existing secret", func(t *testing.T) { - t.Parallel() - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_CurrentSecretBase64EncodedString", - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/new", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_CustomSecretBase64EncodedString", - }), - ) - - // Merge both config and credentials to simulate handler behavior - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{Role: "tenant"}) - var validationErr *destregistry.ErrDestinationValidation - assert.ErrorAs(t, err, &validationErr) - assert.Equal(t, "credentials.secret", validationErr.Errors[0].Field) - assert.Equal(t, "forbidden", validationErr.Errors[0].Type) - }) - - t.Run("tenant should be able to rotate secret", func(t *testing.T) { - t.Parallel() - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_CurrentSecretBase64EncodedString", - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/new", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "rotate_secret": "true", - }), - ) - - // Merge both config and credentials to simulate handler behavior - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{Role: "tenant"}) - require.NoError(t, err) - - // Verify that the current secret became the previous secret - assert.Equal(t, "whsec_CurrentSecretBase64EncodedString", newDestination.Credentials["previous_secret"]) - - // Verify that a new secret was generated with whsec_ prefix - assert.NotEqual(t, "whsec_CurrentSecretBase64EncodedString", newDestination.Credentials["secret"]) - assert.True(t, strings.HasPrefix(newDestination.Credentials["secret"], "whsec_")) - assert.NotEmpty(t, newDestination.Credentials["secret"]) - - // Verify that previous_secret_invalid_at was set to ~24h from now - invalidAt, err := time.Parse(time.RFC3339, newDestination.Credentials["previous_secret_invalid_at"]) - require.NoError(t, err) - expectedTime := time.Now().Add(24 * time.Hour) - assert.WithinDuration(t, expectedTime, invalidAt, 5*time.Second) - }) - - t.Run("admin should be able to set previous_secret directly", func(t *testing.T) { - t.Parallel() - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/new", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - }), - ) - - // Merge both config and credentials to simulate handler behavior - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{Role: "admin"}) - require.NoError(t, err) - - // Verify that previous_secret was kept - assert.Equal(t, "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", newDestination.Credentials["previous_secret"]) - }) - - t.Run("should respect custom invalidation time during rotation", func(t *testing.T) { - t.Parallel() - customInvalidAt := time.Now().Add(48 * time.Hour).Format(time.RFC3339) - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_CurrentSecretBase64EncodedString", - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/new", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "rotate_secret": "true", - "previous_secret_invalid_at": customInvalidAt, - }), - ) - - // Merge both config and credentials to simulate handler behavior, - // passing the raw request credentials via opts as the handler does - requestCredentials := newDestination.Credentials - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{ - Request: destregistry.PreprocessRequest{Credentials: requestCredentials}, - }) - require.NoError(t, err) - - // Verify that the custom invalidation time was preserved - assert.Equal(t, customInvalidAt, newDestination.Credentials["previous_secret_invalid_at"]) - }) - - t.Run("should apply 24h default on rotation when request omits invalid_at", func(t *testing.T) { - t.Parallel() - storedInvalidAt := time.Now().Add(1 * time.Hour).Format(time.RFC3339) - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_CurrentSecretBase64EncodedString", - "previous_secret": "whsec_OlderSecretBase64EncodedStringgg", - "previous_secret_invalid_at": storedInvalidAt, - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "rotate_secret": "true", - }), - ) - - // Merge both config and credentials to simulate handler behavior: - // the stored previous_secret_invalid_at is merged into newDestination - // even though the caller's request didn't contain it. - requestCredentials := newDestination.Credentials - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{ - Request: destregistry.PreprocessRequest{Credentials: requestCredentials}, - }) - require.NoError(t, err) - - // The merged-in stored value must not be carried forward; the - // default window applies. - invalidAt, err := time.Parse(time.RFC3339, newDestination.Credentials["previous_secret_invalid_at"]) - require.NoError(t, err) - assert.WithinDuration(t, time.Now().Add(24*time.Hour), invalidAt, 5*time.Second) - }) - - t.Run("should set default previous_secret_invalid_at when previous_secret is provided", func(t *testing.T) { - t.Parallel() - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/new", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - }), - ) - - // Merge both config and credentials to simulate handler behavior - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{Role: "admin"}) - require.NoError(t, err) - - // Verify that previous_secret_invalid_at was set to ~24h from now - invalidAt, err := time.Parse(time.RFC3339, newDestination.Credentials["previous_secret_invalid_at"]) - require.NoError(t, err) - expectedTime := time.Now().Add(24 * time.Hour) - assert.WithinDuration(t, expectedTime, invalidAt, 5*time.Second) - }) - - t.Run("should remove extra fields from credentials map", func(t *testing.T) { - t.Parallel() - originalDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - }), - ) - - newDestination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com/new", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - "previous_secret_invalid_at": time.Now().Add(24 * time.Hour).Format(time.RFC3339), - "extra_field": "should be removed", - "another_extra": "also removed", - "rotate_secret": "false", - }), - ) - - // Merge both config and credentials to simulate handler behavior - newDestination.Config = maputil.MergeStringMaps(originalDestination.Config, newDestination.Config) - newDestination.Credentials = maputil.MergeStringMaps(originalDestination.Credentials, newDestination.Credentials) - - err := provider.Preprocess(&newDestination, &originalDestination, &destregistry.PreprocessDestinationOpts{Role: "admin"}) - require.NoError(t, err) - - // Verify that only expected fields are present - expectedFields := map[string]bool{ - "secret": true, - "previous_secret": true, - "previous_secret_invalid_at": true, - } - - // Check that only expected fields exist - for key := range newDestination.Credentials { - assert.True(t, expectedFields[key], "unexpected field %q found in credentials", key) - } - - // Check that all expected fields are present - assert.Equal(t, len(expectedFields), len(newDestination.Credentials), "credentials map has wrong number of fields") - - // Verify values are preserved for expected fields - assert.Equal(t, "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", newDestination.Credentials["secret"]) - assert.Equal(t, "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", newDestination.Credentials["previous_secret"]) - assert.NotEmpty(t, newDestination.Credentials["previous_secret_invalid_at"]) - }) -} - -func TestStandardWebhookDestination_ObfuscateDestination(t *testing.T) { - t.Parallel() - - provider := newTestProvider(t) - - t.Run("should keep previous_secret when not expired", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - "previous_secret_invalid_at": time.Now().Add(24 * time.Hour).Format(time.RFC3339), - }), - ) - - result := provider.ObfuscateDestination(&destination) - assert.Equal(t, "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", result.Credentials["secret"]) - assert.Equal(t, "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", result.Credentials["previous_secret"]) - assert.NotEmpty(t, result.Credentials["previous_secret_invalid_at"]) - }) - - t.Run("should strip previous_secret when expired", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - "previous_secret": "whsec_T2xkU2VjcmV0U3RyaW5nMTIz", - "previous_secret_invalid_at": time.Now().Add(-1 * time.Hour).Format(time.RFC3339), - }), - ) - - result := provider.ObfuscateDestination(&destination) - assert.Equal(t, "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", result.Credentials["secret"]) - assert.Empty(t, result.Credentials["previous_secret"], - "previous_secret should be stripped when expired") - assert.Empty(t, result.Credentials["previous_secret_invalid_at"], - "previous_secret_invalid_at should be stripped when expired") - }) - - t.Run("should handle destination without previous_secret", func(t *testing.T) { - t.Parallel() - destination := testutil.DestinationFactory.Any( - testutil.DestinationFactory.WithType("webhook"), - testutil.DestinationFactory.WithConfig(map[string]string{ - "url": "https://example.com", - }), - testutil.DestinationFactory.WithCredentials(map[string]string{ - "secret": "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", - }), - ) - - result := provider.ObfuscateDestination(&destination) - assert.Equal(t, "whsec_Q3VycmVudFNlY3JldFN0cmluZw==", result.Credentials["secret"]) - assert.Empty(t, result.Credentials["previous_secret"]) - assert.Empty(t, result.Credentials["previous_secret_invalid_at"]) - }) -} diff --git a/internal/destregistry/providers/destwebhookstandard/secret.go b/internal/destregistry/providers/destwebhookstandard/secret.go deleted file mode 100644 index 1c6c30f25..000000000 --- a/internal/destregistry/providers/destwebhookstandard/secret.go +++ /dev/null @@ -1,62 +0,0 @@ -package destwebhookstandard - -import ( - "crypto/rand" - "encoding/base64" - "fmt" - "strings" -) - -const ( - SecretPrefix = "whsec_" - SecretLength = 32 // 32 bytes = 256 bits -) - -// validateSecret checks if a secret has the correct whsec_ prefix and valid base64 encoding -func validateSecret(secret string) error { - if !strings.HasPrefix(secret, SecretPrefix) { - return fmt.Errorf("secret must have %s prefix", SecretPrefix) - } - - encodedPart := strings.TrimPrefix(secret, SecretPrefix) - if encodedPart == "" { - return fmt.Errorf("secret is empty after prefix") - } - - if _, err := base64.StdEncoding.DecodeString(encodedPart); err != nil { - return fmt.Errorf("secret is not valid base64: %w", err) - } - - return nil -} - -// parseSecret extracts and decodes the secret portion after whsec_ prefix -// Returns the decoded bytes as a string for use with SignatureManager -func parseSecret(secret string) (string, error) { - if err := validateSecret(secret); err != nil { - return "", err - } - - encodedPart := strings.TrimPrefix(secret, SecretPrefix) - decoded, err := base64.StdEncoding.DecodeString(encodedPart) - if err != nil { - return "", fmt.Errorf("failed to decode secret: %w", err) - } - - // Return as string - SignatureManager will convert to []byte for HMAC - return string(decoded), nil -} - -// generateStandardSecret creates a cryptographically secure random secret in Standard Webhooks format -// Format: whsec_ -func generateStandardSecret() (string, error) { - // Generate 32 random bytes (256 bits) - randomBytes := make([]byte, SecretLength) - if _, err := rand.Read(randomBytes); err != nil { - return "", fmt.Errorf("failed to generate random secret: %w", err) - } - - // Encode and prefix - encoded := base64.StdEncoding.EncodeToString(randomBytes) - return SecretPrefix + encoded, nil -} diff --git a/internal/destregistry/providers/destwebhookstandard/secret_test.go b/internal/destregistry/providers/destwebhookstandard/secret_test.go deleted file mode 100644 index ce9d4931a..000000000 --- a/internal/destregistry/providers/destwebhookstandard/secret_test.go +++ /dev/null @@ -1,143 +0,0 @@ -package destwebhookstandard - -import ( - "encoding/base64" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestValidateSecret(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - secret string - wantErr bool - }{ - { - name: "valid whsec secret", - secret: "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - wantErr: false, - }, - { - name: "missing whsec prefix", - secret: "MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - wantErr: true, - }, - { - name: "empty after prefix", - secret: "whsec_", - wantErr: true, - }, - { - name: "invalid base64", - secret: "whsec_not-valid-base64!!!", - wantErr: true, - }, - { - name: "empty string", - secret: "", - wantErr: true, - }, - { - name: "wrong prefix", - secret: "whsk_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := validateSecret(tt.secret) - if tt.wantErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } -} - -func TestParseSecret(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - secret string - wantErr bool - }{ - { - name: "valid whsec secret", - secret: "whsec_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - wantErr: false, - }, - { - name: "invalid prefix", - secret: "invalid_MfKQ9r8GKYqrTwjUPD8ILPZIo2LaLaSw", - wantErr: true, - }, - { - name: "invalid base64", - secret: "whsec_not-valid-base64!!!", - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - result, err := parseSecret(tt.secret) - if tt.wantErr { - assert.Error(t, err) - assert.Empty(t, result) - } else { - assert.NoError(t, err) - assert.NotEmpty(t, result) - - // Verify that the result is the decoded version - encodedPart := strings.TrimPrefix(tt.secret, SecretPrefix) - decoded, _ := base64.StdEncoding.DecodeString(encodedPart) - assert.Equal(t, string(decoded), result) - } - }) - } -} - -func TestGenerateStandardSecret(t *testing.T) { - t.Parallel() - - t.Run("generates valid whsec secret", func(t *testing.T) { - t.Parallel() - secret, err := generateStandardSecret() - require.NoError(t, err) - - // Should have whsec_ prefix - assert.True(t, strings.HasPrefix(secret, SecretPrefix)) - - // Should be valid base64 after prefix - err = validateSecret(secret) - assert.NoError(t, err) - - // Should decode to 32 bytes - encodedPart := strings.TrimPrefix(secret, SecretPrefix) - decoded, err := base64.StdEncoding.DecodeString(encodedPart) - require.NoError(t, err) - assert.Equal(t, SecretLength, len(decoded)) - }) - - t.Run("generates unique secrets", func(t *testing.T) { - t.Parallel() - secret1, err := generateStandardSecret() - require.NoError(t, err) - - secret2, err := generateStandardSecret() - require.NoError(t, err) - - // Should generate different secrets - assert.NotEqual(t, secret1, secret2) - }) -} diff --git a/internal/destregistry/providers/destwebhookstandard/testutil_test.go b/internal/destregistry/providers/destwebhookstandard/testutil_test.go deleted file mode 100644 index 557ba95be..000000000 --- a/internal/destregistry/providers/destwebhookstandard/testutil_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package destwebhookstandard_test - -import ( - "testing" - - "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhookstandard" - "github.com/hookdeck/outpost/internal/util/testutil" - "github.com/stretchr/testify/require" -) - -// newTestProvider creates a standard webhook provider with the default "webhook-" prefix. -// Tests can override specific options by passing additional options. -func newTestProvider(t *testing.T, opts ...destwebhookstandard.Option) *destwebhookstandard.StandardWebhookDestination { - t.Helper() - - baseOpts := []destwebhookstandard.Option{ - destwebhookstandard.WithHeaderPrefix("webhook-"), - } - baseOpts = append(baseOpts, opts...) - - provider, err := destwebhookstandard.New(testutil.Registry.MetadataLoader(), nil, baseOpts...) - require.NoError(t, err) - - return provider -}