FFM-11542 Add validation to Targets on auth path (#321)

* FFM-11542 Add validation to Targets on auth path

**What**

- Adds the same validation for Targets that Saas uses on targets in the /client/auth
  path

**Why**

- Without this the Proxy will accept invalid targets from SDKs when they
  authenticate and then log out an error when it trys to forward them on
and Saas rejects them

**Testing**

- Added tests

Author: jcox250

transport/encode_decode.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"regexp"
 
 	"github.com/harness/ff-proxy/v2/domain"
 	proxyservice "github.com/harness/ff-proxy/v2/proxy-service"
@@ -102,6 +103,15 @@ func decodeAuthRequest(c echo.Context) (interface{}, error) {
 	if req.Target.Name == "" {
 		req.Target.Name = req.Target.Identifier
 	}
+
+	if req.Target.Identifier != "" && !isIdentifierValid(req.Target.Identifier) {
+		return nil, fmt.Errorf("%w: target identifier is invalid", errBadRequest)
+	}
+
+	if req.Target.Name != "" && !isNameValid(req.Target.Name) {
+		return nil, fmt.Errorf("%w: target name is invalid", errBadRequest)
+	}
+
 	return req, nil
 }
 
@@ -246,3 +256,26 @@ func decodeMetricsRequest(c echo.Context) (interface{}, error) {
 
 	return req, nil
 }
+
+var (
+	identifierRegex = regexp.MustCompile("^[A-Za-z0-9.@_-]*$")
+	nameRegex       = regexp.MustCompile(`^[\p{L}\d .@_-]*$`)
+)
+
+// IsIdentifierValid determine is an identifier confirms to the required format
+// returns true if the identifier is valid, otherwise this will return false
+func isIdentifierValid(identifier string) bool {
+	if identifier == "" {
+		return false
+	}
+	return identifierRegex.MatchString(identifier)
+}
+
+// IsNameValid determine if the name confirms to the required format
+// returns true if the name is valid, otherwise will return false
+func isNameValid(name string) bool {
+	if name == "" {
+		return false
+	}
+	return nameRegex.MatchString(name)
+}

transport/encode_decode_test.go

@@ -0,0 +1,155 @@
+package transport
+
+import "testing"
+
+func Test_isIdentifierValid(t *testing.T) {
+	type args struct {
+		identifier string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			"Alphanumeric is valid",
+			args{
+				identifier: "TargetName123",
+			},
+			true,
+		},
+		{
+			"Spaces are invalid",
+			args{
+				identifier: "target name",
+			},
+			false,
+		},
+		{
+			"Special characters are invalid",
+			args{
+				identifier: "target$({}><?/",
+			},
+			false,
+		},
+		{
+			"Emails are valid",
+			args{
+				identifier: "[email protected]",
+			},
+			true,
+		},
+		{
+			"Underscore is valid",
+			args{
+				identifier: "__global__cf_target",
+			},
+			true,
+		},
+		{
+			"Dash is valid",
+			args{
+				identifier: "test-user",
+			},
+			true,
+		},
+		{
+			"Single character is valid",
+			args{
+				identifier: "t",
+			},
+			true,
+		},
+		{
+			"Empty string is invalid",
+			args{
+				identifier: "",
+			},
+			false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isIdentifierValid(tt.args.identifier); got != tt.want {
+				t.Errorf("IsIdentifierValid() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_isNameValid(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			"Alphanumeric is valid",
+			args{
+				name: "TargetName123",
+			},
+			true,
+		},
+		{
+			"Spaces are valid",
+			args{
+				name: "Global Target",
+			},
+			true,
+		},
+		{
+			"Special characters are invalid",
+			args{
+				name: "target$({}><?/",
+			},
+			false,
+		},
+		{
+			"Emails are valid",
+			args{
+				name: "[email protected]",
+			},
+			true,
+		},
+		{
+			"Underscore is valid",
+			args{
+				name: "__global__cf_target",
+			},
+			true,
+		},
+		{
+			"Dash is valid",
+			args{
+				name: "test-user",
+			},
+			true,
+		},
+		{
+			"Empty string is invalid",
+			args{
+				name: "",
+			},
+			false,
+		},
+		{
+			"Unicode characters are valid",
+			args{
+				name: "ńoooo",
+			},
+			true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isNameValid(tt.args.name); got != tt.want {
+				t.Errorf("IsIdentifierValid() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

transport/http_server_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/alicebob/miniredis/v2"
 	"github.com/harness-community/sse/v3"
 	sdkstream "github.com/harness/ff-golang-server-sdk/stream"
+	clientgen "github.com/harness/ff-proxy/v2/gen/client"
 	"github.com/labstack/echo/v4"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/redis/go-redis/v9"
@@ -24,7 +25,6 @@ import (
 	"github.com/harness/ff-proxy/v2/cache"
 	"github.com/harness/ff-proxy/v2/config/local"
 	"github.com/harness/ff-proxy/v2/domain"
-	clientgen "github.com/harness/ff-proxy/v2/gen/client"
 	"github.com/harness/ff-proxy/v2/hash"
 	"github.com/harness/ff-proxy/v2/log"
 	"github.com/harness/ff-proxy/v2/middleware"
@@ -1041,6 +1041,16 @@ func TestHTTPServer_PostAuthentication(t *testing.T) {
 			expectedCacheTargets:         targets,
 			expectedClientServiceTargets: []domain.Target{},
 		},
+		"Given I make an auth request with an invalid Target Identifier": {
+			method:             http.MethodPost,
+			body:               []byte(fmt.Sprintf(`{"apiKey": "%s", "target": {"identifier": "hello world"}}`, apiKey1)),
+			expectedStatusCode: http.StatusBadRequest,
+		},
+		"Given I make an auth request with an invalid Target Name": {
+			method:             http.MethodPost,
+			body:               []byte(fmt.Sprintf(`{"apiKey": "%s", "target": {"identifier": "helloworld", "name": "Hello/World"}}`, apiKey1)),
+			expectedStatusCode: http.StatusBadRequest,
+		},
 	}
 
 	mr, err := miniredis.Run()