Add support for new config properties

coder · kylecarbs · Oct 3, 2023 · Sep 30, 2023 · Oct 2, 2023 · Oct 2, 2023
commit c4b0fa02e299b0eee44c1a20f9ea1fcfb83c952c
diff --git a/cli/server.go b/cli/server.go
@@ -98,89 +98,6 @@ import (
 	"github.com/coder/wgtunnel/tunnelsdk"
 )
 
-// ReadGitAuthProvidersFromEnv is provided for compatibility purposes with the
-// viper CLI.
-// DEPRECATED
-func ReadGitAuthProvidersFromEnv(environ []string) ([]codersdk.ExternalAuthConfig, error) {
-	// The index numbers must be in-order.
-	sort.Strings(environ)
-
-	var providers []codersdk.ExternalAuthConfig
-	for _, v := range clibase.ParseEnviron(environ, "CODER_GITAUTH_") {
-		tokens := strings.SplitN(v.Name, "_", 2)
-		if len(tokens) != 2 {
-			return nil, xerrors.Errorf("invalid env var: %s", v.Name)
-		}
-
-		providerNum, err := strconv.Atoi(tokens[0])
-		if err != nil {
-			return nil, xerrors.Errorf("parse number: %s", v.Name)
-		}
-
-		var provider codersdk.ExternalAuthConfig
-		switch {
-		case len(providers) < providerNum:
-			return nil, xerrors.Errorf(
-				"provider num %v skipped: %s",
-				len(providers),
-				v.Name,
-			)
-		case len(providers) == providerNum:
-			// At the next next provider.
-			providers = append(providers, provider)
-		case len(providers) == providerNum+1:
-			// At the current provider.
-			provider = providers[providerNum]
-		}
-
-		key := tokens[1]
-		switch key {
-		case "ID":
-			provider.ID = v.Value
-		case "TYPE":
-			provider.Type = v.Value
-		case "CLIENT_ID":
-			provider.ClientID = v.Value
-		case "CLIENT_SECRET":
-			provider.ClientSecret = v.Value
-		case "AUTH_URL":
-			provider.AuthURL = v.Value
-		case "TOKEN_URL":
-			provider.TokenURL = v.Value
-		case "VALIDATE_URL":
-			provider.ValidateURL = v.Value
-		case "REGEX":
-			provider.Regex = v.Value
-		case "DEVICE_FLOW":
-			b, err := strconv.ParseBool(v.Value)
-			if err != nil {
-				return nil, xerrors.Errorf("parse bool: %s", v.Value)
-			}
-			provider.DeviceFlow = b
-		case "DEVICE_CODE_URL":
-			provider.DeviceCodeURL = v.Value
-		case "NO_REFRESH":
-			b, err := strconv.ParseBool(v.Value)
-			if err != nil {
-				return nil, xerrors.Errorf("parse bool: %s", v.Value)
-			}
-			provider.NoRefresh = b
-		case "SCOPES":
-			provider.Scopes = strings.Split(v.Value, " ")
-		case "APP_INSTALL_URL":
-			provider.AppInstallURL = v.Value
-		case "APP_INSTALLATIONS_URL":
-			provider.AppInstallationsURL = v.Value
-		case "DISPLAY_NAME":
-			provider.DisplayName = v.Value
-		case "DISPLAY_ICON":
-			provider.DisplayIcon = v.Value
-		}
-		providers[providerNum] = provider
-	}
-	return providers, nil
-}
-
 func createOIDCConfig(ctx context.Context, vals *codersdk.DeploymentValues) (*coderd.OIDCConfig, error) {
 	if vals.OIDC.ClientID == "" {
 		return nil, xerrors.Errorf("OIDC client ID must be set!")
@@ -572,14 +489,14 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				}
 			}
 
-			gitAuthEnv, err := ReadGitAuthProvidersFromEnv(os.Environ())
+			extAuthEnv, err := ReadExternalAuthProvidersFromEnv(os.Environ())
 			if err != nil {
-				return xerrors.Errorf("read git auth providers from env: %w", err)
+				return xerrors.Errorf("read external auth providers from env: %w", err)
 			}
 
