Merge pull request #229 from moorereason/tidyup

Minor Housecleaning PR
2025-05-28 08:12:28 +00:00 · 2018-02-19 11:00:17 +01:00 · 2018-02-19 11:00:17 +01:00 · b449793825
commit b449793825
parent 4f9ed434c5 66a9e48e39
4 changed files with 106 additions and 117 deletions
--- a/hook/hook.go
+++ b/hook/hook.go
@ -11,8 +11,8 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"math"
 	"log"
+	"math"
 	"net"
 	"net/textproto"
 	"os"
@ -94,9 +94,7 @@ func (e *ParseError) Error() string {

 // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload
 func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) {
-	if strings.HasPrefix(signature, "sha1=") {
-		signature = signature[5:]
-	}
+	signature = strings.TrimPrefix(signature, "sha1=")

 	mac := hmac.New(sha1.New, []byte(secret))
 	_, err := mac.Write(payload)
@ -113,9 +111,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str

 // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload
 func CheckPayloadSignature256(payload []byte, secret string, signature string) (string, error) {
-	if strings.HasPrefix(signature, "sha256=") {
-		signature = signature[7:]
-	}
+	signature = strings.TrimPrefix(signature, "sha256=")

 	mac := hmac.New(sha256.New, []byte(secret))
 	_, err := mac.Write(payload)
@ -129,44 +125,44 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) (
 	}
 	return expectedMAC, err
 }
-
-func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) {
- 	// Check for the signature and date headers
- 	if _, ok := headers["X-Signature"]; !ok {
- 		return false, nil
- 	}
- 	if _, ok := headers["Date"]; !ok {
- 		return false, nil
- 	}
- 	providedSignature := headers["X-Signature"].(string)
- 	dateHeader := headers["Date"].(string)
- 	mac := hmac.New(sha1.New, []byte(signingKey))
- 	mac.Write(body)
- 	mac.Write([]byte(dateHeader))
- 	expectedSignature := hex.EncodeToString(mac.Sum(nil))
- 
- 	if !hmac.Equal([]byte(providedSignature), []byte(expectedSignature)) {
- 		return false, &SignatureError{providedSignature}
- 	}
-
-	if !checkDate {
-		return true, nil
-	}
-  // Example format: Fri 08 Sep 2017 11:24:32 UTC
-  date, err := time.Parse("Mon 02 Jan 2006 15:04:05 MST", dateHeader)
-  //date, err := time.Parse(time.RFC1123, dateHeader)	
-	if err != nil {
-		return false, err
-	}
-	now := time.Now()
-	delta := math.Abs(now.Sub(date).Seconds())
-
-	if delta > 300 {
-		return false, &SignatureError{"outdated"}
-	}
-	return true, nil
- }
- 
+
+func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) {
+	// Check for the signature and date headers
+	if _, ok := headers["X-Signature"]; !ok {
+		return false, nil
+	}
+	if _, ok := headers["Date"]; !ok {
+		return false, nil
+	}
+	providedSignature := headers["X-Signature"].(string)
+	dateHeader := headers["Date"].(string)
+	mac := hmac.New(sha1.New, []byte(signingKey))
+	mac.Write(body)
+	mac.Write([]byte(dateHeader))
+	expectedSignature := hex.EncodeToString(mac.Sum(nil))
+
+	if !hmac.Equal([]byte(providedSignature), []byte(expectedSignature)) {
+		return false, &SignatureError{providedSignature}
+	}
+
+	if !checkDate {
+		return true, nil
+	}
+	// Example format: Fri 08 Sep 2017 11:24:32 UTC
+	date, err := time.Parse("Mon 02 Jan 2006 15:04:05 MST", dateHeader)
+	//date, err := time.Parse(time.RFC1123, dateHeader)
+	if err != nil {
+		return false, err
+	}
+	now := time.Now()
+	delta := math.Abs(now.Sub(date).Seconds())
+
+	if delta > 300 {
+		return false, &SignatureError{"outdated"}
+	}
+	return true, nil
+}
+
 // CheckIPWhitelist makes sure the provided remote address (of the form IP:port) falls within the provided IP range
 // (in CIDR form or a single IP address).
 func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) {
@ -196,7 +192,7 @@ func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) {

 	ipRange = strings.TrimSpace(ipRange)

-	if strings.Index(ipRange, "/") == -1 {
+	if !strings.Contains(ipRange, "/") {
 		ipRange = ipRange + "/32"
 	}

@ -687,7 +683,7 @@ func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body
 		}

 		res = res && rv
-		if res == false {
+		if !res {
 			return res, nil
 		}
 	}
@ -709,7 +705,7 @@ func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body *
 		}

 		res = res || rv
-		if res == true {
+		if res {
 			return res, nil
 		}
 	}
@ -751,10 +747,10 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod
 	if r.Type == IPWhitelist {
 		return CheckIPWhitelist(remoteAddr, r.IPRange)
 	}
