From 56ef00f954acc571f1950e973ed27750749847ae Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 18 Dec 2023 12:13:58 -0600 Subject: [PATCH 1/5] chore: yaml parsing for external auth configs --- codersdk/deployment.go | 34 ++++++++-------- codersdk/deployment_test.go | 69 ++++++++++++++++++++++++++++++++ codersdk/testdata/githubcfg.yaml | 23 +++++++++++ 3 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 codersdk/testdata/githubcfg.yaml diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 54b67f127907d..19d14c96b81ff 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -329,33 +329,33 @@ type TraceConfig struct { type ExternalAuthConfig struct { // Type is the type of external auth config. - Type string `json:"type"` - ClientID string `json:"client_id"` + Type string `json:"type" yaml:"type"` + ClientID string `json:"client_id" yaml:"client_id"` ClientSecret string `json:"-" yaml:"client_secret"` // ID is a unique identifier for the auth config. // It defaults to `type` when not provided. - ID string `json:"id"` - AuthURL string `json:"auth_url"` - TokenURL string `json:"token_url"` - ValidateURL string `json:"validate_url"` - AppInstallURL string `json:"app_install_url"` - AppInstallationsURL string `json:"app_installations_url"` - NoRefresh bool `json:"no_refresh"` - Scopes []string `json:"scopes"` - ExtraTokenKeys []string `json:"extra_token_keys"` - DeviceFlow bool `json:"device_flow"` - DeviceCodeURL string `json:"device_code_url"` + ID string `json:"id" yaml:"id"` + AuthURL string `json:"auth_url" yaml:"auth_url"` + TokenURL string `json:"token_url" yaml:"token_url"` + ValidateURL string `json:"validate_url" yaml:"validate_url"` + AppInstallURL string `json:"app_install_url" yaml:"app_install_url"` + AppInstallationsURL string `json:"app_installations_url" yaml:"app_installations_url"` + NoRefresh bool `json:"no_refresh" yaml:"no_refresh"` + Scopes []string `json:"scopes" yaml:"scopes"` + ExtraTokenKeys []string `json:"extra_token_keys" yaml:"extra_token_keys"` + DeviceFlow bool `json:"device_flow" yaml:"device_flow"` + DeviceCodeURL string `json:"device_code_url" yaml:"device_code_url"` // Regex allows API requesters to match an auth config by // a string (e.g. coder.com) instead of by it's type. // // Git clone makes use of this by parsing the URL from: // 'Username for "https://github.com":' // And sending it to the Coder server to match against the Regex. - Regex string `json:"regex"` + Regex string `json:"regex" yaml:"regex"` // DisplayName is shown in the UI to identify the auth config. - DisplayName string `json:"display_name"` + DisplayName string `json:"display_name" yaml:"display_name"` // DisplayIcon is a URL to an icon to display in the UI. - DisplayIcon string `json:"display_icon"` + DisplayIcon string `json:"display_icon" yaml:"display_icon"` } type ProvisionerConfig struct { @@ -1788,7 +1788,7 @@ Write out the current server config as YAML to stdout.`, Description: "External Authentication providers.", // We need extra scrutiny to ensure this works, is documented, and // tested before enabling. - // YAML: "gitAuthProviders", + YAML: "externalAuthProviders", Value: &c.ExternalAuthConfigs, Hidden: true, }, diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 3a7d32d7ada57..7a7a2092f51f3 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -1,11 +1,14 @@ package codersdk_test import ( + "embed" + "fmt" "strings" "testing" "time" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/codersdk" @@ -277,3 +280,69 @@ func TestDeploymentValues_DurationFormatNanoseconds(t *testing.T) { t.FailNow() } } + +//go:embed testdata/* +var testData embed.FS + +func TestExternalAuthYAMLConfig(t *testing.T) { + t.Parallel() + + file := func(t *testing.T, name string) string { + data, err := testData.ReadFile(fmt.Sprintf("testdata/%s", name)) + require.NoError(t, err, "read testdata file %q", name) + return string(data) + } + + testCases := []struct { + Name string + YAML string + Expected []codersdk.ExternalAuthConfig + }{ + { + Name: "GitHub", + YAML: file(t, "githubcfg.yaml"), + Expected: []codersdk.ExternalAuthConfig{ + { + Type: "github", + ClientID: "client_id", + ClientSecret: "client_secret", + ID: "id", + AuthURL: "https://example.com/auth", + TokenURL: "https://example.com/token", + ValidateURL: "https://example.com/validate", + AppInstallURL: "https://example.com/install", + AppInstallationsURL: "https://example.com/installations", + NoRefresh: true, + Scopes: []string{"user:email", "read:org"}, + ExtraTokenKeys: []string{"extra", "token"}, + DeviceFlow: true, + DeviceCodeURL: "https://example.com/device", + Regex: "^https://example.com/.*$", + DisplayName: "GitHub", + DisplayIcon: "/static/icons/github.svg", + }, + }, + }, + } + + for _, c := range testCases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + + dv := codersdk.DeploymentValues{} + opts := dv.Options() + c.YAML = strings.ReplaceAll(c.YAML, "\t", " ") + + // This is the order things are done in the cli, so just + // keep it the same. + var n yaml.Node + err := yaml.Unmarshal([]byte(c.YAML), &n) + require.NoError(t, err) + + err = n.Decode(&opts) + require.NoError(t, err) + require.ElementsMatchf(t, c.Expected, dv.ExternalAuthConfigs.Value, "from yaml") + }) + } +} diff --git a/codersdk/testdata/githubcfg.yaml b/codersdk/testdata/githubcfg.yaml new file mode 100644 index 0000000000000..4f03d78ad8413 --- /dev/null +++ b/codersdk/testdata/githubcfg.yaml @@ -0,0 +1,23 @@ +externalAuthProviders: + - type: github + client_id: client_id + client_secret: client_secret + id: id + auth_url: https://example.com/auth + token_url: https://example.com/token + validate_url: https://example.com/validate + app_install_url: https://example.com/install + app_installations_url: https://example.com/installations + no_refresh: true + scopes: + - user:email + - read:org + extra_token_keys: + - extra + - token + device_flow: true + device_code_url: https://example.com/device + regex: ^https://example.com/.*$ + display_name: GitHub + display_icon: /static/icons/github.svg + From e210cdb71c27e670bf77f9850aaf79da686afe2b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 18 Dec 2023 17:05:32 -0600 Subject: [PATCH 2/5] Make test parse 2 configs --- cli/testdata/server-config.yaml.golden | 3 ++ codersdk/deployment_test.go | 47 +++++++++++++++----------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 5abf74e675e68..af026023f634a 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -440,6 +440,9 @@ client: # Support links to display in the top right drop down menu. # (default: , type: struct[[]codersdk.LinkConfig]) supportLinks: [] +# External Authentication providers. +# (default: , type: struct[[]codersdk.ExternalAuthConfig]) +externalAuthProviders: [] # Hostname of HTTPS server that runs https://github.com/coder/wgtunnel. By # default, this will pick the best available wgtunnel server hosted by Coder. e.g. # "tunnel.example.com". diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 7a7a2092f51f3..34adaf66db933 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -292,6 +292,25 @@ func TestExternalAuthYAMLConfig(t *testing.T) { require.NoError(t, err, "read testdata file %q", name) return string(data) } + githubCfg := codersdk.ExternalAuthConfig{ + Type: "github", + ClientID: "client_id", + ClientSecret: "client_secret", + ID: "id", + AuthURL: "https://example.com/auth", + TokenURL: "https://example.com/token", + ValidateURL: "https://example.com/validate", + AppInstallURL: "https://example.com/install", + AppInstallationsURL: "https://example.com/installations", + NoRefresh: true, + Scopes: []string{"user:email", "read:org"}, + ExtraTokenKeys: []string{"extra", "token"}, + DeviceFlow: true, + DeviceCodeURL: "https://example.com/device", + Regex: "^https://example.com/.*$", + DisplayName: "GitHub", + DisplayIcon: "/static/icons/github.svg", + } testCases := []struct { Name string @@ -300,27 +319,15 @@ func TestExternalAuthYAMLConfig(t *testing.T) { }{ { Name: "GitHub", - YAML: file(t, "githubcfg.yaml"), + // Just load the GitHub config twice to test loading 2 + YAML: func() string { + f := file(t, "githubcfg.yaml") + lines := strings.SplitN(f, "\n", 2) + // Append github config twice + return f + "\n" + lines[1] + }(), Expected: []codersdk.ExternalAuthConfig{ - { - Type: "github", - ClientID: "client_id", - ClientSecret: "client_secret", - ID: "id", - AuthURL: "https://example.com/auth", - TokenURL: "https://example.com/token", - ValidateURL: "https://example.com/validate", - AppInstallURL: "https://example.com/install", - AppInstallationsURL: "https://example.com/installations", - NoRefresh: true, - Scopes: []string{"user:email", "read:org"}, - ExtraTokenKeys: []string{"extra", "token"}, - DeviceFlow: true, - DeviceCodeURL: "https://example.com/device", - Regex: "^https://example.com/.*$", - DisplayName: "GitHub", - DisplayIcon: "/static/icons/github.svg", - }, + githubCfg, githubCfg, }, }, } From 3b2aff5df69e1816f6d4fe8560080d8b1f984f06 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 09:04:45 -0600 Subject: [PATCH 3/5] make fmt --- codersdk/testdata/githubcfg.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/codersdk/testdata/githubcfg.yaml b/codersdk/testdata/githubcfg.yaml index 4f03d78ad8413..50c16fb30f63c 100644 --- a/codersdk/testdata/githubcfg.yaml +++ b/codersdk/testdata/githubcfg.yaml @@ -20,4 +20,3 @@ externalAuthProviders: regex: ^https://example.com/.*$ display_name: GitHub display_icon: /static/icons/github.svg - From 04c7d3b74c002374834296cf10d626140cabb739 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 10:54:57 -0600 Subject: [PATCH 4/5] Also unmarshal and check the output again --- codersdk/deployment_test.go | 67 ++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 34adaf66db933..e19b003bb2546 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -1,6 +1,7 @@ package codersdk_test import ( + "bytes" "embed" "fmt" "strings" @@ -312,44 +313,40 @@ func TestExternalAuthYAMLConfig(t *testing.T) { DisplayIcon: "/static/icons/github.svg", } - testCases := []struct { - Name string - YAML string - Expected []codersdk.ExternalAuthConfig - }{ - { - Name: "GitHub", - // Just load the GitHub config twice to test loading 2 - YAML: func() string { - f := file(t, "githubcfg.yaml") - lines := strings.SplitN(f, "\n", 2) - // Append github config twice - return f + "\n" + lines[1] - }(), - Expected: []codersdk.ExternalAuthConfig{ - githubCfg, githubCfg, - }, - }, + // Input the github section twice for testing a slice of configs. + inputYAML := func() string { + f := file(t, "githubcfg.yaml") + lines := strings.SplitN(f, "\n", 2) + // Append github config twice + return f + lines[1] + }() + + expected := []codersdk.ExternalAuthConfig{ + githubCfg, githubCfg, } - for _, c := range testCases { - c := c - t.Run(c.Name, func(t *testing.T) { - t.Parallel() + dv := codersdk.DeploymentValues{} + opts := dv.Options() + // replace any tabs with the proper space indentation + inputYAML = strings.ReplaceAll(inputYAML, "\t", " ") - dv := codersdk.DeploymentValues{} - opts := dv.Options() - c.YAML = strings.ReplaceAll(c.YAML, "\t", " ") + // This is the order things are done in the cli, so just + // keep it the same. + var n yaml.Node + err := yaml.Unmarshal([]byte(inputYAML), &n) + require.NoError(t, err) - // This is the order things are done in the cli, so just - // keep it the same. - var n yaml.Node - err := yaml.Unmarshal([]byte(c.YAML), &n) - require.NoError(t, err) + err = n.Decode(&opts) + require.NoError(t, err) + require.ElementsMatchf(t, expected, dv.ExternalAuthConfigs.Value, "from yaml") - err = n.Decode(&opts) - require.NoError(t, err) - require.ElementsMatchf(t, c.Expected, dv.ExternalAuthConfigs.Value, "from yaml") - }) - } + var out bytes.Buffer + enc := yaml.NewEncoder(&out) + enc.SetIndent(2) + err = enc.Encode(dv.ExternalAuthConfigs) + require.NoError(t, err) + + // Because we only marshal the 1 section, the correct section name is not applied. + output := strings.Replace(out.String(), "value:", "externalAuthProviders:", 1) + require.Equal(t, inputYAML, output, "re-marshaled is the same as input") } From 7307e00daf5bfe78d118c3a0a2313db981f4bcd2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 19 Dec 2023 12:02:41 -0600 Subject: [PATCH 5/5] Ignore windows --- codersdk/deployment_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index e19b003bb2546..ef84d64501d60 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -4,6 +4,7 @@ import ( "bytes" "embed" "fmt" + "runtime" "strings" "testing" "time" @@ -288,6 +289,12 @@ var testData embed.FS func TestExternalAuthYAMLConfig(t *testing.T) { t.Parallel() + if runtime.GOOS == "windows" { + // The windows marshal function uses different line endings. + // Not worth the effort getting this to work on windows. + t.SkipNow() + } + file := func(t *testing.T, name string) string { data, err := testData.ReadFile(fmt.Sprintf("testdata/%s", name)) require.NoError(t, err, "read testdata file %q", name)