-			vals.GitAuthProviders.Value = append(vals.GitAuthProviders.Value, gitAuthEnv...)
+			vals.ExternalAuthConfigs.Value = append(vals.ExternalAuthConfigs.Value, extAuthEnv...)
 			externalAuthConfigs, err := externalauth.ConvertConfig(
-				vals.GitAuthProviders.Value,
+				vals.ExternalAuthConfigs.Value,
 				vals.AccessURL.Value(),
 			)
 			if err != nil {
@@ -2246,3 +2163,101 @@ func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValue
 
 	return httpServers, nil
 }
+
+// ReadExternalAuthProvidersFromEnv is provided for compatibility purposes with
+// the viper CLI.
+func ReadExternalAuthProvidersFromEnv(environ []string) ([]codersdk.ExternalAuthConfig, error) {
+	providers, err := readExternalAuthProvidersFromEnv("CODER_EXTERNAL_AUTH_", environ)
+	if err != nil {
+		return nil, err
+	}
+	// Deprecated: To support legacy git auth!
+	gitProviders, err := readExternalAuthProvidersFromEnv("CODER_GITAUTH_", environ)
+	if err != nil {
+		return nil, err
+	}
+	return append(providers, gitProviders...), nil
+}
+
+// readExternalAuthProvidersFromEnv consumes environment variables to parse
+// external auth providers. A prefix is provided to support the legacy
+// parsing of `GITAUTH` environment variables.
+func readExternalAuthProvidersFromEnv(prefix string, environ []string) ([]codersdk.ExternalAuthConfig, error) {
+	// The index numbers must be in-order.
+	sort.Strings(environ)
+
+	var providers []codersdk.ExternalAuthConfig
+	for _, v := range clibase.ParseEnviron(environ, prefix) {
+		tokens := strings.SplitN(v.Name, "_", 2)
+		if len(tokens) != 2 {
+			return nil, xerrors.Errorf("invalid env var: %s", v.Name)
+		}
+
+		providerNum, err := strconv.Atoi(tokens[0])
+		if err != nil {
+			return nil, xerrors.Errorf("parse number: %s", v.Name)
+		}
+
+		var provider codersdk.ExternalAuthConfig
+		switch {
+		case len(providers) < providerNum:
+			return nil, xerrors.Errorf(
+				"provider num %v skipped: %s",
+				len(providers),
+				v.Name,
+			)
+		case len(providers) == providerNum:
+			// At the next next provider.
+			providers = append(providers, provider)
+		case len(providers) == providerNum+1:
+			// At the current provider.
+			provider = providers[providerNum]
+		}
+
+		key := tokens[1]
+		switch key {
+		case "ID":
+			provider.ID = v.Value
+		case "TYPE":
+			provider.Type = v.Value
+		case "CLIENT_ID":
+			provider.ClientID = v.Value
+		case "CLIENT_SECRET":
+			provider.ClientSecret = v.Value
+		case "AUTH_URL":
+			provider.AuthURL = v.Value
+		case "TOKEN_URL":
+			provider.TokenURL = v.Value
+		case "VALIDATE_URL":
+			provider.ValidateURL = v.Value
+		case "REGEX":
+			provider.Regex = v.Value
+		case "DEVICE_FLOW":
+			b, err := strconv.ParseBool(v.Value)
+			if err != nil {
+				return nil, xerrors.Errorf("parse bool: %s", v.Value)
+			}
+			provider.DeviceFlow = b
+		case "DEVICE_CODE_URL":
+			provider.DeviceCodeURL = v.Value
+		case "NO_REFRESH":
+			b, err := strconv.ParseBool(v.Value)
+			if err != nil {
+				return nil, xerrors.Errorf("parse bool: %s", v.Value)
+			}
+			provider.NoRefresh = b
+		case "SCOPES":
+			provider.Scopes = strings.Split(v.Value, " ")
+		case "APP_INSTALL_URL":
+			provider.AppInstallURL = v.Value
+		case "APP_INSTALLATIONS_URL":
+			provider.AppInstallationsURL = v.Value
+		case "DISPLAY_NAME":
+			provider.DisplayName = v.Value
+		case "DISPLAY_ICON":
+			provider.DisplayIcon = v.Value
+		}
+		providers[providerNum] = provider
+	}
+	return providers, nil
+}
diff --git a/cli/server_test.go b/cli/server_test.go
@@ -49,27 +49,66 @@ import (
 	"github.com/coder/coder/v2/testutil"
 )
 