-	if r.Type == ScalrSignature {
-		return CheckScalrSignature(*headers, *body, r.Secret, true)
+	if r.Type == ScalrSignature {
+		return CheckScalrSignature(*headers, *body, r.Secret, true)
 	}
-	
+
 	if arg, ok := r.Parameter.Get(headers, query, payload); ok {
 		switch r.Type {
 		case MatchValue:
--- a/hook/hook_test.go
+++ b/hook/hook_test.go
@ -60,52 +60,52 @@ func TestCheckPayloadSignature256(t *testing.T) {
 	}
 }

-var checkScalrSignatureTests = []struct {
-	description       string
-	headers           map[string]interface{}
-	payload           []byte
-	secret            string
-	expectedSignature string
-	ok                bool
-}{
-	{
-		"Valid signature",
-		map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "48e395e38ac48988929167df531eb2da00063a7d"},
-		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
-		"48e395e38ac48988929167df531eb2da00063a7d", true,
-	},
-	{
-		"Wrong signature",
-		map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "999395e38ac48988929167df531eb2da00063a7d"},
-		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
-		"48e395e38ac48988929167df531eb2da00063a7d", false,
-	},
-	{
-		"Missing Date header",
-		map[string]interface{}{"X-Signature": "999395e38ac48988929167df531eb2da00063a7d"},
-		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
-		"48e395e38ac48988929167df531eb2da00063a7d", false,
-	},
-	{
-		"Missing X-Signature header",
-		map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC"},
-		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
-		"48e395e38ac48988929167df531eb2da00063a7d", false,
-	},
-}
-
-func TestCheckScalrSignature(t *testing.T) {
-	for _, testCase := range checkScalrSignatureTests {
-		valid, err := CheckScalrSignature(testCase.headers, testCase.payload, testCase.secret, false)
-		if valid != testCase.ok {
-			t.Errorf("failed to check scalr signature fot test case: %s\nexpected ok:%#v, got ok:%#v}",
-				testCase.description, testCase.ok, valid)
-		}
-
-		if err != nil && strings.Contains(err.Error(), testCase.expectedSignature) {
-			t.Errorf("error message should not disclose expected mac: %s on test case %s", err, testCase.description)
-		}
-	}
+var checkScalrSignatureTests = []struct {
+	description       string
+	headers           map[string]interface{}
+	payload           []byte
+	secret            string
+	expectedSignature string
+	ok                bool
+}{
+	{
+		"Valid signature",
+		map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "48e395e38ac48988929167df531eb2da00063a7d"},
+		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
+		"48e395e38ac48988929167df531eb2da00063a7d", true,
+	},
+	{
+		"Wrong signature",
+		map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "999395e38ac48988929167df531eb2da00063a7d"},
+		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
+		"48e395e38ac48988929167df531eb2da00063a7d", false,
+	},
+	{
+		"Missing Date header",
+		map[string]interface{}{"X-Signature": "999395e38ac48988929167df531eb2da00063a7d"},
+		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
+		"48e395e38ac48988929167df531eb2da00063a7d", false,
+	},
+	{
+		"Missing X-Signature header",
+		map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC"},
+		[]byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+",
+		"48e395e38ac48988929167df531eb2da00063a7d", false,
+	},
+}
+
+func TestCheckScalrSignature(t *testing.T) {
+	for _, testCase := range checkScalrSignatureTests {
+		valid, err := CheckScalrSignature(testCase.headers, testCase.payload, testCase.secret, false)
+		if valid != testCase.ok {
+			t.Errorf("failed to check scalr signature fot test case: %s\nexpected ok:%#v, got ok:%#v}",
+				testCase.description, testCase.ok, valid)
+		}
+
+		if err != nil && strings.Contains(err.Error(), testCase.expectedSignature) {
+			t.Errorf("error message should not disclose expected mac: %s on test case %s", err, testCase.description)
+		}
+	}
 }

 var extractParameterTests = []struct {
--- a/test/hookecho.go
+++ b/test/hookecho.go
@ -5,8 +5,8 @@ package main
 import (
 	"fmt"
 	"os"
-	"strings"
 	"strconv"
+	"strings"
 )

 func main() {
--- a/webhook.go
+++ b/webhook.go
@ -120,7 +120,7 @@ func main() {

 	newHooksFiles := hooksFiles[:0]
 	for _, filePath := range hooksFiles {
-		if _, ok := loadedHooksFromFiles[filePath]; ok == true {
+		if _, ok := loadedHooksFromFiles[filePath]; ok {
 			newHooksFiles = append(newHooksFiles, filePath)
 		}
 	}
@ -250,10 +250,9 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 		}

 		// handle hook
-		if errors := matchedHook.ParseJSONParameters(&headers, &query, &payload); errors != nil {
-			for _, err := range errors {
-				log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err)
-			}
+		errors := matchedHook.ParseJSONParameters(&headers, &query, &payload)
+		for _, err := range errors {
+			log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err)
 		}

 		var ok bool
@ -264,7 +263,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 			ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr)
 			if err != nil {
 				msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err)
-				log.Printf(msg)
+				log.Print(msg)
 				w.WriteHeader(http.StatusInternalServerError)
 				fmt.Fprintf(w, "Error occurred while evaluating hook rules.")
 				return
@ -341,27 +340,21 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in
 	cmd.Dir = h.CommandWorkingDirectory

 	cmd.Args, errors = h.ExtractCommandArguments(headers, query, payload)
-	if errors != nil {
-		for _, err := range errors {
-			log.Printf("[%s] error extracting command arguments: %s\n", rid, err)
-		}
+	for _, err := range errors {
+		log.Printf("[%s] error extracting command arguments: %s\n", rid, err)
 	}

 	var envs []string
 	envs, errors = h.ExtractCommandArgumentsForEnv(headers, query, payload)

-	if errors != nil {
-		for _, err := range errors {
-			log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err)
-		}
+	for _, err := range errors {
+		log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err)
 	}

 	files, errors := h.ExtractCommandArgumentsForFile(headers, query, payload)

-	if errors != nil {
-		for _, err := range errors {
-			log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err)
-		}
+	for _, err := range errors {
+		log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err)
 	}

 	for i := range files {
@ -436,7 +429,7 @@ func reloadHooks(hooksFilePath string) {
 				}
 			}

-			if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] == true {
+			if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] {
 				log.Printf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!", hook.ID)
 				log.Println("reverting hooks back to the previous configuration")
 				return