From 67dddb645dbe405fd8d94f59f118608abd42a6a2 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Tue, 3 May 2022 14:06:06 -0700 Subject: [PATCH] =?UTF-8?q?updated=20v2=20register=20webhook=20endpoint=20?= =?UTF-8?q?to=20not=20enforce=20stricter=20validati=E2=80=A6=20(#277)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * updated v2 register webhook endpoint to not enforce stricter validation, only loopback * updated changelog * fix log * added nil check --- CHANGELOG.md | 6 ++- main.go | 28 ++++++++++-- primaryHandler.go | 109 +++++++++++++++++++++++++--------------------- 3 files changed, 89 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c07fdd25..727d9199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +## [v0.7.4] +- Updated v2 hook endpoint to only enforce loopback validation (when configured). [#277](https://github.com/xmidt-org/tr1d1um/pull/277) + ## [v0.7.3] - Bumped ancla to v0.3.9 to fix Duration bug in webhook registration - Duration should be an int in seconds. It will also accept strings such as "5m". [#270](https://github.com/xmidt-org/tr1d1um/pull/270) - Updated v2 webhook registration to allow for no Duration or Until set. [#270](https://github.com/xmidt-org/tr1d1um/pull/270) @@ -142,7 +145,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Initial creation. -[Unreleased]: https://github.com/xmidt-org/tr1d1um/compare/v0.7.3...HEAD +[Unreleased]: https://github.com/xmidt-org/tr1d1um/compare/v0.7.4...HEAD +[v0.7.4]: https://github.com/xmidt-org/tr1d1um/compare/v0.7.3...v0.7.4 [v0.7.3]: https://github.com/xmidt-org/tr1d1um/compare/v0.7.2...v0.7.3 [v0.7.2]: https://github.com/xmidt-org/tr1d1um/compare/v0.7.1...v0.7.2 [v0.7.1]: https://github.com/xmidt-org/tr1d1um/compare/v0.7.0...v0.7.1 diff --git a/main.go b/main.go index 6de873a2..53cba38b 100644 --- a/main.go +++ b/main.go @@ -181,6 +181,10 @@ func tr1d1um(arguments []string) (exitCode int) { } defer stopWatch() + getAllWebhooksHandler := ancla.NewGetAllWebhooksHandler(svc, ancla.HandlerConfig{ + GetLogger: getLogger, + }) + builtValidators, err := ancla.BuildValidators(webhookConfig.Validation) if err != nil { fmt.Fprintf(os.Stderr, "Failed to initialize webhook validators: %s\n", err.Error()) @@ -193,11 +197,29 @@ func tr1d1um(arguments []string) (exitCode int) { GetLogger: getLogger, }) - getAllWebhooksHandler := ancla.NewGetAllWebhooksHandler(svc, ancla.HandlerConfig{ - GetLogger: getLogger, + //build validators and webhook handler for previous version that only check loopback. + v2Validation := ancla.ValidatorConfig{ + URL: ancla.URLVConfig{ + AllowLoopback: webhookConfig.Validation.URL.AllowLoopback, + AllowIP: true, + AllowSpecialUseHosts: true, + AllowSpecialUseIPs: true, + }, + TTL: webhookConfig.Validation.TTL, + } + v2Validators, err := ancla.BuildValidators(v2Validation) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to initialize v2 webhook validators: %s\n", err.Error()) + return 1 + } + + v2AddWebhookHandler := ancla.NewAddWebhookHandler(svc, ancla.HandlerConfig{ + V: v2Validators, + DisablePartnerIDs: webhookConfig.DisablePartnerIDs, + GetLogger: getLogger, }) - fixV2Middleware, err := fixV2Duration(getLogger, webhookConfig.Validation.TTL) + fixV2Middleware, err := fixV2Duration(getLogger, webhookConfig.Validation.TTL, v2AddWebhookHandler) if err != nil { fmt.Fprintf(os.Stderr, "Failed to initialize v2 endpoint middleware: %v\n", err) return 1 diff --git a/primaryHandler.go b/primaryHandler.go index 481f976e..46b4143b 100644 --- a/primaryHandler.go +++ b/primaryHandler.go @@ -246,7 +246,7 @@ func authenticationHandler(v *viper.Viper, logger log.Logger, registry xmetrics. } //nolint:funlen -func fixV2Duration(getLogger ancla.GetLoggerFunc, config ancla.TTLVConfig) (alice.Constructor, error) { +func fixV2Duration(getLogger ancla.GetLoggerFunc, config ancla.TTLVConfig, v2Handler http.Handler) (alice.Constructor, error) { if getLogger == nil { getLogger = func(_ context.Context) log.Logger { return nil @@ -267,68 +267,77 @@ func fixV2Duration(getLogger ancla.GetLoggerFunc, config ancla.TTLVConfig) (alic return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) + // if this isn't v2, do nothing. + if vars == nil || vars["version"] != prevAPIVersion { + next.ServeHTTP(w, r) + return + } + // if this is v2, we need to unmarshal and check the duration. If - // the duration is bad, change it to 5m and add a header. - if vars != nil && vars["version"] == prevAPIVersion { - logger := getLogger(r.Context()) - if logger == nil { - logger = log.NewNopLogger() - } - requestPayload, err := ioutil.ReadAll(r.Body) - if err != nil { - v2ErrEncode(w, logger, err, 0) - return - } + // the duration is bad, change it to 5m and add a header. Then use + // the v2 handler. + logger := getLogger(r.Context()) + if logger == nil { + logger = log.NewNopLogger() + } + requestPayload, err := ioutil.ReadAll(r.Body) + if err != nil { + v2ErrEncode(w, logger, err, 0) + return + } - var wr ancla.WebhookRegistration - err = json.Unmarshal(requestPayload, &wr) - if err != nil { - var e *json.UnmarshalTypeError - if errors.As(err, &e) { - v2ErrEncode(w, logger, - fmt.Errorf("%w: %v must be of type %v", errFailedWebhookUnmarshal, e.Field, e.Type), - http.StatusBadRequest) - return - } - v2ErrEncode(w, logger, fmt.Errorf("%w: %v", errFailedWebhookUnmarshal, err), + var wr ancla.WebhookRegistration + err = json.Unmarshal(requestPayload, &wr) + if err != nil { + var e *json.UnmarshalTypeError + if errors.As(err, &e) { + v2ErrEncode(w, logger, + fmt.Errorf("%w: %v must be of type %v", errFailedWebhookUnmarshal, e.Field, e.Type), http.StatusBadRequest) return } + v2ErrEncode(w, logger, fmt.Errorf("%w: %v", errFailedWebhookUnmarshal, err), + http.StatusBadRequest) + return + } - // check to see if the Webhook has a valid until/duration. - // If not, set the WebhookRegistration duration to 5m. - webhook := wr.ToWebhook() - if webhook.Until.IsZero() { - if webhook.Duration == 0 { - wr.Duration = ancla.CustomDuration(config.Max) - w.Header().Add(v2WarningHeader, - fmt.Sprintf("Unset duration and until fields will not be accepted in v3, webhook duration defaulted to %v", config.Max)) - } else { - durationErr := durationCheck(webhook) - if durationErr != nil { - wr.Duration = ancla.CustomDuration(config.Max) - w.Header().Add(v2WarningHeader, - fmt.Sprintf("Invalid duration will not be accepted in v3: %v, webhook duration defaulted to %v", durationErr, config.Max)) - } - } + // check to see if the Webhook has a valid until/duration. + // If not, set the WebhookRegistration duration to 5m. + webhook := wr.ToWebhook() + if webhook.Until.IsZero() { + if webhook.Duration == 0 { + wr.Duration = ancla.CustomDuration(config.Max) + w.Header().Add(v2WarningHeader, + fmt.Sprintf("Unset duration and until fields will not be accepted in v3, webhook duration defaulted to %v", config.Max)) } else { - untilErr := untilCheck(webhook) - if untilErr != nil { - wr.Until = time.Time{} + durationErr := durationCheck(webhook) + if durationErr != nil { wr.Duration = ancla.CustomDuration(config.Max) w.Header().Add(v2WarningHeader, - fmt.Sprintf("Invalid until value will not be accepted in v3: %v, webhook duration defaulted to 5m", untilErr)) + fmt.Sprintf("Invalid duration will not be accepted in v3: %v, webhook duration defaulted to %v", durationErr, config.Max)) } } - - // put the body back in the request - body, err := json.Marshal(wr) - if err != nil { - v2ErrEncode(w, logger, fmt.Errorf("failed to recreate request body: %v", err), 0) + } else { + untilErr := untilCheck(webhook) + if untilErr != nil { + wr.Until = time.Time{} + wr.Duration = ancla.CustomDuration(config.Max) + w.Header().Add(v2WarningHeader, + fmt.Sprintf("Invalid until value will not be accepted in v3: %v, webhook duration defaulted to 5m", untilErr)) } - r.Body = ioutil.NopCloser(bytes.NewBuffer(body)) } - next.ServeHTTP(w, r) + + // put the body back in the request + body, err := json.Marshal(wr) + if err != nil { + v2ErrEncode(w, logger, fmt.Errorf("failed to recreate request body: %v", err), 0) + } + r.Body = ioutil.NopCloser(bytes.NewBuffer(body)) + + if v2Handler == nil { + v2Handler = next + } + v2Handler.ServeHTTP(w, r) }) }, nil }