feat: implement bitbucket-server external auth defaults #10520
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import ( | |
| "net/http" | ||
| "net/url" | ||
| "regexp" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "golang.org/x/oauth2" | ||
|
|
@@ -494,7 +495,36 @@ 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[configType]; ok { | ||
| copyDefaultSettings(config, defaults) | ||
| return | ||
| } | ||
|
|
||
| // Dynamic defaults | ||
| switch codersdk.EnhancedExternalAuthProvider(config.Type) { | ||
| case codersdk.EnhancedExternalAuthProviderBitBucketServer: | ||
| copyDefaultSettings(config, bitbucketServerDefaults(config)) | ||
| return | ||
| default: | ||
| // No defaults for this type. We still want to run this apply with | ||
| // an empty set of defaults. | ||
| copyDefaultSettings(config, codersdk.ExternalAuthConfig{}) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| func copyDefaultSettings(config *codersdk.ExternalAuthConfig, defaults codersdk.ExternalAuthConfig) { | ||
| if config.AuthURL == "" { | ||
| config.AuthURL = defaults.AuthURL | ||
| } | ||
|
|
@@ -542,7 +572,43 @@ func applyDefaultsToConfig(config *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, ".", `\.`)) | ||
|
There was a problem hiding this comment. Shouldn't we be doing these for all of them? Seems like making it just for BitBucket server is weird. There was a problem hiding this comment. perhaps the self-hosted ones. There was a problem hiding this comment. We do. Azure coder/coderd/externalauth/externalauth.go Line 607 in 84509a1 Bitbucket Cloud coder/coderd/externalauth/externalauth.go Line 616 in 84509a1 Gitlab coder/coderd/externalauth/externalauth.go Line 625 in 84509a1 Github coder/coderd/externalauth/externalauth.go Line 634 in 84509a1 Bitbucket Server is always hosted on a different domain for each customer. So it's a dynamic regex string based on the There was a problem hiding this comment. @spikecurtis yes, the self hosted ones need this dynamic approach. We need to handle them case by case. Maybe there will be generalizations, but for now I only implemented self hosted bitbucket server. Gitlab for example can definitely have some broken defaults right now. There was a problem hiding this comment. @Emyrk automatically doing the Regex should be a global change in my mind; it's weird to do it for a single provider. There was a problem hiding this comment. The regex is automatic for all providers. As in, the default values are correct for every type now There was a problem hiding this comment. Talked to @kylecarbs offline a bit. Yes this problem exists with Gitlab and Github Enterprise. We should probably have some solution for these. I do not want to break backwards compatibility, and I do not have the test environments to confirm those (eg, are the urls consistent? bitbucket cloud and server are different). So for this PR, I'd rather not try and solve github enterprise and gitlab self hosted. |
||
|
|
||
| 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", | ||
|
|
@@ -551,7 +617,7 @@ var defaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.ExternalAuthCo | |
| Regex: `^(https?://)?dev\.azure\.com(/.*)?$`, | ||
| Scopes: []string{"vso.code_write"}, | ||
| }, | ||
| 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", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| package externalauth | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/coder/coder/v2/codersdk" | ||
| ) | ||
|
|
||
| func Test_bitbucketServerConfigDefaults(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| bbType := string(codersdk.EnhancedExternalAuthProviderBitBucketServer) | ||
| tests := []struct { | ||
| name string | ||
| config *codersdk.ExternalAuthConfig | ||
| expected codersdk.ExternalAuthConfig | ||
| }{ | ||
| { | ||
| // Very few fields are statically defined for Bitbucket Server. | ||
| name: "EmptyBitbucketServer", | ||
| 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", | ||
| }, | ||
| }, | ||
| { | ||
| // 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 | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| applyDefaultsToConfig(tt.config) | ||
| require.Equal(t, tt.expected, *tt.config) | ||
| }) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused why we need to do this when we set it above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because the actual string "bitbucket" is being used in the
copyDefaultSettingsfunction, so for legacy reasons it has to be the legacy value. But at the end of the function, we need to set it to the new type. And I just did it in a defer to not put the if statement in 2 places.