From 24417d6e7c7661c1a288a1f01502af17fdb54e64 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Peter=20Scha=CC=88fer?=
 <101886095+PeterSchafer@users.noreply.github.com>
Date: Tue, 13 Aug 2024 17:39:43 +0200
Subject: [PATCH] fix: ensure environment variable precedence for auth tokens

---
 cliv2/cmd/cliv2/configuration.go              | 32 +++++++++++++++++++
 cliv2/cmd/cliv2/main.go                       | 22 ++-----------
 .../acceptance/cli-token-precedence.spec.ts   | 14 +++++++-
 3 files changed, 47 insertions(+), 21 deletions(-)
 create mode 100644 cliv2/cmd/cliv2/configuration.go

diff --git a/cliv2/cmd/cliv2/configuration.go b/cliv2/cmd/cliv2/configuration.go
new file mode 100644
index 0000000000..98c9310698
--- /dev/null
+++ b/cliv2/cmd/cliv2/configuration.go
@@ -0,0 +1,32 @@
+package main
+
+// !!! This import needs to be the first import, please do not change this !!!
+import _ "github.com/snyk/go-application-framework/pkg/networking/fips_enable"
+
+import (
+	"os"
+
+	"github.com/snyk/go-application-framework/pkg/auth"
+	"github.com/snyk/go-application-framework/pkg/configuration"
+)
+
+func defaultOAuthFF(config configuration.Configuration) configuration.DefaultValueFunction {
+	return func(existingValue interface{}) interface{} {
+		if _, ok := os.LookupEnv(auth.CONFIG_KEY_OAUTH_TOKEN); ok {
+			return true
+		}
+
+		keysThatMightDisableOAuth := config.GetAllKeysThatContainValues(configuration.AUTHENTICATION_BEARER_TOKEN)
+		alternativeTokenKeys := config.GetAllKeysThatContainValues(configuration.AUTHENTICATION_TOKEN)
+		keysThatMightDisableOAuth = append(keysThatMightDisableOAuth, alternativeTokenKeys...)
+
+		for _, key := range keysThatMightDisableOAuth {
+			keyType := config.GetKeyType(key)
+			if keyType == configuration.EnvVarKeyType {
+				return false
+			}
+		}
+
+		return true
+	}
+}
diff --git a/cliv2/cmd/cliv2/main.go b/cliv2/cmd/cliv2/main.go
index ad64396b0c..4cd83c5b5d 100644
--- a/cliv2/cmd/cliv2/main.go
+++ b/cliv2/cmd/cliv2/main.go
@@ -22,7 +22,6 @@ import (
 	"github.com/snyk/container-cli/pkg/container"
 	"github.com/snyk/go-application-framework/pkg/analytics"
 	"github.com/snyk/go-application-framework/pkg/app"
-	"github.com/snyk/go-application-framework/pkg/auth"
 	"github.com/snyk/go-application-framework/pkg/configuration"
 	"github.com/snyk/go-application-framework/pkg/instrumentation"
 	"github.com/spf13/cobra"
@@ -92,25 +91,6 @@ func initApplicationConfiguration(config configuration.Configuration) {
 	config.AddAlternativeKeys(configuration.ORGANIZATION, []string{"snyk_cfg_org"})
 	config.AddAlternativeKeys(configuration.PREVIEW_FEATURES_ENABLED, []string{"snyk_preview"})
 	config.AddAlternativeKeys(configuration.LOG_LEVEL, []string{debug_level_flag})
-
-	// if the CONFIG_KEY_OAUTH_TOKEN is specified as env var, we don't apply any additional logic
-	_, ok := os.LookupEnv(auth.CONFIG_KEY_OAUTH_TOKEN)
-	if !ok {
-		alternativeBearerKeys := config.GetAlternativeKeys(configuration.AUTHENTICATION_BEARER_TOKEN)
-		alternativeBearerKeys = append(alternativeBearerKeys, configuration.AUTHENTICATION_BEARER_TOKEN)
-		for _, key := range alternativeBearerKeys {
-			hasPrefix := strings.HasPrefix(key, "snyk_")
-			if hasPrefix {
-				formattedKey := strings.ToUpper(key)
-				_, ok := os.LookupEnv(formattedKey)
-				if ok {
-					globalLogger.Printf("Found environment variable %s, disabling OAuth flow", formattedKey)
-					config.Set(configuration.FF_OAUTH_AUTH_FLOW_ENABLED, false)
-					break
-				}
-			}
-		}
-	}
 }
 
 func getFullCommandString(cmd *cobra.Command) string {
@@ -480,6 +460,8 @@ func MainWithErrorCode() int {
 
 	globalEngine = app.CreateAppEngineWithOptions(app.WithZeroLogger(globalLogger), app.WithConfiguration(globalConfiguration), app.WithRuntimeInfo(rInfo))
 
+	globalConfiguration.AddDefaultValue(configuration.FF_OAUTH_AUTH_FLOW_ENABLED, defaultOAuthFF(globalConfiguration))
+
 	if noProxyAuth := globalConfiguration.GetBool(basic_workflows.PROXY_NOAUTH); noProxyAuth {
 		globalConfiguration.Set(configuration.PROXY_AUTHENTICATION_MECHANISM, httpauth.StringFromAuthenticationMechanism(httpauth.NoAuth))
 	}
diff --git a/test/jest/acceptance/cli-token-precedence.spec.ts b/test/jest/acceptance/cli-token-precedence.spec.ts
index 6731c4fc92..4d9d4ec000 100644
--- a/test/jest/acceptance/cli-token-precedence.spec.ts
+++ b/test/jest/acceptance/cli-token-precedence.spec.ts
@@ -110,7 +110,7 @@ describe('cli token precedence', () => {
         );
       });
 
-      describe('when oauth env vars are set', () => {
+      describe('when env vars are set', () => {
         it('SNYK_OAUTH_TOKEN should override config', async () => {
           env = {
             ...env,
@@ -134,6 +134,18 @@ describe('cli token precedence', () => {
           const authHeader = server.popRequest().headers?.authorization;
           expect(authHeader).toEqual(`Bearer ${env.SNYK_DOCKER_TOKEN}`);
         });
+
+        it('SNYK_TOKEN should override config', async () => {
+          env = {
+            ...env,
+            SNYK_TOKEN: 'SnykApiTokenEnvVar',
+          };
+
+          await runSnykCLI(`-d`, { env });
+
+          const authHeader = server.popRequest().headers?.authorization;
+          expect(authHeader).toEqual(`token ${env.SNYK_TOKEN}`);
+        });
       });
 
       if (snykOAuthConfig.name != auth.name) {