+func TestReadExternalAuthProvidersFromEnv(t *testing.T) {
+	t.Parallel()
+	t.Run("Valid", func(t *testing.T) {
+		t.Parallel()
+		providers, err := cli.ReadExternalAuthProvidersFromEnv([]string{
+			"CODER_EXTERNAL_AUTH_0_ID=1",
+			"CODER_EXTERNAL_AUTH_0_TYPE=gitlab",
+			"CODER_EXTERNAL_AUTH_1_ID=2",
+			"CODER_EXTERNAL_AUTH_1_CLIENT_ID=sid",
+			"CODER_EXTERNAL_AUTH_1_CLIENT_SECRET=hunter12",
+			"CODER_EXTERNAL_AUTH_1_TOKEN_URL=google.com",
+			"CODER_EXTERNAL_AUTH_1_VALIDATE_URL=bing.com",
+			"CODER_EXTERNAL_AUTH_1_SCOPES=repo:read repo:write",
+			"CODER_EXTERNAL_AUTH_1_NO_REFRESH=true",
+			"CODER_EXTERNAL_AUTH_1_DISPLAY_NAME=Google",
+			"CODER_EXTERNAL_AUTH_1_DISPLAY_ICON=/icon/google.svg",
+		})
+		require.NoError(t, err)
+		require.Len(t, providers, 2)
+
+		// Validate the first provider.
+		assert.Equal(t, "1", providers[0].ID)
+		assert.Equal(t, "gitlab", providers[0].Type)
+
+		// Validate the second provider.
+		assert.Equal(t, "2", providers[1].ID)
+		assert.Equal(t, "sid", providers[1].ClientID)
+		assert.Equal(t, "hunter12", providers[1].ClientSecret)
+		assert.Equal(t, "google.com", providers[1].TokenURL)
+		assert.Equal(t, "bing.com", providers[1].ValidateURL)
+		assert.Equal(t, []string{"repo:read", "repo:write"}, providers[1].Scopes)
+		assert.Equal(t, true, providers[1].NoRefresh)
+		assert.Equal(t, "Google", providers[1].DisplayName)
+		assert.Equal(t, "/icon/google.svg", providers[1].DisplayIcon)
+	})
+}
+
+// TestReadGitAuthProvidersFromEnv ensures that the deprecated `CODER_GITAUTH_`
+// environment variables are still supported.
 func TestReadGitAuthProvidersFromEnv(t *testing.T) {
 	t.Parallel()
 	t.Run("Empty", func(t *testing.T) {
 		t.Parallel()
-		providers, err := cli.ReadGitAuthProvidersFromEnv([]string{
+		providers, err := cli.ReadExternalAuthProvidersFromEnv([]string{
 			"HOME=/home/frodo",
 		})
 		require.NoError(t, err)
 		require.Empty(t, providers)
 	})
 	t.Run("InvalidKey", func(t *testing.T) {
 		t.Parallel()
-		providers, err := cli.ReadGitAuthProvidersFromEnv([]string{
+		providers, err := cli.ReadExternalAuthProvidersFromEnv([]string{
 			"CODER_GITAUTH_XXX=invalid",
 		})
 		require.Error(t, err, "providers: %+v", providers)
 		require.Empty(t, providers)
 	})
 	t.Run("SkipKey", func(t *testing.T) {
 		t.Parallel()
-		providers, err := cli.ReadGitAuthProvidersFromEnv([]string{
+		providers, err := cli.ReadExternalAuthProvidersFromEnv([]string{
 			"CODER_GITAUTH_0_ID=invalid",
 			"CODER_GITAUTH_2_ID=invalid",
 		})
@@ -78,7 +117,7 @@ func TestReadGitAuthProvidersFromEnv(t *testing.T) {
 	})
 	t.Run("Valid", func(t *testing.T) {
 		t.Parallel()
-		providers, err := cli.ReadGitAuthProvidersFromEnv([]string{
+		providers, err := cli.ReadExternalAuthProvidersFromEnv([]string{
 			"CODER_GITAUTH_0_ID=1",
 			"CODER_GITAUTH_0_TYPE=gitlab",
 			"CODER_GITAUTH_1_ID=2",