From d081875fce3c6b84b603ecb769d24b5a05f02ebe Mon Sep 17 00:00:00 2001 From: Head of Product & Engineering Date: Sat, 4 Apr 2026 17:52:09 +0200 Subject: [PATCH] fix: add uninstall handler, idempotency guard, and OAuth error handling GHL Marketplace submission blockers resolved: - Add POST /api/ghl/v1/webhook/uninstall to delete token on app removal - Add in-memory messageId deduplication (10-min TTL) to prevent duplicate SMS sends on webhook retries - Handle ?error= param in OAuth callback for user-denied auth flows - Pass store to WebhookHandler; update tests accordingly Co-Authored-By: Paperclip --- cmd/server/main.go | 3 +- internal/ghl/oauth.go | 6 +++ internal/ghl/types.go | 5 +++ internal/ghl/webhook.go | 79 +++++++++++++++++++++++++++++++++++- internal/ghl/webhook_test.go | 2 +- 5 files changed, 92 insertions(+), 3 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index 83decf7..18531a5 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -40,7 +40,7 @@ func main() { ghlAPI := ghl.NewAPIClient() oauthHandler := ghl.NewOAuthHandler(cfg.GHLClientID, cfg.GHLClientSecret, cfg.BaseURL, cfg.GHLConversationProviderID, s) - webhookHandler, err := ghl.NewWebhookHandler(cfg.GHLWebhookPublicKey, castClient, ghlAPI, oauthHandler) + webhookHandler, err := ghl.NewWebhookHandler(cfg.GHLWebhookPublicKey, castClient, ghlAPI, oauthHandler, s) if err != nil { slog.Error("failed to initialize webhook handler", "err", err) os.Exit(1) @@ -56,6 +56,7 @@ func main() { r.Get("/install", oauthHandler.HandleInstall) r.Get("/oauth-callback", oauthHandler.HandleCallback) r.Post("/api/ghl/v1/webhook/messages", webhookHandler.HandleWebhook) + r.Post("/api/ghl/v1/webhook/uninstall", webhookHandler.HandleUninstall) srv := &http.Server{ Addr: ":" + cfg.Port, diff --git a/internal/ghl/oauth.go b/internal/ghl/oauth.go index 80fb06e..767d3bf 100644 --- a/internal/ghl/oauth.go +++ b/internal/ghl/oauth.go @@ -68,6 +68,12 @@ func (h *OAuthHandler) HandleInstall(w http.ResponseWriter, r *http.Request) { } func (h *OAuthHandler) HandleCallback(w http.ResponseWriter, r *http.Request) { + if errParam := r.URL.Query().Get("error"); errParam != "" { + slog.Warn("ghl oauth denied by user", "error", errParam) + http.Error(w, "authorization denied: "+errParam, http.StatusBadRequest) + return + } + code := r.URL.Query().Get("code") if code == "" { http.Error(w, "missing authorization code", http.StatusBadRequest) diff --git a/internal/ghl/types.go b/internal/ghl/types.go index 638907a..34b1950 100644 --- a/internal/ghl/types.go +++ b/internal/ghl/types.go @@ -27,6 +27,11 @@ type MessageStatusUpdate struct { ErrorMessage string `json:"error_message,omitempty"` } +type UninstallWebhook struct { + LocationID string `json:"locationId"` + CompanyID string `json:"companyId"` +} + type InboundMessage struct { Type string `json:"type"` Message string `json:"message"` diff --git a/internal/ghl/webhook.go b/internal/ghl/webhook.go index ae0d0c9..1e252fd 100644 --- a/internal/ghl/webhook.go +++ b/internal/ghl/webhook.go @@ -12,20 +12,30 @@ import ( "io" "log/slog" "net/http" + "sync" "time" castclient "git.sds.dev/CAST/cast-ghl-plugin/internal/cast" "git.sds.dev/CAST/cast-ghl-plugin/internal/phone" ) +const seenMessageTTL = 10 * time.Minute + +type seenEntry struct { + at time.Time +} + type WebhookHandler struct { webhookPubKey *ecdsa.PublicKey castClient *castclient.Client ghlAPI *APIClient oauthHandler *OAuthHandler + store TokenStore + seenMu sync.Mutex + seenMessages map[string]seenEntry } -func NewWebhookHandler(pubKeyPEM string, castClient *castclient.Client, ghlAPI *APIClient, oauth *OAuthHandler) (*WebhookHandler, error) { +func NewWebhookHandler(pubKeyPEM string, castClient *castclient.Client, ghlAPI *APIClient, oauth *OAuthHandler, store TokenStore) (*WebhookHandler, error) { key, err := parseECDSAPublicKey(pubKeyPEM) if err != nil { return nil, fmt.Errorf("failed to parse webhook public key: %w", err) @@ -35,9 +45,32 @@ func NewWebhookHandler(pubKeyPEM string, castClient *castclient.Client, ghlAPI * castClient: castClient, ghlAPI: ghlAPI, oauthHandler: oauth, + store: store, + seenMessages: make(map[string]seenEntry), }, nil } +// markSeen returns true if messageID was already seen within seenMessageTTL (duplicate). +// Otherwise records it and returns false. +func (h *WebhookHandler) markSeen(messageID string) bool { + h.seenMu.Lock() + defer h.seenMu.Unlock() + + now := time.Now() + // Evict expired entries on every call to avoid unbounded growth. + for id, e := range h.seenMessages { + if now.Sub(e.at) > seenMessageTTL { + delete(h.seenMessages, id) + } + } + + if _, exists := h.seenMessages[messageID]; exists { + return true + } + h.seenMessages[messageID] = seenEntry{at: now} + return false +} + func (h *WebhookHandler) HandleWebhook(w http.ResponseWriter, r *http.Request) { sigHeader := r.Header.Get("x-wh-signature") @@ -67,6 +100,12 @@ func (h *WebhookHandler) HandleWebhook(w http.ResponseWriter, r *http.Request) { return } + if h.markSeen(webhook.MessageID) { + slog.Warn("webhook: duplicate messageId ignored", "message_id", webhook.MessageID) + w.WriteHeader(http.StatusOK) + return + } + slog.Info("webhook: received outbound SMS", "message_id", webhook.MessageID, "location_id", webhook.LocationID) w.WriteHeader(http.StatusOK) @@ -121,6 +160,44 @@ func (h *WebhookHandler) verifySignature(body []byte, signatureB64 string) bool return ecdsa.VerifyASN1(h.webhookPubKey, hash[:], sigBytes) } +func (h *WebhookHandler) HandleUninstall(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + slog.Error("uninstall: failed to read body", "err", err) + http.Error(w, "failed to read request body", http.StatusInternalServerError) + return + } + + if !h.verifySignature(body, r.Header.Get("x-wh-signature")) { + slog.Warn("uninstall: invalid signature") + http.Error(w, "invalid webhook signature", http.StatusUnauthorized) + return + } + + var payload UninstallWebhook + if err := json.Unmarshal(body, &payload); err != nil { + slog.Error("uninstall: failed to parse payload", "err", err) + http.Error(w, "invalid payload", http.StatusBadRequest) + return + } + + if payload.LocationID == "" { + slog.Error("uninstall: missing locationId") + http.Error(w, "missing locationId", http.StatusBadRequest) + return + } + + ctx := r.Context() + if err := h.store.DeleteToken(ctx, payload.LocationID); err != nil { + slog.Error("uninstall: failed to delete token", "location_id", payload.LocationID, "err", err) + http.Error(w, "failed to process uninstall", http.StatusInternalServerError) + return + } + + slog.Info("uninstall: token deleted", "location_id", payload.LocationID) + w.WriteHeader(http.StatusOK) +} + func parseECDSAPublicKey(pemStr string) (*ecdsa.PublicKey, error) { block, _ := pem.Decode([]byte(pemStr)) if block == nil { diff --git a/internal/ghl/webhook_test.go b/internal/ghl/webhook_test.go index 9e8d12d..d11d29a 100644 --- a/internal/ghl/webhook_test.go +++ b/internal/ghl/webhook_test.go @@ -44,7 +44,7 @@ func newTestHandler(t *testing.T, pubPEM string) *WebhookHandler { t.Helper() ms := &inMemStore{} oauth := NewOAuthHandler("c", "s", "http://x", "p", ms) - handler, err := NewWebhookHandler(pubPEM, castclient.NewClient("http://localhost:1", "k", ""), NewAPIClient(), oauth) + handler, err := NewWebhookHandler(pubPEM, castclient.NewClient("http://localhost:1", "k", ""), NewAPIClient(), oauth, ms) if err != nil { t.Fatalf("failed to create handler: %v", err) }