From a1d88a6cdda26b96c342c18d4fd33730f0870233 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Oliveirinha?= <joliveirinha@cloudflare.com>
Date: Wed, 9 Nov 2022 12:12:37 +0000
Subject: [PATCH] TUN-6927: Refactor validate access configuration to allow
 empty audTags only

---
 ingress/ingress.go      | 16 ++++------------
 ingress/ingress_test.go | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/ingress/ingress.go b/ingress/ingress.go
index b4600453..8905fb6d 100644
--- a/ingress/ingress.go
+++ b/ingress/ingress.go
@@ -175,18 +175,10 @@ func validateAccessConfiguration(cfg *config.AccessConfig) error {
 		return nil
 	}
 
-	// It is possible to set `required:true` and not have these two configured yet.
-	// But if one of them is configured, we'd validate for correctness.
-	if len(cfg.AudTag) == 0 && cfg.TeamName == "" {
-		return nil
-	}
-
-	if len(cfg.AudTag) == 0 {
-		return errors.New("access audtag cannot be empty")
-	}
-
-	if cfg.TeamName == "" {
-		return errors.New("access.TeamName cannot be blank")
+	// we allow for an initial setup where user can force Access but not configure the rest of the keys.
+	// however, if the user specified audTags but forgot teamName, we should alert it.
+	if cfg.TeamName == "" && len(cfg.AudTag) > 0 {
+		return errors.New("access.TeamName cannot be blank when access.audTags are present")
 	}
 
 	return nil
diff --git a/ingress/ingress_test.go b/ingress/ingress_test.go
index 1e09fac7..dd54c1c0 100644
--- a/ingress/ingress_test.go
+++ b/ingress/ingress_test.go
@@ -674,6 +674,46 @@ ingress:
 	}
 }
 
+func TestParseAccessConfig(t *testing.T) {
+	tests := []struct {
+		name        string
+		cfg         config.AccessConfig
+		expectError bool
+	}{
+		{
+			name:        "Config required with teamName only",
+			cfg:         config.AccessConfig{Required: true, TeamName: "team"},
+			expectError: false,
+		},
+		{
+			name:        "required false",
+			cfg:         config.AccessConfig{Required: false},
+			expectError: false,
+		},
+		{
+			name:        "required true but empty config",
+			cfg:         config.AccessConfig{Required: true},
+			expectError: false,
+		},
+		{
+			name:        "complete config",
+			cfg:         config.AccessConfig{Required: true, TeamName: "team", AudTag: []string{"a"}},
+			expectError: false,
+		},
+		{
+			name:        "required true with audTags but no teamName",
+			cfg:         config.AccessConfig{Required: true, AudTag: []string{"a"}},
+			expectError: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := validateAccessConfiguration(&test.cfg)
+			require.Equal(t, err != nil, test.expectError)
+		})
+	}
+}
+
 func MustReadIngress(s string) *config.Configuration {
 	var conf config.Configuration
 	err := yaml.Unmarshal([]byte(s), &conf)