From f7a5742a5493bf03a610ccead79fec2d2c664758 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 Nov 2023 14:56:20 -0500 Subject: [PATCH 1/7] feat: implement bitbucket-server external auth defaults Bitbucket cloud != Bitbucket server Add reasonable defaults for server --- coderd/externalauth/externalauth.go | 59 +++++++++++++++++-- .../externalauth_internal_test.go | 58 ++++++++++++++++++ coderd/workspaceagents.go | 2 +- codersdk/externalauth.go | 8 ++- site/src/api/typesGenerated.ts | 2 + 5 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 coderd/externalauth/externalauth_internal_test.go diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index 8802b5d5f6108..aa541a50aeff4 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "regexp" + "strings" "time" "golang.org/x/oauth2" @@ -409,7 +410,7 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([ // Applies defaults to the config entry. // This allows users to very simply state that they type is "GitHub", // apply their client secret and ID, and have the UI appear nicely. - applyDefaultsToConfig(&entry) + configDefaults(&entry) valid := httpapi.NameValid(entry.ID) if valid != nil { @@ -490,8 +491,22 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([ } // applyDefaultsToConfig applies defaults to the config entry. -func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig) { - defaults := defaults[codersdk.EnhancedExternalAuthProvider(config.Type)] +func configDefaults(config *codersdk.ExternalAuthConfig) { + // If static defaults exist, apply them. + if defaults, ok := staticDefaults[codersdk.EnhancedExternalAuthProvider(config.Type)]; ok { + applyDefaultsToConfig(config, defaults) + return + } + + // Dynamic defaults + switch codersdk.EnhancedExternalAuthProvider(config.Type) { + case codersdk.EnhancedExternalAuthProviderBitBucketServer: + applyDefaultsToConfig(config, bitbucketServerDefaults(config)) + return + } +} + +func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig, defaults codersdk.ExternalAuthConfig) { if config.AuthURL == "" { config.AuthURL = defaults.AuthURL } @@ -539,7 +554,43 @@ func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig) { } } -var defaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.ExternalAuthConfig{ +func bitbucketServerDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthConfig { + defaults := codersdk.ExternalAuthConfig{ + DisplayName: "Bitbucket Server", + Scopes: []string{"PUBLIC_REPOS", "REPO_READ", "REPO_WRITE"}, + DisplayIcon: "/icon/bitbucket.svg", + } + // Bitbucket servers will have some base url, e.g. https://bitbucket.coder.com. + // We will grab this from the Auth URL. This choice is a bit arbitrary, + // but we need to require at least 1 field to be populated. + if config.AuthURL == "" { + // No auth url, means we cannot guess the urls. + return defaults + } + + auth, err := url.Parse(config.AuthURL) + if err != nil { + // We need a valid URL to continue with. + return defaults + } + + // Populate Regex, ValidateURL, and TokenURL. + // Default regex should be anything using the same host as the auth url. + defaults.Regex = fmt.Sprintf(`^(https?://)?%s(/.*)?$`, strings.ReplaceAll(auth.Host, ".", `\.`)) + + tokenURL := auth.ResolveReference(&url.URL{Path: "/rest/oauth2/latest/token"}) + defaults.TokenURL = tokenURL.String() + + // validate needs to return a 200 when logged in and a 401 when unauthenticated. + // This endpoint returns the count of the number of PR's in the authenticated + // user's inbox. Which will work perfectly for our use case. + validate := auth.ResolveReference(&url.URL{Path: "/rest/api/latest/inbox/pull-requests/count"}) + defaults.ValidateURL = validate.String() + + return defaults +} + +var staticDefaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.ExternalAuthConfig{ codersdk.EnhancedExternalAuthProviderAzureDevops: { AuthURL: "https://app.vssps.visualstudio.com/oauth2/authorize", TokenURL: "https://app.vssps.visualstudio.com/oauth2/token", diff --git a/coderd/externalauth/externalauth_internal_test.go b/coderd/externalauth/externalauth_internal_test.go new file mode 100644 index 0000000000000..312dd17b677d7 --- /dev/null +++ b/coderd/externalauth/externalauth_internal_test.go @@ -0,0 +1,58 @@ +package externalauth + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/codersdk" +) + +func Test_bitbucketServerConfigDefaults(t *testing.T) { + bbType := string(codersdk.EnhancedExternalAuthProviderBitBucketServer) + tests := []struct { + name string + config *codersdk.ExternalAuthConfig + expected codersdk.ExternalAuthConfig + }{ + { + // Very few fields are statically defined for Bitbucket Server. + name: "EmpyBitbucketServer", + config: &codersdk.ExternalAuthConfig{ + Type: bbType, + }, + expected: codersdk.ExternalAuthConfig{ + Type: bbType, + ID: bbType, + DisplayName: "Bitbucket Server", + Scopes: []string{"PUBLIC_REPOS", "REPO_READ", "REPO_WRITE"}, + DisplayIcon: "/icon/bitbucket.svg", + }, + }, + { + // Only the AuthURL is required for defaults to work. + name: "AuthURL", + config: &codersdk.ExternalAuthConfig{ + Type: bbType, + AuthURL: "https://bitbucket.example.com/login/oauth/authorize", + }, + expected: codersdk.ExternalAuthConfig{ + Type: bbType, + ID: bbType, + AuthURL: "https://bitbucket.example.com/login/oauth/authorize", + TokenURL: "https://bitbucket.example.com/rest/oauth2/latest/token", + ValidateURL: "https://bitbucket.example.com/rest/api/latest/inbox/pull-requests/count", + Scopes: []string{"PUBLIC_REPOS", "REPO_READ", "REPO_WRITE"}, + Regex: `^(https?://)?bitbucket\.example\.com(/.*)?$`, + DisplayName: "Bitbucket Server", + DisplayIcon: "/icon/bitbucket.svg", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + configDefaults(tt.config) + require.Equal(t, tt.expected, *tt.config) + }) + } +} diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index dc2d240384475..aaa5b715ec00e 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -2452,7 +2452,7 @@ func createExternalAuthResponse(typ, token string, extra pqtype.NullRawMessage) Username: "oauth2", Password: token, } - case string(codersdk.EnhancedExternalAuthProviderBitBucket): + case string(codersdk.EnhancedExternalAuthProviderBitBucket), string(codersdk.EnhancedExternalAuthProviderBitBucketServer): // https://support.atlassian.com/bitbucket-cloud/docs/use-oauth-on-bitbucket-cloud/#Cloning-a-repository-with-an-access-token resp = agentsdk.ExternalAuthResponse{ Username: "x-token-auth", diff --git a/codersdk/externalauth.go b/codersdk/externalauth.go index 0167ca8156259..03a0670dc054c 100644 --- a/codersdk/externalauth.go +++ b/codersdk/externalauth.go @@ -22,6 +22,7 @@ func (e EnhancedExternalAuthProvider) Git() bool { case EnhancedExternalAuthProviderGitHub, EnhancedExternalAuthProviderGitLab, EnhancedExternalAuthProviderBitBucket, + EnhancedExternalAuthProviderBitBucketServer, EnhancedExternalAuthProviderAzureDevops: return true default: @@ -33,8 +34,11 @@ const ( EnhancedExternalAuthProviderAzureDevops EnhancedExternalAuthProvider = "azure-devops" EnhancedExternalAuthProviderGitHub EnhancedExternalAuthProvider = "github" EnhancedExternalAuthProviderGitLab EnhancedExternalAuthProvider = "gitlab" - EnhancedExternalAuthProviderBitBucket EnhancedExternalAuthProvider = "bitbucket" - EnhancedExternalAuthProviderSlack EnhancedExternalAuthProvider = "slack" + // EnhancedExternalAuthProviderBitBucket is the Bitbucket Cloud provider. + // Not to be confused with the self-hosted 'EnhancedExternalAuthProviderBitBucketServer' + EnhancedExternalAuthProviderBitBucket EnhancedExternalAuthProvider = "bitbucket" + EnhancedExternalAuthProviderBitBucketServer EnhancedExternalAuthProvider = "bitbucket-server" + EnhancedExternalAuthProviderSlack EnhancedExternalAuthProvider = "slack" ) type ExternalAuth struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ce7facdc55522..56b8e0a92a48a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1682,12 +1682,14 @@ export const DisplayApps: DisplayApp[] = [ export type EnhancedExternalAuthProvider = | "azure-devops" | "bitbucket" + | "bitbucket-server" | "github" | "gitlab" | "slack"; export const EnhancedExternalAuthProviders: EnhancedExternalAuthProvider[] = [ "azure-devops", "bitbucket", + "bitbucket-server", "github", "gitlab", "slack", From 5bfa3579d6ef6891e54ebdfd26d59cf41de3ba27 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 Nov 2023 15:25:21 -0500 Subject: [PATCH 2/7] make fmt --- codersdk/externalauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/externalauth.go b/codersdk/externalauth.go index e17701aa7c577..686082d8a0a32 100644 --- a/codersdk/externalauth.go +++ b/codersdk/externalauth.go @@ -39,7 +39,7 @@ const ( EnhancedExternalAuthProviderBitBucket EnhancedExternalAuthProvider = "bitbucket" EnhancedExternalAuthProviderBitBucketServer EnhancedExternalAuthProvider = "bitbucket-server" EnhancedExternalAuthProviderSlack EnhancedExternalAuthProvider = "slack" - EnhancedExternalAuthProviderJFrog EnhancedExternalAuthProvider = "jfrog" + EnhancedExternalAuthProviderJFrog EnhancedExternalAuthProvider = "jfrog" ) type ExternalAuth struct { From 5d4d8fe6e8d0eab7d433ec72350e49c9d50b5fcf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 Nov 2023 15:27:36 -0500 Subject: [PATCH 3/7] fix jfrog --- coderd/externalauth/externalauth.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index ff990749ce9c3..7a8172f9bbde6 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -506,6 +506,11 @@ func configDefaults(config *codersdk.ExternalAuthConfig) { case codersdk.EnhancedExternalAuthProviderBitBucketServer: applyDefaultsToConfig(config, bitbucketServerDefaults(config)) return + default: + // No defaults for this type. We still want to run this apply with + // an empty set of defaults. + applyDefaultsToConfig(config, codersdk.ExternalAuthConfig{}) + return } } From b3d75ec7a87668b3879a5429d8a2a2756f616be3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 3 Nov 2023 15:39:51 -0500 Subject: [PATCH 4/7] parallel unit test --- coderd/externalauth/externalauth_internal_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/coderd/externalauth/externalauth_internal_test.go b/coderd/externalauth/externalauth_internal_test.go index 312dd17b677d7..30045b87472d2 100644 --- a/coderd/externalauth/externalauth_internal_test.go +++ b/coderd/externalauth/externalauth_internal_test.go @@ -9,6 +9,8 @@ import ( ) func Test_bitbucketServerConfigDefaults(t *testing.T) { + t.Parallel() + bbType := string(codersdk.EnhancedExternalAuthProviderBitBucketServer) tests := []struct { name string @@ -50,7 +52,9 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { }, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() configDefaults(tt.config) require.Equal(t, tt.expected, *tt.config) }) From 84509a19de051f0a9fd43c7bd770cfd373c96ba6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Nov 2023 07:40:56 -0600 Subject: [PATCH 5/7] Rename functions --- coderd/externalauth/externalauth.go | 12 ++++++------ coderd/externalauth/externalauth_internal_test.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index 7a8172f9bbde6..b81046bf8bbca 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -410,7 +410,7 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([ // Applies defaults to the config entry. // This allows users to very simply state that they type is "GitHub", // apply their client secret and ID, and have the UI appear nicely. - configDefaults(&entry) + applyDefaultsToConfig(&entry) valid := httpapi.NameValid(entry.ID) if valid != nil { @@ -494,27 +494,27 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([ } // applyDefaultsToConfig applies defaults to the config entry. -func configDefaults(config *codersdk.ExternalAuthConfig) { +func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig) { // If static defaults exist, apply them. if defaults, ok := staticDefaults[codersdk.EnhancedExternalAuthProvider(config.Type)]; ok { - applyDefaultsToConfig(config, defaults) + copyDefaultSettings(config, defaults) return } // Dynamic defaults switch codersdk.EnhancedExternalAuthProvider(config.Type) { case codersdk.EnhancedExternalAuthProviderBitBucketServer: - applyDefaultsToConfig(config, bitbucketServerDefaults(config)) + copyDefaultSettings(config, bitbucketServerDefaults(config)) return default: // No defaults for this type. We still want to run this apply with // an empty set of defaults. - applyDefaultsToConfig(config, codersdk.ExternalAuthConfig{}) + copyDefaultSettings(config, codersdk.ExternalAuthConfig{}) return } } -func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig, defaults codersdk.ExternalAuthConfig) { +func copyDefaultSettings(config *codersdk.ExternalAuthConfig, defaults codersdk.ExternalAuthConfig) { if config.AuthURL == "" { config.AuthURL = defaults.AuthURL } diff --git a/coderd/externalauth/externalauth_internal_test.go b/coderd/externalauth/externalauth_internal_test.go index 30045b87472d2..c6a766823063f 100644 --- a/coderd/externalauth/externalauth_internal_test.go +++ b/coderd/externalauth/externalauth_internal_test.go @@ -55,7 +55,7 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - configDefaults(tt.config) + applyDefaultsToConfig(tt.config) require.Equal(t, tt.expected, *tt.config) }) } From 57da5cc68fb37d5ee4a81b424f5f9c44fc22de2c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Nov 2023 10:16:55 -0600 Subject: [PATCH 6/7] change "bitbucket" to "bitbucket-cloud" --- coderd/externalauth/externalauth.go | 14 +++++++++++-- .../externalauth_internal_test.go | 21 ++++++++++++++++++- coderd/workspaceagents.go | 3 ++- codersdk/externalauth.go | 6 +++--- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index b81046bf8bbca..6eb30c835b31b 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -495,8 +495,18 @@ func ConvertConfig(entries []codersdk.ExternalAuthConfig, accessURL *url.URL) ([ // applyDefaultsToConfig applies defaults to the config entry. func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig) { + configType := codersdk.EnhancedExternalAuthProvider(config.Type) + if configType == "bitbucket" { + // For backwards compatibility, we need to support the "bitbucket" string. + configType = codersdk.EnhancedExternalAuthProviderBitBucketCloud + defer func() { + // The config type determines the config ID (if unset). So change the legacy + // type to the correct new type after the defaults have been configured. + config.Type = string(codersdk.EnhancedExternalAuthProviderBitBucketCloud) + }() + } // If static defaults exist, apply them. - if defaults, ok := staticDefaults[codersdk.EnhancedExternalAuthProvider(config.Type)]; ok { + if defaults, ok := staticDefaults[configType]; ok { copyDefaultSettings(config, defaults) return } @@ -607,7 +617,7 @@ var staticDefaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.External Regex: `^(https?://)?dev\.azure\.com(/.*)?$`, Scopes: []string{"vso.code_write"}, }, - codersdk.EnhancedExternalAuthProviderBitBucket: { + codersdk.EnhancedExternalAuthProviderBitBucketCloud: { AuthURL: "https://bitbucket.org/site/oauth2/authorize", TokenURL: "https://bitbucket.org/site/oauth2/access_token", ValidateURL: "https://api.bitbucket.org/2.0/user", diff --git a/coderd/externalauth/externalauth_internal_test.go b/coderd/externalauth/externalauth_internal_test.go index c6a766823063f..5bb14a4c8f697 100644 --- a/coderd/externalauth/externalauth_internal_test.go +++ b/coderd/externalauth/externalauth_internal_test.go @@ -19,7 +19,7 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { }{ { // Very few fields are statically defined for Bitbucket Server. - name: "EmpyBitbucketServer", + name: "EmptyBitbucketServer", config: &codersdk.ExternalAuthConfig{ Type: bbType, }, @@ -50,6 +50,25 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { DisplayIcon: "/icon/bitbucket.svg", }, }, + { + // Ensure backwards compatibility. The type should update to "bitbucket-cloud", + // but the ID and other fields should remain the same. + name: "BitbucketLegacy", + config: &codersdk.ExternalAuthConfig{ + Type: "bitbucket", + }, + expected: codersdk.ExternalAuthConfig{ + Type: string(codersdk.EnhancedExternalAuthProviderBitBucketCloud), + ID: "bitbucket", // Legacy ID remains unchanged + AuthURL: "https://bitbucket.org/site/oauth2/authorize", + TokenURL: "https://bitbucket.org/site/oauth2/access_token", + ValidateURL: "https://api.bitbucket.org/2.0/user", + DisplayName: "BitBucket", + DisplayIcon: "/icon/bitbucket.svg", + Regex: `^(https?://)?bitbucket\.org(/.*)?$`, + Scopes: []string{"account", "repository:write"}, + }, + }, } for _, tt := range tests { tt := tt diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index aaa5b715ec00e..bfc92d92a0849 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -2452,7 +2452,8 @@ func createExternalAuthResponse(typ, token string, extra pqtype.NullRawMessage) Username: "oauth2", Password: token, } - case string(codersdk.EnhancedExternalAuthProviderBitBucket), string(codersdk.EnhancedExternalAuthProviderBitBucketServer): + case string(codersdk.EnhancedExternalAuthProviderBitBucketCloud), string(codersdk.EnhancedExternalAuthProviderBitBucketServer): + // The string "bitbucket" was a legacy parameter that needs to still be supported. // https://support.atlassian.com/bitbucket-cloud/docs/use-oauth-on-bitbucket-cloud/#Cloning-a-repository-with-an-access-token resp = agentsdk.ExternalAuthResponse{ Username: "x-token-auth", diff --git a/codersdk/externalauth.go b/codersdk/externalauth.go index 686082d8a0a32..8d858670eed3d 100644 --- a/codersdk/externalauth.go +++ b/codersdk/externalauth.go @@ -21,7 +21,7 @@ func (e EnhancedExternalAuthProvider) Git() bool { switch e { case EnhancedExternalAuthProviderGitHub, EnhancedExternalAuthProviderGitLab, - EnhancedExternalAuthProviderBitBucket, + EnhancedExternalAuthProviderBitBucketCloud, EnhancedExternalAuthProviderBitBucketServer, EnhancedExternalAuthProviderAzureDevops: return true @@ -34,9 +34,9 @@ const ( EnhancedExternalAuthProviderAzureDevops EnhancedExternalAuthProvider = "azure-devops" EnhancedExternalAuthProviderGitHub EnhancedExternalAuthProvider = "github" EnhancedExternalAuthProviderGitLab EnhancedExternalAuthProvider = "gitlab" - // EnhancedExternalAuthProviderBitBucket is the Bitbucket Cloud provider. + // EnhancedExternalAuthProviderBitBucketCloud is the Bitbucket Cloud provider. // Not to be confused with the self-hosted 'EnhancedExternalAuthProviderBitBucketServer' - EnhancedExternalAuthProviderBitBucket EnhancedExternalAuthProvider = "bitbucket" + EnhancedExternalAuthProviderBitBucketCloud EnhancedExternalAuthProvider = "bitbucket-cloud" EnhancedExternalAuthProviderBitBucketServer EnhancedExternalAuthProvider = "bitbucket-server" EnhancedExternalAuthProviderSlack EnhancedExternalAuthProvider = "slack" EnhancedExternalAuthProviderJFrog EnhancedExternalAuthProvider = "jfrog" From 7d239d8993050cd0de8a50abe7d0978d256ee8ba Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 6 Nov 2023 10:20:45 -0600 Subject: [PATCH 7/7] Make gen --- site/src/api/typesGenerated.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index afcd94ea6220d..3940ecfe83aea 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1681,7 +1681,7 @@ export const DisplayApps: DisplayApp[] = [ // From codersdk/externalauth.go export type EnhancedExternalAuthProvider = | "azure-devops" - | "bitbucket" + | "bitbucket-cloud" | "bitbucket-server" | "github" | "gitlab" @@ -1689,7 +1689,7 @@ export type EnhancedExternalAuthProvider = | "slack"; export const EnhancedExternalAuthProviders: EnhancedExternalAuthProvider[] = [ "azure-devops", - "bitbucket", + "bitbucket-cloud", "bitbucket-server", "github", "gitlab",