From e011633cdc5889c579b8835657dde2615e90291b Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Wed, 18 Oct 2023 18:10:00 +0200
Subject: [PATCH] Add tests for AuthConfig HasEnvToken

---
 internal/config/auth_config_test.go | 48 +++++++++++++++++++++++++++++
 internal/config/config.go           |  8 +++++
 2 files changed, 56 insertions(+)

diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go
index 7459f89c2..621bc9c8b 100644
--- a/internal/config/auth_config_test.go
+++ b/internal/config/auth_config_test.go
@@ -105,6 +105,54 @@ func TestTokenFromKeyringNonExistent(t *testing.T) {
 	require.ErrorIs(t, err, keyring.ErrNotFound)
 }
 
+func TestHasEnvTokenWithoutAnyEnvToken(t *testing.T) {
+	// Given an empty hosts configuration
+	authCfg := newTestAuthConfig()
+	ghConfig.Read = func() (*ghConfig.Config, error) {
+		return authCfg.cfg, nil
+	}
+
+	// When we check if it has an env token
+	hasEnvToken := authCfg.HasEnvToken()
+
+	// Then it returns false
+	require.False(t, hasEnvToken, "expected not to have env token")
+}
+
+func TestHasEnvTokenWithEnvToken(t *testing.T) {
+	// Given an empty hosts configuration but a token set in the env var
+	authCfg := newTestAuthConfig()
+	ghConfig.Read = func() (*ghConfig.Config, error) {
+		return authCfg.cfg, nil
+	}
+	t.Setenv("GH_ENTERPRISE_TOKEN", "test-token")
+
+	// When we check if it has an env token
+	hasEnvToken := authCfg.HasEnvToken()
+
+	// Then it returns true
+	require.True(t, hasEnvToken, "expected to have env token")
+}
+
+func TestHasEnvTokenWithNoEnvTokenButAConfigVar(t *testing.T) {
+	t.Skip("this test is explicitly breaking some implementation assumptions")
+
+	// Given a token in the config
+	authCfg := newTestAuthConfig()
+	ghConfig.Read = func() (*ghConfig.Config, error) {
+		return authCfg.cfg, nil
+	}
+	// Using example.com here will cause the token to be returned from the config
+	_, err := authCfg.Login("example.com", "test-user", "test-token", "", false)
+	require.NoError(t, err)
+
+	// When we check if it has an env token
+	hasEnvToken := authCfg.HasEnvToken()
+
+	// Then it SHOULD return false
+	require.False(t, hasEnvToken, "expected not to have env token")
+}
+
 func TestNoUserInAuthConfig(t *testing.T) {
 	// Given a host configuration without a user
 	authCfg := newTestAuthConfig()
diff --git a/internal/config/config.go b/internal/config/config.go
index a9a421bd4..d93c37e3c 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -135,6 +135,14 @@ func (c *AuthConfig) HasEnvToken() bool {
 			return true
 		}
 	}
+	// TODO: This is _extremely_ knowledgable about the implementation of TokenFromEnvOrConfig
+	// It has to use a hostname that is not going to be found in the hosts so that it
+	// can guarantee that tokens will only be returned from a set env var.
+	// Discussed here, but maybe worth revisiting: https://github.com/cli/cli/pull/7169#discussion_r1136979033
+	//
+	// By providing example.com. it's also _only_ looking for GH_ENTERPRISE_TOKEN or GITHUB_ENTERPRISE_TOKEN
+	// or the GITHUB_TOKEN if we happen to be in a codespace?? This doesn't seem to reflect the actual
+	// name of the function at all.
 	token, _ := ghAuth.TokenFromEnvOrConfig(hostname)
 	return token != ""
 }