Adds more tests for missing headers

Author: gnikyt

http_shopify_webhook.go

@@ -9,6 +9,19 @@ import (
 	"net/http"
 )
 
+// HTTP errors.
+const (
+	errMissingSignature = "Missing signature from request"
+	errMissingShop      = "Missing shop from request"
+	errSignature        = "Invalid webhook signature"
+)
+
+// HTTP headers.
+const (
+	headerHmac = "X-Shopify-Hmac-Sha256"
+	headerShop = "X-Shopify-Shop-Domain"
+)
+
 // Public webhook verify wrapper.
 // Can be used with any framework tapping into net/http.
 // Simply pass in the secret key for the Shopify app.
@@ -27,18 +40,18 @@ func WebhookVerify(key string, fn http.HandlerFunc) http.HandlerFunc {
 // Pass in the secret key for the Shopify app and the next handler.`
 func WebhookVerifyRequest(key string, w http.ResponseWriter, r *http.Request) bool {
 	// HMAC from request headers and the shop.
-	shmac := r.Header.Get("X-Shopify-Hmac-Sha256")
-	shop := r.Header.Get("X-Shopify-Shop-Domain")
+	shmac := r.Header.Get(headerHmac)
+	shop := r.Header.Get(headerShop)
 
 	if shop == "" {
 		// No shop provided.
-		http.Error(w, "Missing shop domain", http.StatusBadRequest)
+		httpError(errMissingShop, http.StatusBadRequest, w)
 		return false
 	}
 
 	if shmac == "" {
 		// No HMAC provided.
-		http.Error(w, "Missing signature", http.StatusBadRequest)
+		httpError(errMissingSignature, http.StatusBadRequest, w)
 		return false
 	}
 
@@ -52,7 +65,7 @@ func WebhookVerifyRequest(key string, w http.ResponseWriter, r *http.Request) bo
 
 	// Verify all is ok.
 	if ok := isValidSignature(lhmac, shmac); !ok {
-		http.Error(w, "Invalid webhook signature", http.StatusBadRequest)
+		httpError(errSignature, http.StatusBadRequest, w)
 		return false
 	}
 	return true
@@ -71,3 +84,8 @@ func newSignature(key string, bb []byte) string {
 func isValidSignature(lhmac string, shmac string) bool {
 	return lhmac == shmac
 }
+
+// Common place to issue an error to the response writer.
+func httpError(msg string, code int, w http.ResponseWriter) {
+	http.Error(w, msg, code)
+}

http_shopify_webhook_test.go

@@ -5,23 +5,24 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
 // Test base verification function works.
-func TestVerifyRequest(t *testing.T) {
+func TestIsValidSignatureSuccess(t *testing.T) {
 	// Setup a simple body with a matching HMAC.
 	body := []byte(`{"key":"value"}`)
 	hmac := "7iASoA8WSbw19M/h+lgrLr2ly/LvgnE9bcLsk9gflvs="
 
 	// Create a signature
 	lhmac := newSignature("secret", body)
 	if ok := isValidSignature(lhmac, hmac); !ok {
-		t.Errorf("expected request data to verify")
+		t.Error("expected request data to verify")
 	}
 }
 
-func TestVerifyRequestError(t *testing.T) {
+func TestIsValidSignatureFailure(t *testing.T) {
 	// Setup a simple body with a matching HMAC, but missing shop.
 	body := []byte(`{"key":"value"}`)
 	hmac := "ee2012a00f1649bc35f4cfe1fa582b2ebda5cbf2ef82713d6dc2ec93d81f96fb"
@@ -38,7 +39,7 @@ func TestVerifyRequestError(t *testing.T) {
 	// Create a signature
 	lhmac = newSignature("secret", body)
 	if ok := isValidSignature(lhmac, hmac); ok {
-		t.Errorf("expected request data to not verify, but it did")
+		t.Error("expected request data to not verify, but it did")
 	}
 }
 
@@ -53,11 +54,11 @@ func TestNetHttpSuccess(t *testing.T) {
 	// Setup the server with our data.
 	rec, ran := setupServer(key, shop, hmac, body)
 	if c := rec.Code; c != http.StatusOK {
-		t.Errorf("expected status code %v got %v", http.StatusOK, c)
+		t.Errorf("expected status code %d got %v", http.StatusOK, c)
 	}
 
 	if !ran {
-		t.Errorf("expected next handler to run but did not")
+		t.Error("expected next handler to run but did not")
 	}
 }
 
@@ -72,11 +73,55 @@ func TestNetHttpFailure(t *testing.T) {
 	// Setup the server with our data.
 	rec, ran := setupServer(key, shop, hmac, body)
 	if c := rec.Code; c != http.StatusBadRequest {
-		t.Errorf("expected status code %v got %v", http.StatusBadRequest, c)
+		t.Errorf("expected status code %d got %v", http.StatusBadRequest, c)
+	}
+
+	if ran == true {
+		t.Error("expected next handler to not run but it did")
+	}
+}
+
+// Test for missing HMAC header from request.
+func TestMissingHeaderHMAC(t *testing.T) {
+	// Set our data.
+	key := "secret"
+	body := `{"key":"value"}`
+	shop := "example.myshopify.com"
+
+	// Setup the server with our data. No shop.
+	rec, ran := setupServer(key, shop, "", body)
+	if c := rec.Code; c != http.StatusBadRequest {
+		t.Errorf("expected status code %d got %v", http.StatusBadRequest, c)
+	}
+
+	if b := rec.Body; !strings.Contains(b.String(), errMissingSignature) {
+		t.Errorf("expected '%s' body got '%v'", errMissingSignature, b)
+	}
+
+	if ran == true {
+		t.Error("expected next handler to not run but it did")
+	}
+}
+
+// Test for missing shop header from request.
+func TestMissingHeaderShop(t *testing.T) {
+	// Set our data.
+	key := "secret"
+	body := `{"key":"value"}`
+	hmac := "ee2012a00f1649bc35f"
+
+	// Setup the server with our data. No shop.
+	rec, ran := setupServer(key, "", hmac, body)
+	if c := rec.Code; c != http.StatusBadRequest {
+		t.Errorf("expected status code %d got %v", http.StatusBadRequest, c)
+	}
+
+	if b := rec.Body; !strings.Contains(b.String(), errMissingShop) {
+		t.Errorf("expected '%s' body got '%v'", errMissingShop, b)
 	}
 
 	if ran == true {
-		t.Errorf("expected next handler to not run but it did")
+		t.Error("expected next handler to not run but it did")
 	}
 }