Refactor verifySignature for re-use

api/webhook/taskcluster.go

@@ -6,9 +6,6 @@ package webhook
 
 import (
 	"context"
-	"crypto/hmac"
-	"crypto/sha1"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -42,29 +39,14 @@ func tcWebhookHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	ctx := shared.NewAppEngineContext(r)
-	log := shared.GetLogger(ctx)
-
-	payload, err := ioutil.ReadAll(r.Body)
-	r.Body.Close()
+	payload, err := verifyAndGetPayload(r)
 	if err != nil {
-		log.Errorf("%v", err)
-		http.Error(w, err.Error(), http.StatusInternalServerError)
-		return
-	}
-
-	secret, err := getSecret(ctx)
-	if err != nil {
-		log.Errorf("%v", err)
-		http.Error(w, err.Error(), http.StatusInternalServerError)
-		return
-	}
-
-	if !verifySignature(payload, r.Header.Get("X-Hub-Signature"), secret) {
-		http.Error(w, "HMAC verification failed", http.StatusUnauthorized)
+		http.Error(w, err.Error(), http.StatusUnauthorized)
 		return
 	}
 
+	ctx := appengine.NewContext(r)
+	log := shared.GetLogger(ctx)
 	log.Debugf("GitHub Delivery: %s", r.Header.Get("X-GitHub-Delivery"))
 
 	processed, err := handleStatusEvent(ctx, payload)
@@ -267,26 +249,6 @@ func getAuth(ctx context.Context) (username string, password string, err error)
 	return u.Username, u.Password, err
 }
 
-func getSecret(ctx context.Context) (token string, err error) {
-	var t shared.Token
-	key := datastore.NewKey(ctx, "Token", "github-tc-webhook-secret", 0, nil)
-	err = datastore.Get(ctx, key, &t)
-	return t.Secret, err
-}
-
-func verifySignature(message []byte, signature string, secret string) bool {
-	// https://developer.github.com/webhooks/securing/
-	signature = strings.TrimPrefix(signature, "sha1=")
-	messageMAC, err := hex.DecodeString(signature)
-	if err != nil {
-		return false
-	}
-	mac := hmac.New(sha1.New, []byte(secret))
-	mac.Write(message)
-	expectedMAC := mac.Sum(nil)
-	return hmac.Equal(messageMAC, expectedMAC)
-}
-
 func createAllRuns(log shared.Logger,
 	client *http.Client,
 	api,

api/webhook/taskcluster_test.go

@@ -112,23 +112,6 @@ func TestExtractResultURLs(t *testing.T) {
 	}, urls)
 }
 
