From f7411b41c97e63a26928702524061f87010502e8 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Oct 2024 22:33:00 +0000 Subject: [PATCH] feat: [CODE-2500]: add support for webhook internal url (#2806) * feat: [CODE-2500]: add support for webhook internal url * feat: [CODE-2500]: add support for webhook internal url * feat: [CODE-2500]: add support for webhook internal url --- app/api/controller/webhook/common.go | 7 ++++++- app/api/controller/webhook/create.go | 11 ++++++++--- app/api/controller/webhook/update.go | 3 ++- app/services/migrate/webhook.go | 2 +- app/services/webhook/service.go | 3 +++ app/services/webhook/trigger.go | 16 ++++++++++++++-- cli/operations/server/config.go | 1 + types/config.go | 2 ++ 8 files changed, 37 insertions(+), 8 deletions(-) diff --git a/app/api/controller/webhook/common.go b/app/api/controller/webhook/common.go index 5525bf03e..a1fd477f4 100644 --- a/app/api/controller/webhook/common.go +++ b/app/api/controller/webhook/common.go @@ -33,7 +33,12 @@ const ( var ErrInternalWebhookOperationNotAllowed = usererror.Forbidden("changes to internal webhooks are not allowed") // CheckURL validates the url of a webhook. -func CheckURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool) error { +func CheckURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool, internal bool) error { + // for internal webhooks skip URL validation as it is not used + if internal { + return nil + } + // check URL if len(rawURL) > webhookMaxURLLength { return check.NewValidationErrorf("The URL of a webhook can be at most %d characters long.", diff --git a/app/api/controller/webhook/create.go b/app/api/controller/webhook/create.go index 45c693e31..90cf40c35 100644 --- a/app/api/controller/webhook/create.go +++ b/app/api/controller/webhook/create.go @@ -56,7 +56,7 @@ func (c *Controller) Create( internal bool, ) (*types.Webhook, error) { // validate input - err := sanitizeCreateInput(in, c.allowLoopback, c.allowPrivateNetwork || internal) + err := sanitizeCreateInput(in, c.allowLoopback, c.allowPrivateNetwork, internal) if err != nil { return nil, err } @@ -121,7 +121,12 @@ func (c *Controller) Create( return hook, nil } -func sanitizeCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork bool) error { +func sanitizeCreateInput( + in *CreateInput, + allowLoopback bool, + allowPrivateNetwork bool, + internal bool, +) error { // TODO [CODE-1363]: remove after identifier migration. if in.Identifier == "" { in.Identifier = in.UID @@ -149,7 +154,7 @@ func sanitizeCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwor if err := check.Description(in.Description); err != nil { return err } - if err := CheckURL(in.URL, allowLoopback, allowPrivateNetwork); err != nil { + if err := CheckURL(in.URL, allowLoopback, allowPrivateNetwork, internal); err != nil { return err } if err := checkSecret(in.Secret); err != nil { diff --git a/app/api/controller/webhook/update.go b/app/api/controller/webhook/update.go index 929a503aa..a7bb7f018 100644 --- a/app/api/controller/webhook/update.go +++ b/app/api/controller/webhook/update.go @@ -125,7 +125,8 @@ func sanitizeUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwor } } if in.URL != nil { - if err := CheckURL(*in.URL, allowLoopback, allowPrivateNetwork); err != nil { + // internal is set to false as internal webhooks cannot be updated + if err := CheckURL(*in.URL, allowLoopback, allowPrivateNetwork, false); err != nil { return err } } diff --git a/app/services/migrate/webhook.go b/app/services/migrate/webhook.go index 4173218f5..ed9a2c4f9 100644 --- a/app/services/migrate/webhook.go +++ b/app/services/migrate/webhook.go @@ -117,7 +117,7 @@ func sanitizeWebhook( return err } - if err := webhookpkg.CheckURL(in.Target, allowLoopback, allowPrivateNetwork); err != nil { + if err := webhookpkg.CheckURL(in.Target, allowLoopback, allowPrivateNetwork, false); err != nil { return err } diff --git a/app/services/webhook/service.go b/app/services/webhook/service.go index 11e20a0aa..dfc2455ac 100644 --- a/app/services/webhook/service.go +++ b/app/services/webhook/service.go @@ -47,6 +47,9 @@ type Config struct { MaxRetries int AllowPrivateNetwork bool AllowLoopback bool + + // InternalWebhooksURL specifies the internal webhook URL which will be used if webhook is marked internal + InternalWebhooksURL string } func (c *Config) Prepare() error { diff --git a/app/services/webhook/trigger.go b/app/services/webhook/trigger.go index 361a39d5f..db11eda6f 100644 --- a/app/services/webhook/trigger.go +++ b/app/services/webhook/trigger.go @@ -285,8 +285,7 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr // NOTE: if the body is an io.Reader, the value is used as response body as is, otherwise it'll be JSON serialized. func (s *Service) prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, triggerType enum.WebhookTrigger, webhook *types.Webhook, body any) (*http.Request, error) { - // set URL as is (already has been validated, any other error will be caught in request creation) - execution.Request.URL = webhook.URL + execution.Request.URL = s.getWebhookURL(ctx, webhook) // Serialize body before anything else. // This allows the user to retrigger the execution even in case of bad URL. @@ -369,6 +368,19 @@ func (s *Service) prepareHTTPRequest(ctx context.Context, execution *types.Webho return req, nil } +func (s *Service) getWebhookURL(ctx context.Context, webhook *types.Webhook) string { + // in case a webhook is internal use the specified URL if not empty instead of using one saved in db. + if webhook.Internal && s.config.InternalWebhooksURL != "" { + return s.config.InternalWebhooksURL + } + + if webhook.Internal && s.config.InternalWebhooksURL == "" { + log.Ctx(ctx).Error().Msg("internal webhook URL is empty, falling back to one in db") + } + // set URL as is (already has been validated, any other error will be caught in request creation) + return webhook.URL +} + func (s *Service) toXHeader(name string) string { return fmt.Sprintf("X-%s-%s", s.config.HeaderIdentity, name) } diff --git a/cli/operations/server/config.go b/cli/operations/server/config.go index 7ac8baf71..9dc041d82 100644 --- a/cli/operations/server/config.go +++ b/cli/operations/server/config.go @@ -331,6 +331,7 @@ func ProvideWebhookConfig(config *types.Config) webhook.Config { MaxRetries: config.Webhook.MaxRetries, AllowPrivateNetwork: config.Webhook.AllowPrivateNetwork, AllowLoopback: config.Webhook.AllowLoopback, + InternalWebhooksURL: config.Webhook.InternalWebhooksURL, } } diff --git a/types/config.go b/types/config.go index 2cffac410..975265549 100644 --- a/types/config.go +++ b/types/config.go @@ -339,6 +339,8 @@ type Config struct { AllowLoopback bool `envconfig:"GITNESS_WEBHOOK_ALLOW_LOOPBACK" default:"false"` // RetentionTime is the duration after which webhook executions will be purged from the DB. RetentionTime time.Duration `envconfig:"GITNESS_WEBHOOK_RETENTION_TIME" default:"168h"` // 7 days + // InternalWebhooksURL is the url for webhooks which are marked as internal + InternalWebhooksURL string `envconfig:"GITNESS_WEBHOOK_INTERNAL_WEBHOOKS_URL"` } Trigger struct {