diff --git a/internal/api/hooks.go b/internal/api/hooks.go index 937568a4d..8fe1d159f 100644 --- a/internal/api/hooks.go +++ b/internal/api/hooks.go @@ -89,11 +89,8 @@ func (a *API) runHTTPHook(hookConfig conf.ExtensibilityPointConfiguration, input return nil, internalServerError("Over size limit") } - start := time.Now() for i := 0; i < defaultHookRetries; i++ { - dur := time.Since(start) // TODO: Add a check on global timeout? hookLog.Infof("invocation attempt: %d", i) - msgID := uuid.Must(uuid.NewV4()) currentTime := time.Now() signatureList, err := generateSignatures(hookConfig.HTTPHookSecrets, msgID, currentTime, inputPayload) @@ -107,8 +104,8 @@ func (a *API) runHTTPHook(hookConfig conf.ExtensibilityPointConfiguration, input } req.Header.Set("Content-Type", "application/json") + req.Header.Set("webhook-id", msgID.String()) - // req.Header.Set("webhook-id", fmt.Sprintf("msg_%s", msgID)) req.Header.Set("webhook-timestamp", fmt.Sprintf("%d", currentTime.Unix())) req.Header.Set("webhook-signature", strings.Join(signatureList, ", ")) @@ -116,23 +113,20 @@ func (a *API) runHTTPHook(hookConfig conf.ExtensibilityPointConfiguration, input rsp, err := client.Do(req) if err != nil { if terr, ok := err.(net.Error); ok && terr.Timeout() { - hookLog.Info("Request timed out") + hookLog.Errorf("Request timed out for attempt %d", i) time.Sleep(defaultTimeout) continue } else if !watcher.gotConn { - hookLog.Info("Failed to establish a connection") + hookLog.Errorf("Failed to establish a connection on attempt %d", i) time.Sleep(defaultTimeout) continue } else { - // We got a connection, but another error occurred. - hookLog.Error("Error making HTTP request: ", err) - return nil, internalServerError("Failed to trigger auth hook").WithInternalError(err) + return nil, internalServerError("Failed to trigger auth hook, error making HTTP request").WithInternalError(err) } } switch rsp.StatusCode { case http.StatusOK, http.StatusNoContent, http.StatusAccepted: - hookLog.Infof("Finished processing webhook in %s", dur) if rsp.Body == nil { return nil, nil } @@ -150,30 +144,23 @@ func (a *API) runHTTPHook(hookConfig conf.ExtensibilityPointConfiguration, input if retryAfterHeader != "" { continue } - // Probably return an error here - return []byte{}, nil - + return []byte{}, tooManyRequestsError("too many requests") case http.StatusBadRequest: - // TODO: Decide if there should be special handling - hookLog.Infof("bad request made") + return nil, badRequestError("bad request error") case http.StatusUnauthorized: - // TODO: See if there's a way to highlight that a jwt wa needed etc - hookLog.Infof("unauthorized") - return []byte{}, nil + return []byte{}, unauthorizedError("unauthorized") default: - hookLog.Infof("Bad response for hook %d in %s", rsp.StatusCode, dur) + return []byte{}, internalServerError("error executing hooks") } } return nil, nil } + func generateSignatures(secrets []string, msgID uuid.UUID, currentTime time.Time, inputPayload []byte) ([]string, error) { var signatureList []string - //TODO: If secret rotation is enabled we use two secrets for _, secret := range secrets { - // Need to decide between Asymmetric and symmetric after implementing the standardwebhooks library if strings.HasPrefix(secret, "v1,") { trimmedSecret := strings.TrimPrefix(secret, "v1,") - fmt.Println(trimmedSecret) wh, err := standardwebhooks.NewWebhook(trimmedSecret) if err != nil { return nil, err @@ -183,11 +170,8 @@ func generateSignatures(secrets []string, msgID uuid.UUID, currentTime time.Time return nil, err } signatureList = append(signatureList, signature) - } else if strings.HasPrefix(secret, "v1") { - // TODO: Handle asymmetric case once library is fixed - return nil, nil } else { - // TODO: return an error here + // TODO(joel): Handle asymmetric case once library has been upgraded return nil, nil } } @@ -213,8 +197,8 @@ func (c *connectionWatcher) GotConn(_ httptrace.GotConnInfo) { } func isOverSizeLimit(payload []byte) bool { - // Check the size of the payload - const maxSizeKB = 20 * 1024 // 20KB in bytes + // As per spec, payloads should be under 20KB https://github.com/standard-webhooks/standard-webhooks/blob/main/spec/standard-webhooks.md#payload-size + const maxSizeKB = 20 * 1024 return len(payload) > maxSizeKB } diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 752c1decd..019b082e9 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -455,7 +455,6 @@ type HookConfiguration struct { type HTTPHookSecrets []string func (h *HTTPHookSecrets) Decode(value string) error { - // TODO: write a quick and simple test for this parts := strings.Split(value, ";") for _, part := range parts { if part != "" {