Fix auth switch writing bogus default entry into auth-contexts

commands/auth.go

@@ -243,17 +243,20 @@ func RunAuthToken(c *CmdConfig) error {
 }
 
 func ensureDefaultContextAndKeysOrder(contexts map[string]any) []string {
-	// Because the default context isn't present on the auth-contexts field,
-	// we add it manually so that it's always included in the output, and so
-	// we can check if it's the current context.
-	contexts[doctl.ArgDefaultContext] = true
-
-	// Extract and sort the map keys so that the order that we display the
-	// auth contexts is consistent.
-	keys := make([]string, 0)
+	// The default context isn't stored under auth-contexts (its token lives in
+	// the top-level access-token). Include it in the display keys without
+	// mutating the caller's map, which may be backed by Viper's live config.
+	keys := make([]string, 0, len(contexts)+1)
+	hasDefault := false
 	for ctx := range contexts {
+		if ctx == doctl.ArgDefaultContext {
+			hasDefault = true
+		}
 		keys = append(keys, ctx)
 	}
+	if !hasDefault {
+		keys = append(keys, doctl.ArgDefaultContext)
+	}
 	sort.Strings(keys)
 
 	return keys
@@ -298,30 +301,21 @@ func RunAuthSwitch(c *CmdConfig) error {
 		context = strings.ToLower(viper.GetString("context"))
 	}
 
-	// check that context exists
-	contextsAvail := viper.GetStringMap("auth-contexts")
-	contextsAvail[doctl.ArgDefaultContext] = true
-	keys := make([]string, 0)
-	for ctx := range contextsAvail {
-		keys = append(keys, ctx)
-	}
-
-	var contextExists bool
-	for _, ctx := range keys {
-		if ctx == context {
-			contextExists = true
+	// The default context is always valid; its token is stored in the
+	// top-level access-token, not under auth-contexts. Do not inject a
+	// synthetic "default" key into auth-contexts — that previously caused
+	// writeConfig() to persist default: "true" (see #1816).
+	contexts := viper.GetStringMapString("auth-contexts")
+	if context != doctl.ArgDefaultContext {
+		if _, ok := contexts[context]; !ok {
+			return errors.New("context does not exist")
 		}
 	}
 
-	if !contextExists {
-		return errors.New("context does not exist")
-	}
-
 	// The two lines below aren't required for doctl specific functionality,
 	// but somehow magically fixes an issue
 	// (https://github.com/digitalocean/doctl/issues/996) where auth-contexts
 	// are mangled when running this command.
-	contexts := viper.GetStringMapString("auth-contexts")
 	viper.Set("auth-contexts", contexts)
 
 	viper.Set("context", context)

commands/auth_test.go

@@ -148,7 +148,7 @@ func TestAuthForcesLowercase(t *testing.T) {
 		err := RunAuthInit(retrieveUserTokenFunc)(config)
 		assert.NoError(t, err)
 
-		contexts = map[string]any{doctl.ArgDefaultContext: true, "TestCapitalCase": true}
+		contexts = map[string]any{"TestCapitalCase": "token"}
 		viper.Set("auth-contexts", contexts)
 		viper.Set("context", "contextDoesntExist")
 		err = RunAuthSwitch(config)
@@ -162,6 +162,47 @@ func TestAuthForcesLowercase(t *testing.T) {
 	})
 }
 
+func TestAuthSwitchDefaultDoesNotPolluteAuthContexts(t *testing.T) {
+	cfw := cfgFileWriter
+	defer func() {
+		cfgFileWriter = cfw
+		viper.Set("auth-contexts", nil)
+		viper.Set("context", nil)
+		Context = ""
+	}()
+
+	cfgFileWriter = func() (io.WriteCloser, error) { return &nopWriteCloser{Writer: io.Discard}, nil }
+
+	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+		viper.Set("auth-contexts", map[string]any{
+			"teamadf": "dop_v1_fake_team_token",
+		})
+		viper.Set("context", "teamadf")
+		Context = doctl.ArgDefaultContext
+
+		err := RunAuthSwitch(config)
+		assert.NoError(t, err)
+		assert.Equal(t, doctl.ArgDefaultContext, viper.GetString("context"))
+
+		contexts := viper.GetStringMapString("auth-contexts")
+		_, hasDefault := contexts[doctl.ArgDefaultContext]
+		assert.False(t, hasDefault, "auth-contexts must not contain a synthetic default entry")
+		assert.Equal(t, "dop_v1_fake_team_token", contexts["teamadf"])
+	})
+}
+
+func TestEnsureDefaultContextAndKeysOrderDoesNotMutate(t *testing.T) {
+	contexts := map[string]any{
+		"teamadf": true,
+	}
+
+	keys := ensureDefaultContextAndKeysOrder(contexts)
+	assert.Equal(t, []string{doctl.ArgDefaultContext, "teamadf"}, keys)
+
+	_, hasDefault := contexts[doctl.ArgDefaultContext]
+	assert.False(t, hasDefault, "ensureDefaultContextAndKeysOrder must not mutate the input map")
+}
+
 func TestAuthList(t *testing.T) {
 	buf := &bytes.Buffer{}
 	config := &CmdConfig{Out: buf}

integration/auth_test.go

@@ -291,6 +291,39 @@ context: default
 		})
 	})
 
+	when("switching back to the default context", func() {
+		it("does not write a synthetic default entry into auth-contexts", func() {
+			var testConfigBytes = []byte(`access-token: first-token
+auth-contexts:
+  next: second-token
+context: next
+`)
+
+			tmpDir := t.TempDir()
+			testConfig := filepath.Join(tmpDir, "test-config.yml")
+			expect.NoError(os.WriteFile(testConfig, testConfigBytes, 0644))
+
+			cmd := exec.Command(builtBinaryPath,
+				"-u", server.URL,
+				"auth",
+				"switch",
+				"--config", testConfig,
+				"--context", "default",
+			)
+			output, err := cmd.CombinedOutput()
+			expect.NoError(err, string(output))
+			expect.Contains(string(output), "Now using context [default] by default")
+
+			fileBytes, err := os.ReadFile(testConfig)
+			expect.NoError(err)
+			fileContents := string(fileBytes)
+			expect.Contains(fileContents, "context: default")
+			expect.Contains(fileContents, "next: second-token")
+			expect.NotContains(fileContents, "default: \"true\"")
+			expect.NotContains(fileContents, "default: true")
+		})
+	})
+
 	when("switching contexts containing a period", func() {
 		it("does not mangle that context", func() {
 			var testConfigBytes = []byte(`access-token: first-token
@@ -314,7 +347,10 @@ context: default
 
 			fileBytes, err := os.ReadFile(testConfig)
 			expect.NoError(err)
-			expect.Contains(string(fileBytes), "test@example.com: second-token")
+			fileContents := string(fileBytes)
+			expect.Contains(fileContents, "test@example.com: second-token")
+			expect.NotContains(fileContents, "default: \"true\"")
+			expect.NotContains(fileContents, "default: true")
 
 			err = os.Remove(testConfig)
 			expect.NoError(err)