Make validation turn on by default

Turn on validation on customers and hmac by default
now you must be explicit on turning it off by setting it
to `0` or `false` started by @ivanayov

Signed-off-by: Martin Dekov (VMware) <[email protected]>

github-event/handler.go

@@ -39,7 +39,7 @@ func Handle(req []byte) string {
 		return fmt.Sprintf("%s cannot handle event: %s", Source, eventHeader)
 	}
 
-	if readBool("validate_customers") {
+	if ValidateCustomers() {
 		customersURL := os.Getenv("customers_url")
 
 		customers, getErr := getCustomers(customersURL)
@@ -91,8 +91,7 @@ func Handle(req []byte) string {
 		eventHeader == "installation_repositories" ||
 		eventHeader == "integration_installation" {
 
-		shouldValidate := os.Getenv("validate_hmac")
-		if len(shouldValidate) > 0 && (shouldValidate == "1" || shouldValidate == "true") {
+		if HmacEnabled() {
 			webhookSecretKey, secretErr := sdk.ReadSecret("github-webhook-secret")
 			if secretErr != nil {
 				return secretErr.Error()
@@ -324,3 +323,17 @@ func readBool(key string) bool {
 	}
 	return false
 }
+
+func HmacEnabled() bool {
+	if val, exists := os.LookupEnv("validate_hmac"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
+}
+
+func ValidateCustomers() bool {
+	if val, exists := os.LookupEnv("validate_customers"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
+}

github-push/handler.go

@@ -42,8 +42,7 @@ func Handle(req []byte) string {
 
 	xHubSignature := os.Getenv("Http_X_Hub_Signature")
 
-	shouldValidate := readBool("validate_hmac")
-	if shouldValidate {
+	if HmacEnabled() {
 		webhookSecretKey, secretErr := sdk.ReadSecret("github-webhook-secret")
 		if secretErr != nil {
 			return secretErr.Error()
@@ -169,6 +168,9 @@ func reportGitHubStatus(status *sdk.Status) {
 	}
 }
 
-func init() {
-
+func HmacEnabled() bool {
+	if val, exists := os.LookupEnv("validate_hmac"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
 }

github-push/handler_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/openfaas/openfaas-cloud/sdk"
@@ -19,6 +20,7 @@ func (h HTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 func Test_Handle_Push_InvalidBranch(t *testing.T) {
 	audit = sdk.NilLogger{}
 	os.Setenv("Http_X_Github_Event", "push")
+	os.Setenv("validate_hmac", "false")
 	os.Setenv("validate_customers", "false")
 
 	res := Handle([]byte(
@@ -55,21 +57,19 @@ func Test_Handle_IssueComment(t *testing.T) {
 		t.Fail()
 	}
 }
+
 func Test_Handle_ValidateCustomers_Matched(t *testing.T) {
 	server := httptest.NewServer(&HTTPHandler{})
-
 	os.Setenv("Http_X_Github_Event", "push")
 	os.Setenv("validate_customers", "true")
 	os.Setenv("customers_url", server.URL)
-
 	res := Handle([]byte(
 		`{"ref":"refs/heads/master","repository":{ "owner": { "login": "alexellis" } }}`,
 	))
-
 	// This error is as far as we can get right now without subbing more code.
-	want := "unable to read secret: /var/openfaas/secrets/payload-secret, error: open /var/openfaas/secrets/payload-secret: no such file or directory"
-	if res != want {
-		t.Errorf("want error: \"%s\", got: \"%s\"", want, res)
+	secretErr := "unable to read secret"
+	if !strings.Contains(res, secretErr) {
+		t.Errorf("want error: \"%s\", got: \"%s\"", secretErr, res)
 		t.Fail()
 	}
 }

github-status/handler.go

@@ -35,8 +35,7 @@ var (
 // function along with the function stack by sending
 // commit statuses to GitHub on pending, failure or success
 func Handle(req []byte) string {
-
-	if hmacEnabled() {
+	if HmacEnabled() {
 
 		key, keyErr := sdk.ReadSecret("payload-secret")
 		if keyErr != nil {
@@ -317,10 +316,6 @@ func getCheckRunDescription(status *sdk.CommitStatus, url *string) *string {
 	return &status.Description
 }
 
-func hmacEnabled() bool {
-	return os.Getenv("validate_hmac") == "1" || os.Getenv("validate_hmac") == "true"
-}
-
 func buildStatus(status string, desc string, context string, url string) *github.RepoStatus {
 	return &github.RepoStatus{State: &status, TargetURL: &url, Description: &desc, Context: &context}
 }
@@ -362,3 +357,10 @@ func formatLog(logs string, maxCheckMessageLength int) string {
 
 	return logValue
 }
+
+func HmacEnabled() bool {
+	if val, exists := os.LookupEnv("validate_hmac"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
+}

import-secrets/handler.go

@@ -23,7 +23,7 @@ import (
 func Handle(req []byte) string {
 	event := getEventFromHeader()
 
-	if hmacEnabled() {
+	if HmacEnabled() {
 		key, err := sdk.ReadSecret("payload-secret")
 		if err != nil {
 			fmt.Fprintf(os.Stderr, err.Error())
@@ -119,10 +119,6 @@ func Handle(req []byte) string {
 	return fmt.Sprintf("Imported SealedSecret: %s as new object", name)
 }
 
-func hmacEnabled() bool {
-	return os.Getenv("validate_hmac") == "1" || os.Getenv("validate_hmac") == "true"
-}
-
 func updateEncryptedData(ss *ssv1alpha1.SealedSecret, userSecret *SealedSecret) error {
 	for k, v := range userSecret.Spec.EncryptedData {
 		encodedBytes, err := base64.StdEncoding.DecodeString(v)
@@ -159,3 +155,10 @@ type SealedSecret struct {
 	Metadata   *metav1.ObjectMeta `yaml:"metadata"`
 	Spec       SealedSecretSpec   `yaml:"spec"`
 }
+
+func HmacEnabled() bool {
+	if val, exists := os.LookupEnv("validate_hmac"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
+}

sdk/customers.go

@@ -0,0 +1,13 @@
+package sdk
+
+import "os"
+
+// ValidateCustomers checks environmental
+// variable validate_customers if customer
+// validation is explicitly disabled
+func ValidateCustomers() bool {
+	if val, exists := os.LookupEnv("validate_customers"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
+}

sdk/customers_test.go

@@ -0,0 +1,55 @@
+package sdk
+
+import (
+	"os"
+	"testing"
+)
+
+func Test_ValidateCustomers(t *testing.T) {
+	tests := []struct {
+		title        string
+		value        string
+		expectedBool bool
+	}{
+		{
+			title:        "environmental variable `validate_customers` is unset",
+			value:        "",
+			expectedBool: true,
+		},
+		{
+			title:        "environmental variable `validate_customers` is set to true",
+			value:        "true",
+			expectedBool: true,
+		},
+		{
+			title:        "environmental variable `validate_customers` is set to 1",
+			value:        "1",
+			expectedBool: true,
+		},
+		{
+			title:        "environmental variable `validate_customers` is set with random value",
+			value:        "random",
+			expectedBool: true,
+		},
+		{
+			title:        "environmental variable `validate_customers` is set with explicit `0`",
+			value:        "0",
+			expectedBool: false,
+		},
+		{
+			title:        "environmental variable `validate_customers` is set with explicit `false`",
+			value:        "false",
+			expectedBool: false,
+		},
+	}
+	customersEnvVar := "validate_customers"
+	for _, test := range tests {
+		t.Run(test.title, func(t *testing.T) {
+			os.Setenv(customersEnvVar, test.value)
+			value := ValidateCustomers()
+			if value != test.expectedBool {
+				t.Errorf("Expected value: %v got: %v", test.expectedBool, value)
+			}
+		})
+	}
+}

sdk/hmac.go

@@ -8,9 +8,12 @@ import (
 )
 
 // HmacEnabled uses validate_hmac env-var to verify if the
-// feature is enabled
+// feature is disabled
 func HmacEnabled() bool {
-	return os.Getenv("validate_hmac") == "1" || os.Getenv("validate_hmac") == "true"
+	if val, exists := os.LookupEnv("validate_hmac"); exists {
+		return val != "false" && val != "0"
+	}
+	return true
 }
 
 // ValidHMAC returns an error if HMAC could not be validated or if
@@ -32,3 +35,10 @@ func validHMACWithSecretKey(payload *[]byte, secretText string, digest string) e
 	}
 	return nil
 }
+
+func readBool(key string) bool {
+	if val, exists := os.LookupEnv(key); exists {
+		return val != "false" && val != "0"
+	}
+	return true
+}

sdk/hmac_test.go

@@ -3,6 +3,7 @@ package sdk
 import (
 	"encoding/hex"
 	"fmt"
+	"os"
 	"testing"
 
 	"github.com/alexellis/hmac"
@@ -35,3 +36,42 @@ func Test_validHMACWithSecretKey_invalidSecret(t *testing.T) {
 		t.Fail()
 	}
 }
+
+func Test_HmacEnabled(t *testing.T) {
+	tests := []struct {
+		title        string
+		value        string
+		expectedBool bool
+	}{
+		{
+			title:        "environmental variable `validate_hmac` is unset",
+			value:        "",
+			expectedBool: true,
+		},
+		{
+			title:        "environmental variable `validate_hmac` is set with random value",
+			value:        "random",
+			expectedBool: true,
+		},
+		{
+			title:        "environmental variable `validate_hmac` is set with explicit `0`",
+			value:        "0",
+			expectedBool: false,
+		},
+		{
+			title:        "environmental variable `validate_hmac` is set with explicit `false`",
+			value:        "false",
+			expectedBool: false,
+		},
+	}
+	hmacEnvVar := "validate_hmac"
+	for _, test := range tests {
+		t.Run(test.title, func(t *testing.T) {
+			os.Setenv(hmacEnvVar, test.value)
+			value := HmacEnabled()
+			if value != test.expectedBool {
+				t.Errorf("Expected value: %v got: %v", test.expectedBool, value)
+			}
+		})
+	}
+}

stack.yml

@@ -6,7 +6,7 @@ functions:
   system-github-event:
     lang: go
     handler: ./github-event
-    image: functions/github-event:0.7.0
+    image: functions/github-event:0.7.1
     labels:
       openfaas-cloud: "1"
       role: openfaas-system
@@ -25,7 +25,7 @@ functions:
   github-push:
     lang: go
     handler: ./github-push
-    image: functions/github-push:0.7.0
+    image: functions/github-push:0.7.1
     labels:
       openfaas-cloud: "1"
       role: openfaas-system
@@ -107,7 +107,7 @@ functions:
   github-status:
     lang: go
     handler: ./github-status
-    image: functions/github-status:0.3.4
+    image: functions/github-status:0.3.5
     labels:
       openfaas-cloud: "1"
       role: openfaas-system
@@ -127,7 +127,7 @@ functions:
   import-secrets:
     lang: go
     handler: ./import-secrets
-    image: functions/import-secrets:0.3.1
+    image: functions/import-secrets:0.3.2
     labels:
       openfaas-cloud: "1"
       role: openfaas-system