-func TestVerifySignature(t *testing.T) {
-	message := []byte("test")
-	signature := "a053ee211b4693456ef071e336f74ab699250318"
-	secret := "95bfab9afa719185ee7c3658356b166b7f45349a"
-	assert.True(t, verifySignature(
-		message, signature, secret))
-	assert.False(t, verifySignature(
-		[]byte("foobar"), signature, secret))
-	assert.False(t, verifySignature(
-		message, "875a5feef4cde4265d6d5d21c304d755903ccb60", secret))
-	assert.False(t, verifySignature(
-		message, signature, "875a5feef4cde4265d6d5d21c304d755903ccb60"))
-	// Test an ill-formed (odd-length) signature.
-	assert.False(t, verifySignature(
-		message, "875a5feef4cde4265d6d5d21c304d755903ccb6", secret))
-}
-
 func TestCreateAllRuns_success(t *testing.T) {
 	var requested uint32
 	requested = 0

api/webhook/verify.go

@@ -0,0 +1,49 @@
+package webhook
+
+import (
+	"crypto/hmac"
+	"crypto/sha1"
+	"encoding/hex"
+	"errors"
+	"io/ioutil"
+	"net/http"
+	"strings"
+
+	"github.com/web-platform-tests/wpt.fyi/shared"
+)
+
+func verifyAndGetPayload(r *http.Request) ([]byte, error) {
+	ctx := shared.NewAppEngineContext(r)
+	log := shared.GetLogger(ctx)
+
+	payload, err := ioutil.ReadAll(r.Body)
+	r.Body.Close()
+	if err != nil {
+		log.Errorf("Failed to read request body: %s", err.Error())
+		return nil, errors.New("Failed to read request body")
+	}
+
+	secret, err := shared.GetSecret(ctx, "github-tc-webhook-secret")
+	if err != nil {
+		log.Errorf("Failed to get verification secret: %s", err.Error())
+		return nil, errors.New("Internal error")
+	}
+
+	if !verifySignature(payload, r.Header.Get("X-Hub-Signature"), secret) {
+		return nil, errors.New("HMAC verification failed")
+	}
+	return payload, nil
+}
+
+func verifySignature(message []byte, signature string, secret string) bool {
+	// https://developer.github.com/webhooks/securing/
+	signature = strings.TrimPrefix(signature, "sha1=")
+	messageMAC, err := hex.DecodeString(signature)
+	if err != nil {
+		return false
+	}
+	mac := hmac.New(sha1.New, []byte(secret))
+	mac.Write(message)
+	expectedMAC := mac.Sum(nil)
+	return hmac.Equal(messageMAC, expectedMAC)
+}

api/webhook/verify_test.go

@@ -0,0 +1,25 @@
+// +build small
+
+package webhook
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestVerifySignature(t *testing.T) {
+	message := []byte("test")
+	signature := "a053ee211b4693456ef071e336f74ab699250318"
+	secret := "95bfab9afa719185ee7c3658356b166b7f45349a"
+	assert.True(t, verifySignature(message, signature, secret))
+
+	assert.False(t, verifySignature([]byte("foobar"), signature, secret))
+	assert.False(t, verifySignature(
+		message, "875a5feef4cde4265d6d5d21c304d755903ccb60", secret))
+	assert.False(t, verifySignature(
+		message, signature, "875a5feef4cde4265d6d5d21c304d755903ccb60"))
+	// Test an ill-formed (odd-length) signature.
+	assert.False(t, verifySignature(
+		message, "875a5feef4cde4265d6d5d21c304d755903ccb6", secret))
+}

shared/datastore.go

@@ -279,3 +279,14 @@ func IsFeatureEnabled(ctx context.Context, flagName string) bool {
 	}
 	return flag.Enabled
 }
+
+// GetSecret is a helper wrapper for loading a token's secret from the datastore
+// by name.
+func GetSecret(ctx context.Context, tokenName string) (string, error) {
+	key := datastore.NewKey(ctx, "Token", tokenName, 0, nil)
+	var token Token
+	if err := datastore.Get(ctx, key, &token); err != nil {
+		return "", err
+	}
+	return token.Secret, nil
+}

shared/datastore_medium_test.go

@@ -442,3 +442,22 @@ func TestIsFeatureEnabled(t *testing.T) {
 	datastore.Put(ctx, key, &shared.Flag{Enabled: true})
 	assert.True(t, shared.IsFeatureEnabled(ctx, flagName))
 }
+
+func TestGetSecret(t *testing.T) {
+	i, err := sharedtest.NewAEInstance(true)
+	assert.Nil(t, err)
+	defer i.Close()
+	r, err := i.NewRequest("GET", "/", nil)
+	assert.Nil(t, err)
+	tokenName := "foo"
+	ctx := shared.NewAppEngineContext(r)
+	key := datastore.NewKey(ctx, "Token", tokenName, 0, nil)
+	secret, err := shared.GetSecret(ctx, tokenName)
+	assert.NotNil(t, err)
+	assert.Equal(t, "", secret)
+	// Token.
+	datastore.Put(ctx, key, &shared.Token{Secret: "bar"})
+	secret, err = shared.GetSecret(ctx, tokenName)
+	assert.Nil(t, err)
+	assert.Equal(t, "bar", secret)
+}