diff --git a/cli/server.go b/cli/server.go index e8e2d24de1873..dbc35141e83b9 100644 --- a/cli/server.go +++ b/cli/server.go @@ -186,6 +186,14 @@ func createOIDCConfig(ctx context.Context, logger slog.Logger, vals *codersdk.De secondaryClaimsSrc = coderd.MergedClaimsSourceAccessToken } + var pkceSupport struct { + CodeChallengeMethodsSupported []promoauth.Oauth2PKCEChallengeMethod `json:"code_challenge_methods_supported"` + } + err = oidcProvider.Claims(&pkceSupport) + if err != nil { + return nil, xerrors.Errorf("pkce detect in claims: %w", err) + } + return &coderd.OIDCConfig{ OAuth2Config: useCfg, Provider: oidcProvider, @@ -206,6 +214,7 @@ func createOIDCConfig(ctx context.Context, logger slog.Logger, vals *codersdk.De SignupsDisabledText: vals.OIDC.SignupsDisabledText.String(), IconURL: vals.OIDC.IconURL.String(), IgnoreEmailVerified: vals.OIDC.IgnoreEmailVerified.Value(), + PKCEMethods: pkceSupport.CodeChallengeMethodsSupported, }, nil } @@ -2761,6 +2770,8 @@ func parseExternalAuthProvidersFromEnv(prefix string, environ []string) ([]coder provider.MCPToolAllowRegex = v.Value case "MCP_TOOL_DENY_REGEX": provider.MCPToolDenyRegex = v.Value + case "PKCE_METHODS": + provider.CodeChallengeMethodsSupported = strings.Split(v.Value, " ") } providers[providerNum] = provider } diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index b8e3331ecd1f2..8b69721d35206 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -14631,6 +14631,12 @@ const docTemplate = `{ "client_id": { "type": "string" }, + "code_challenge_methods_supported": { + "type": "array", + "items": { + "type": "string" + } + }, "device_code_url": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 396a704a06119..0c927eee90bd9 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -13208,6 +13208,12 @@ "client_id": { "type": "string" }, + "code_challenge_methods_supported": { + "type": "array", + "items": { + "type": "string" + } + }, "device_code_url": { "type": "string" }, diff --git a/coderd/coderd.go b/coderd/coderd.go index b356f372dc56c..29a678a6ba669 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -24,6 +24,7 @@ import ( "github.com/coder/coder/v2/coderd/oauth2provider" "github.com/coder/coder/v2/coderd/pproflabel" "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/coder/coder/v2/coderd/promoauth" "github.com/coder/coder/v2/coderd/usage" "github.com/coder/coder/v2/coderd/wsbuilder" @@ -940,7 +941,7 @@ func New(options *Options) *API { r.Route(fmt.Sprintf("/%s/callback", externalAuthConfig.ID), func(r chi.Router) { r.Use( apiKeyMiddlewareRedirect, - httpmw.ExtractOAuth2(externalAuthConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil), + httpmw.ExtractOAuth2(externalAuthConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil, externalAuthConfig.CodeChallengeMethodsSupported), ) r.Get("/", api.externalAuthCallback(externalAuthConfig)) }) @@ -1289,14 +1290,15 @@ func New(options *Options) *API { r.Get("/github/device", api.userOAuth2GithubDevice) r.Route("/github", func(r chi.Router) { r.Use( - httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil), + // Github supports PKCE S256 + httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil, []promoauth.Oauth2PKCEChallengeMethod{promoauth.PKCEChallengeMethodSha256}), ) r.Get("/callback", api.userOAuth2Github) }) }) r.Route("/oidc/callback", func(r chi.Router) { r.Use( - httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, oidcAuthURLParams), + httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, oidcAuthURLParams, options.OIDCConfig.PKCESupported()), ) r.Get("/", api.userOIDC) }) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index d5215b9964a14..6eb5bc000003d 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -169,6 +169,7 @@ type FakeIDP struct { // clientID to be used by coderd clientID string clientSecret string + pkce bool // TODO: Implement for refresh token flow as well // externalProviderID is optional to match the provider in coderd for // redirectURLs. externalProviderID string @@ -181,6 +182,8 @@ type FakeIDP struct { // These maps are used to control the state of the IDP. // That is the various access tokens, refresh tokens, states, etc. codeToStateMap *syncmap.Map[string, string] + // Code -> PKCE Challenge + codeToChallengeMap *syncmap.Map[string, string] // Token -> Email accessTokens *syncmap.Map[string, token] // Refresh Token -> Email @@ -239,6 +242,12 @@ func (s statusHookError) Error() string { type FakeIDPOpt func(idp *FakeIDP) +func WithPKCE() func(*FakeIDP) { + return func(f *FakeIDP) { + f.pkce = true + } +} + func WithAuthorizedRedirectURL(hook func(redirectURL string) error) func(*FakeIDP) { return func(f *FakeIDP) { f.hookValidRedirectURL = hook @@ -450,6 +459,7 @@ func NewFakeIDP(t testing.TB, opts ...FakeIDPOpt) *FakeIDP { clientSecret: uuid.NewString(), logger: slog.Make(), codeToStateMap: syncmap.New[string, string](), + codeToChallengeMap: syncmap.New[string, string](), accessTokens: syncmap.New[string, token](), refreshTokens: syncmap.New[string, string](), refreshTokensUsed: syncmap.New[string, bool](), @@ -557,8 +567,16 @@ func (f *FakeIDP) realServer(t testing.TB) *httptest.Server { func (f *FakeIDP) GenerateAuthenticatedToken(claims jwt.MapClaims) (*oauth2.Token, error) { state := uuid.NewString() f.stateToIDTokenClaims.Store(state, claims) - code := f.newCode(state) - return f.locked.Config().Exchange(oidc.ClientContext(context.Background(), f.HTTPClient(nil)), code) + + exchangeOpts := []oauth2.AuthCodeOption{} + verifier := "" + if f.pkce { + verifier = oauth2.GenerateVerifier() + exchangeOpts = append(exchangeOpts, oauth2.VerifierOption(verifier)) + } + code := f.newCode(state, oauth2.S256ChallengeFromVerifier(verifier)) + + return f.locked.Config().Exchange(oidc.ClientContext(context.Background(), f.HTTPClient(nil)), code, exchangeOpts...) } // Login does the full OIDC flow starting at the "LoginButton". @@ -756,10 +774,16 @@ func (f *FakeIDP) OIDCCallback(t testing.TB, state string, idTokenClaims jwt.Map panic("cannot use OIDCCallback with WithServing. This is only for the in memory usage") } + opts := []oauth2.AuthCodeOption{} + if f.pkce { + verifier := oauth2.GenerateVerifier() + opts = append(opts, oauth2.S256ChallengeOption(oauth2.S256ChallengeFromVerifier(verifier))) + } + f.stateToIDTokenClaims.Store(state, idTokenClaims) cli := f.HTTPClient(nil) - u := f.locked.Config().AuthCodeURL(state) + u := f.locked.Config().AuthCodeURL(state, opts...) req, err := http.NewRequest("GET", u, nil) require.NoError(t, err) @@ -790,9 +814,10 @@ type ProviderJSON struct { // newCode enforces the code exchanged is actually a valid code // created by the IDP. -func (f *FakeIDP) newCode(state string) string { +func (f *FakeIDP) newCode(state string, challenge string) string { code := uuid.NewString() f.codeToStateMap.Store(code, state) + f.codeToChallengeMap.Store(code, challenge) return code } @@ -918,6 +943,22 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { mux.Handle(authorizePath, http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { f.logger.Info(r.Context(), "http call authorize", slogRequestFields(r)...) + challenge := "" + if f.pkce { + method := r.URL.Query().Get("code_challenge_method") + challenge = r.URL.Query().Get("code_challenge") + + if method == "" { + httpError(rw, http.StatusBadRequest, xerrors.New("missing code_challenge_method")) + return + } + + if challenge == "" { + httpError(rw, http.StatusBadRequest, xerrors.New("missing code_challenge")) + return + } + } + clientID := r.URL.Query().Get("client_id") if !assert.Equal(t, f.clientID, clientID, "unexpected client_id") { httpError(rw, http.StatusBadRequest, xerrors.New("invalid client_id")) @@ -959,7 +1000,7 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { q := ru.Query() q.Set("state", state) - q.Set("code", f.newCode(state)) + q.Set("code", f.newCode(state, challenge)) ru.RawQuery = q.Encode() http.Redirect(rw, r, ru.String(), http.StatusTemporaryRedirect) @@ -1009,6 +1050,21 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { http.Error(rw, "invalid code", http.StatusBadRequest) return } + + if f.pkce { + challenge, ok := f.codeToChallengeMap.Load(code) + if !ok { + httpError(rw, http.StatusBadRequest, xerrors.New("code challenge not found for code")) + return + } + codeVerifier := values.Get("code_verifier") + expecter := oauth2.S256ChallengeFromVerifier(codeVerifier) + if challenge != expecter { + httpError(rw, http.StatusBadRequest, xerrors.New("invalid code verifier")) + return + } + } + // Always invalidate the code after it is used. f.codeToStateMap.Delete(code) diff --git a/coderd/externalauth.go b/coderd/externalauth.go index 23ae7e9fe2654..95978a5ac8b76 100644 --- a/coderd/externalauth.go +++ b/coderd/externalauth.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" ) @@ -417,14 +418,15 @@ func ExternalAuthConfigs(auths []*externalauth.Config) []codersdk.ExternalAuthLi func ExternalAuthConfig(cfg *externalauth.Config) codersdk.ExternalAuthLinkProvider { return codersdk.ExternalAuthLinkProvider{ - ID: cfg.ID, - Type: cfg.Type, - Device: cfg.DeviceAuth != nil, - DisplayName: cfg.DisplayName, - DisplayIcon: cfg.DisplayIcon, - AllowRefresh: !cfg.NoRefresh, - AllowValidate: cfg.ValidateURL != "", - SupportsRevocation: cfg.RevokeURL != "", + ID: cfg.ID, + Type: cfg.Type, + Device: cfg.DeviceAuth != nil, + DisplayName: cfg.DisplayName, + DisplayIcon: cfg.DisplayIcon, + AllowRefresh: !cfg.NoRefresh, + AllowValidate: cfg.ValidateURL != "", + SupportsRevocation: cfg.RevokeURL != "", + CodeChallengeMethodsSupported: slice.ToStrings(cfg.CodeChallengeMethodsSupported), } } diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index f33a9d36700b8..d730d8cc7d7dd 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -25,6 +25,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/promoauth" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk" "github.com/coder/retry" ) @@ -102,7 +103,8 @@ type Config struct { // injected into Coder AI Bridge upstream requests. // In the case of conflicts, items evaluated by this list override [MCPToolAllowRegex]. // This field can be nil if unspecified in the config. - MCPToolDenyRegex *regexp.Regexp + MCPToolDenyRegex *regexp.Regexp + CodeChallengeMethodsSupported []promoauth.Oauth2PKCEChallengeMethod } // GenerateTokenExtra generates the extra token data to store in the database. @@ -723,24 +725,25 @@ func ConvertConfig(instrument *promoauth.Factory, entries []codersdk.ExternalAut } cfg := &Config{ - InstrumentedOAuth2Config: instrumented, - ID: entry.ID, - ClientID: entry.ClientID, - ClientSecret: entry.ClientSecret, - Regex: regex, - Type: entry.Type, - NoRefresh: entry.NoRefresh, - ValidateURL: entry.ValidateURL, - RevokeURL: entry.RevokeURL, - RevokeTimeout: tokenRevocationTimeout, - AppInstallationsURL: entry.AppInstallationsURL, - AppInstallURL: entry.AppInstallURL, - DisplayName: entry.DisplayName, - DisplayIcon: entry.DisplayIcon, - ExtraTokenKeys: entry.ExtraTokenKeys, - MCPURL: entry.MCPURL, - MCPToolAllowRegex: mcpToolAllow, - MCPToolDenyRegex: mcpToolDeny, + InstrumentedOAuth2Config: instrumented, + ID: entry.ID, + ClientID: entry.ClientID, + ClientSecret: entry.ClientSecret, + Regex: regex, + Type: entry.Type, + NoRefresh: entry.NoRefresh, + ValidateURL: entry.ValidateURL, + RevokeURL: entry.RevokeURL, + RevokeTimeout: tokenRevocationTimeout, + AppInstallationsURL: entry.AppInstallationsURL, + AppInstallURL: entry.AppInstallURL, + DisplayName: entry.DisplayName, + DisplayIcon: entry.DisplayIcon, + ExtraTokenKeys: entry.ExtraTokenKeys, + MCPURL: entry.MCPURL, + MCPToolAllowRegex: mcpToolAllow, + MCPToolDenyRegex: mcpToolDeny, + CodeChallengeMethodsSupported: slice.StringEnums[promoauth.Oauth2PKCEChallengeMethod](entry.CodeChallengeMethodsSupported), } if entry.DeviceFlow { @@ -800,8 +803,7 @@ func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig) { copyDefaultSettings(config, azureDevopsEntraDefaults(config)) return default: - // No defaults for this type. We still want to run this apply with - // an empty set of defaults. + // Global defaults are specified at the end of the `copyDefaultSettings` function. copyDefaultSettings(config, codersdk.ExternalAuthConfig{}) return } @@ -844,6 +846,9 @@ func copyDefaultSettings(config *codersdk.ExternalAuthConfig, defaults codersdk. if len(config.ExtraTokenKeys) == 0 { config.ExtraTokenKeys = defaults.ExtraTokenKeys } + if config.CodeChallengeMethodsSupported == nil { + config.CodeChallengeMethodsSupported = defaults.CodeChallengeMethodsSupported + } // Apply defaults if it's still empty... if config.ID == "" { @@ -856,6 +861,9 @@ func copyDefaultSettings(config *codersdk.ExternalAuthConfig, defaults codersdk. // This is a key emoji. config.DisplayIcon = "/emojis/1f511.png" } + if config.CodeChallengeMethodsSupported == nil { + config.CodeChallengeMethodsSupported = []string{string(promoauth.PKCEChallengeMethodSha256)} + } } // gitHubDefaults returns default config values for GitHub. @@ -869,9 +877,10 @@ func gitHubDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthCo DisplayIcon: "/icon/github.svg", Regex: `^(https?://)?github\.com(/.*)?$`, // "workflow" is required for managing GitHub Actions in a repository. - Scopes: []string{"repo", "workflow"}, - DeviceCodeURL: "https://github.com/login/device/code", - AppInstallationsURL: "https://api.github.com/user/installations", + Scopes: []string{"repo", "workflow"}, + DeviceCodeURL: "https://github.com/login/device/code", + AppInstallationsURL: "https://api.github.com/user/installations", + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, } if config.RevokeURL == "" && config.ClientID != "" { @@ -886,6 +895,8 @@ func bitbucketServerDefaults(config *codersdk.ExternalAuthConfig) codersdk.Exter DisplayName: "Bitbucket Server", Scopes: []string{"PUBLIC_REPOS", "REPO_READ", "REPO_WRITE"}, DisplayIcon: "/icon/bitbucket.svg", + // TODO: PKCE support? Test 'S256' as the string value + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, } // 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, @@ -923,14 +934,15 @@ func bitbucketServerDefaults(config *codersdk.ExternalAuthConfig) codersdk.Exter // Any user specific fields will override this if provided. func gitlabDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthConfig { cloud := codersdk.ExternalAuthConfig{ - AuthURL: "https://gitlab.com/oauth/authorize", - TokenURL: "https://gitlab.com/oauth/token", - ValidateURL: "https://gitlab.com/oauth/token/info", - RevokeURL: "https://gitlab.com/oauth/revoke", - DisplayName: "GitLab", - DisplayIcon: "/icon/gitlab.svg", - Regex: `^(https?://)?gitlab\.com(/.*)?$`, - Scopes: []string{"write_repository"}, + AuthURL: "https://gitlab.com/oauth/authorize", + TokenURL: "https://gitlab.com/oauth/token", + ValidateURL: "https://gitlab.com/oauth/token/info", + RevokeURL: "https://gitlab.com/oauth/revoke", + DisplayName: "GitLab", + DisplayIcon: "/icon/gitlab.svg", + Regex: `^(https?://)?gitlab\.com(/.*)?$`, + Scopes: []string{"write_repository"}, + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, } if config.AuthURL == "" || config.AuthURL == cloud.AuthURL { @@ -946,14 +958,15 @@ func gitlabDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthCo // At this point, assume it is self-hosted and use the AuthURL return codersdk.ExternalAuthConfig{ - DisplayName: cloud.DisplayName, - Scopes: cloud.Scopes, - DisplayIcon: cloud.DisplayIcon, - AuthURL: au.ResolveReference(&url.URL{Path: "/oauth/authorize"}).String(), - TokenURL: au.ResolveReference(&url.URL{Path: "/oauth/token"}).String(), - ValidateURL: au.ResolveReference(&url.URL{Path: "/oauth/token/info"}).String(), - RevokeURL: au.ResolveReference(&url.URL{Path: "/oauth/revoke"}).String(), - Regex: fmt.Sprintf(`^(https?://)?%s(/.*)?$`, strings.ReplaceAll(au.Host, ".", `\.`)), + DisplayName: cloud.DisplayName, + Scopes: cloud.Scopes, + DisplayIcon: cloud.DisplayIcon, + AuthURL: au.ResolveReference(&url.URL{Path: "/oauth/authorize"}).String(), + TokenURL: au.ResolveReference(&url.URL{Path: "/oauth/token"}).String(), + ValidateURL: au.ResolveReference(&url.URL{Path: "/oauth/token/info"}).String(), + RevokeURL: au.ResolveReference(&url.URL{Path: "/oauth/revoke"}).String(), + Regex: fmt.Sprintf(`^(https?://)?%s(/.*)?$`, strings.ReplaceAll(au.Host, ".", `\.`)), + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, } } @@ -962,6 +975,8 @@ func jfrogArtifactoryDefaults(config *codersdk.ExternalAuthConfig) codersdk.Exte DisplayName: "JFrog Artifactory", Scopes: []string{"applied-permissions/user"}, DisplayIcon: "/icon/jfrog.svg", + // TODO: PKCE support? Test 'S256' as the string value + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, } // Artifactory servers will have some base url, e.g. https://jfrog.coder.com. // We will grab this from the Auth URL. This choice is not arbitrary. It is a @@ -997,9 +1012,10 @@ func jfrogArtifactoryDefaults(config *codersdk.ExternalAuthConfig) codersdk.Exte func giteaDefaults(config *codersdk.ExternalAuthConfig) codersdk.ExternalAuthConfig { defaults := codersdk.ExternalAuthConfig{ - DisplayName: "Gitea", - Scopes: []string{"read:repository", " write:repository", "read:user"}, - DisplayIcon: "/icon/gitea.svg", + DisplayName: "Gitea", + Scopes: []string{"read:repository", " write:repository", "read:user"}, + DisplayIcon: "/icon/gitea.svg", + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, } // Gitea's servers will have some base url, e.g: https://gitea.coder.com. // If an auth url is not set, we will assume they are using the default @@ -1031,6 +1047,8 @@ func azureDevopsEntraDefaults(config *codersdk.ExternalAuthConfig) codersdk.Exte DisplayName: "Azure DevOps (Entra)", DisplayIcon: "/icon/azure-devops.svg", Regex: `^(https?://)?dev\.azure\.com(/.*)?$`, + // TODO: PKCE support? Test 'S256' as the string value + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, } // The tenant ID is required for urls and is in the auth url. if config.AuthURL == "" { @@ -1069,6 +1087,8 @@ var staticDefaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.External DisplayIcon: "/icon/azure-devops.svg", Regex: `^(https?://)?dev\.azure\.com(/.*)?$`, Scopes: []string{"vso.code_write"}, + // TODO: PKCE support? Test 'S256' as the string value + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, codersdk.EnhancedExternalAuthProviderBitBucketCloud: { AuthURL: "https://bitbucket.org/site/oauth2/authorize", @@ -1078,6 +1098,8 @@ var staticDefaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.External DisplayIcon: "/icon/bitbucket.svg", Regex: `^(https?://)?bitbucket\.org(/.*)?$`, Scopes: []string{"account", "repository:write"}, + // TODO: PKCE support? Test 'S256' as the string value + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, codersdk.EnhancedExternalAuthProviderSlack: { AuthURL: "https://slack.com/oauth/v2/authorize", @@ -1087,6 +1109,8 @@ var staticDefaults = map[codersdk.EnhancedExternalAuthProvider]codersdk.External DisplayIcon: "/icon/slack.svg", // See: https://api.slack.com/authentication/oauth-v2#exchanging ExtraTokenKeys: []string{"authed_user"}, + // TODO: PKCE support? Test 'S256' as the string value + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, } diff --git a/coderd/externalauth/externalauth_internal_test.go b/coderd/externalauth/externalauth_internal_test.go index 65bb5ee7deb62..f88299412eb82 100644 --- a/coderd/externalauth/externalauth_internal_test.go +++ b/coderd/externalauth/externalauth_internal_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/coderd/promoauth" "github.com/coder/coder/v2/codersdk" ) @@ -13,17 +14,20 @@ func TestGitlabDefaults(t *testing.T) { // The default cloud setup. Copying this here as hard coded // values. - cloud := codersdk.ExternalAuthConfig{ - Type: string(codersdk.EnhancedExternalAuthProviderGitLab), - ID: string(codersdk.EnhancedExternalAuthProviderGitLab), - AuthURL: "https://gitlab.com/oauth/authorize", - TokenURL: "https://gitlab.com/oauth/token", - ValidateURL: "https://gitlab.com/oauth/token/info", - RevokeURL: "https://gitlab.com/oauth/revoke", - DisplayName: "GitLab", - DisplayIcon: "/icon/gitlab.svg", - Regex: `^(https?://)?gitlab\.com(/.*)?$`, - Scopes: []string{"write_repository"}, + cloud := func() codersdk.ExternalAuthConfig { + return codersdk.ExternalAuthConfig{ + Type: string(codersdk.EnhancedExternalAuthProviderGitLab), + ID: string(codersdk.EnhancedExternalAuthProviderGitLab), + AuthURL: "https://gitlab.com/oauth/authorize", + TokenURL: "https://gitlab.com/oauth/token", + ValidateURL: "https://gitlab.com/oauth/token/info", + RevokeURL: "https://gitlab.com/oauth/revoke", + DisplayName: "GitLab", + DisplayIcon: "/icon/gitlab.svg", + Regex: `^(https?://)?gitlab\.com(/.*)?$`, + Scopes: []string{"write_repository"}, + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, + } } tests := []struct { @@ -38,7 +42,7 @@ func TestGitlabDefaults(t *testing.T) { input: codersdk.ExternalAuthConfig{ Type: string(codersdk.EnhancedExternalAuthProviderGitLab), }, - expected: cloud, + expected: cloud(), }, { // If someone was to manually configure the gitlab cli. @@ -47,7 +51,7 @@ func TestGitlabDefaults(t *testing.T) { Type: string(codersdk.EnhancedExternalAuthProviderGitLab), AuthURL: "https://gitlab.com/oauth/authorize", }, - expected: cloud, + expected: cloud(), }, { // Changing some of the defaults of the cloud option @@ -60,7 +64,7 @@ func TestGitlabDefaults(t *testing.T) { DisplayName: "custom", Regex: ".*", }, - expected: cloud, + expected: cloud(), mutateExpected: func(config *codersdk.ExternalAuthConfig) { config.AuthURL = "https://gitlab.com/oauth/authorize?foo=bar" config.DisplayName = "custom" @@ -75,7 +79,7 @@ func TestGitlabDefaults(t *testing.T) { Type: string(codersdk.EnhancedExternalAuthProviderGitLab), AuthURL: "https://gitlab.company.org/oauth/authorize?foo=bar", }, - expected: cloud, + expected: cloud(), mutateExpected: func(config *codersdk.ExternalAuthConfig) { config.AuthURL = "https://gitlab.company.org/oauth/authorize?foo=bar" config.ValidateURL = "https://gitlab.company.org/oauth/token/info" @@ -88,20 +92,22 @@ func TestGitlabDefaults(t *testing.T) { // Strange values name: "RandomValues", input: codersdk.ExternalAuthConfig{ - Type: string(codersdk.EnhancedExternalAuthProviderGitLab), - AuthURL: "https://auth.com/auth", - ValidateURL: "https://validate.com/validate", - TokenURL: "https://token.com/token", - RevokeURL: "https://token.com/revoke", - Regex: "random", + Type: string(codersdk.EnhancedExternalAuthProviderGitLab), + AuthURL: "https://auth.com/auth", + ValidateURL: "https://validate.com/validate", + TokenURL: "https://token.com/token", + RevokeURL: "https://token.com/revoke", + Regex: "random", + CodeChallengeMethodsSupported: []string{"random"}, }, - expected: cloud, + expected: cloud(), mutateExpected: func(config *codersdk.ExternalAuthConfig) { config.AuthURL = "https://auth.com/auth" config.ValidateURL = "https://validate.com/validate" config.TokenURL = "https://token.com/token" config.RevokeURL = "https://token.com/revoke" config.Regex = `random` + config.CodeChallengeMethodsSupported = []string{"random"} }, }, } @@ -133,11 +139,12 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { Type: bbType, }, expected: codersdk.ExternalAuthConfig{ - Type: bbType, - ID: bbType, - DisplayName: "Bitbucket Server", - Scopes: []string{"PUBLIC_REPOS", "REPO_READ", "REPO_WRITE"}, - DisplayIcon: "/icon/bitbucket.svg", + Type: bbType, + ID: bbType, + DisplayName: "Bitbucket Server", + Scopes: []string{"PUBLIC_REPOS", "REPO_READ", "REPO_WRITE"}, + DisplayIcon: "/icon/bitbucket.svg", + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, }, { @@ -148,15 +155,16 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { 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", + 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", + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, }, { @@ -167,15 +175,16 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { 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"}, + 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"}, + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, }, } @@ -187,3 +196,45 @@ func Test_bitbucketServerConfigDefaults(t *testing.T) { }) } } + +func TestUntyped(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input codersdk.ExternalAuthConfig + expected codersdk.ExternalAuthConfig + }{ + { + // Unknown Type uses S256 by default. + name: "RandomValues", + input: codersdk.ExternalAuthConfig{ + Type: "unknown", + AuthURL: "https://auth.com/auth", + ValidateURL: "https://validate.com/validate", + TokenURL: "https://token.com/token", + RevokeURL: "https://token.com/revoke", + Regex: "random", + }, + expected: codersdk.ExternalAuthConfig{ + ID: "unknown", + Type: "unknown", + DisplayName: "unknown", + DisplayIcon: "/emojis/1f511.png", + AuthURL: "https://auth.com/auth", + ValidateURL: "https://validate.com/validate", + TokenURL: "https://token.com/token", + RevokeURL: "https://token.com/revoke", + Regex: `random`, + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, + }, + }, + } + for _, c := range tests { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + applyDefaultsToConfig(&c.input) + require.Equal(t, c.input, c.expected) + }) + } +} diff --git a/coderd/externalauth/externalauth_test.go b/coderd/externalauth/externalauth_test.go index 670d1cbf1123b..61fdbb2de539d 100644 --- a/coderd/externalauth/externalauth_test.go +++ b/coderd/externalauth/externalauth_test.go @@ -729,6 +729,7 @@ func TestConstantQueryParams(t *testing.T) { authURL.RawQuery = url.Values{constantQueryParamKey: []string{constantQueryParamValue}}.Encode() cfg.OAuth2Config.(*oauth2.Config).Endpoint.AuthURL = authURL.String() require.Contains(t, cfg.OAuth2Config.(*oauth2.Config).Endpoint.AuthURL, constantQueryParam) + cfg.PKCEMethods = []promoauth.Oauth2PKCEChallengeMethod{promoauth.PKCEChallengeMethodSha256} }, }, }) @@ -792,7 +793,7 @@ func setupOauth2Test(t *testing.T, settings testConfig) (*oidctest.FakeIDP, *ext const providerID = "test-idp" fake := oidctest.NewFakeIDP(t, - append([]oidctest.FakeIDPOpt{}, settings.FakeIDPOpts...)..., + append([]oidctest.FakeIDPOpt{oidctest.WithPKCE()}, settings.FakeIDPOpts...)..., ) f := promoauth.NewFactory(prometheus.NewRegistry()) @@ -800,12 +801,13 @@ func setupOauth2Test(t *testing.T, settings testConfig) (*oidctest.FakeIDP, *ext config := &externalauth.Config{ InstrumentedOAuth2Config: f.New("test-oauth2", fake.OIDCConfig(t, nil, settings.CoderOIDCConfigOpts...)), - ID: providerID, - ClientID: cid, - ClientSecret: cs, - ValidateURL: fake.WellknownConfig().UserInfoURL, - RevokeURL: fake.WellknownConfig().RevokeURL, - RevokeTimeout: 1 * time.Second, + ID: providerID, + ClientID: cid, + ClientSecret: cs, + ValidateURL: fake.WellknownConfig().UserInfoURL, + RevokeURL: fake.WellknownConfig().RevokeURL, + RevokeTimeout: 1 * time.Second, + CodeChallengeMethodsSupported: []promoauth.Oauth2PKCEChallengeMethod{promoauth.PKCEChallengeMethodSha256}, } settings.ExternalAuthOpt(config) diff --git a/coderd/externalauth_test.go b/coderd/externalauth_test.go index 5219b54344320..343f151a2b05c 100644 --- a/coderd/externalauth_test.go +++ b/coderd/externalauth_test.go @@ -26,6 +26,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/promoauth" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/provisioner/echo" @@ -34,6 +35,24 @@ import ( func TestExternalAuthByID(t *testing.T) { t.Parallel() + t.Run("PKCEMissing", func(t *testing.T) { + t.Parallel() + const providerID = "fake-github" + fake := oidctest.NewFakeIDP(t, oidctest.WithServing()) + + client := coderdtest.New(t, &coderdtest.Options{ + ExternalAuthConfigs: []*externalauth.Config{ + fake.ExternalAuthConfig(t, providerID, nil, func(cfg *externalauth.Config) { + cfg.Type = codersdk.EnhancedExternalAuthProviderGitHub.String() + cfg.CodeChallengeMethodsSupported = []promoauth.Oauth2PKCEChallengeMethod{} + }), + }, + }) + coderdtest.CreateFirstUser(t, client) + auth, err := client.ExternalAuthByID(context.Background(), providerID) + require.NoError(t, err) + require.False(t, auth.Authenticated) + }) t.Run("Unauthenticated", func(t *testing.T) { t.Parallel() const providerID = "fake-github" diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index 28e6400c8a5a4..9cd8c38d0eedd 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -6,6 +6,7 @@ import ( "net/http" "net/url" "reflect" + "slices" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -40,13 +41,19 @@ func OAuth2(r *http.Request) OAuth2State { // a "code" URL parameter will be redirected. // AuthURLOpts are passed to the AuthCodeURL function. If this is nil, // the default option oauth2.AccessTypeOffline will be used. -func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg codersdk.HTTPCookieConfig, authURLOpts map[string]string) func(http.Handler) http.Handler { +// +// pkceMethods should be a list like ['S256', 'plain'] indicating +// which PKCE methods are supported by the OAuth2 provider. If empty, +// PKCE will not be used. +func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg codersdk.HTTPCookieConfig, authURLOpts map[string]string, pkceMethods []promoauth.Oauth2PKCEChallengeMethod) func(http.Handler) http.Handler { opts := make([]oauth2.AuthCodeOption, 0, len(authURLOpts)+1) opts = append(opts, oauth2.AccessTypeOffline) for k, v := range authURLOpts { opts = append(opts, oauth2.SetAuthURLParam(k, v)) } + // Only S256 PKCE is currently supported. + sha256PKCESupported := slices.Contains(pkceMethods, promoauth.PKCEChallengeMethodSha256) return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -133,7 +140,20 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg HttpOnly: true, })) - http.Redirect(rw, r, config.AuthCodeURL(state, opts...), http.StatusTemporaryRedirect) + authOpts := slices.Clone(opts) + if sha256PKCESupported { + verifier := oauth2.GenerateVerifier() + authOpts = append(authOpts, oauth2.S256ChallengeOption(verifier)) + + http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{ + Name: codersdk.OAuth2PKCEChallenge, + Value: verifier, + Path: "/", + HttpOnly: true, + })) + } + + http.Redirect(rw, r, config.AuthCodeURL(state, authOpts...), http.StatusTemporaryRedirect) return } @@ -163,7 +183,19 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg redirect = stateRedirect.Value } - oauthToken, err := config.Exchange(ctx, code) + exchangeOpts := make([]oauth2.AuthCodeOption, 0) + if sha256PKCESupported { + pkceChallenge, err := r.Cookie(codersdk.OAuth2PKCEChallenge) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "PKCE challenge must be provided.", + }) + return + } + exchangeOpts = append(exchangeOpts, oauth2.VerifierOption(pkceChallenge.Value)) + } + + oauthToken, err := config.Exchange(ctx, code, exchangeOpts...) if err != nil { errorCode := http.StatusInternalServerError detail := err.Error() diff --git a/coderd/httpmw/oauth2_test.go b/coderd/httpmw/oauth2_test.go index 9739735f3eaf7..baedd2cc2fe6b 100644 --- a/coderd/httpmw/oauth2_test.go +++ b/coderd/httpmw/oauth2_test.go @@ -50,7 +50,7 @@ func TestOAuth2(t *testing.T) { t.Parallel() req := httptest.NewRequest("GET", "/", nil) res := httptest.NewRecorder() - httpmw.ExtractOAuth2(nil, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(nil, nil, codersdk.HTTPCookieConfig{}, nil, nil)(nil).ServeHTTP(res, req) require.Equal(t, http.StatusBadRequest, res.Result().StatusCode) }) t.Run("RedirectWithoutCode", func(t *testing.T) { @@ -58,7 +58,7 @@ func TestOAuth2(t *testing.T) { req := httptest.NewRequest("GET", "/?redirect="+url.QueryEscape("/dashboard"), nil) res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil, nil)(nil).ServeHTTP(res, req) location := res.Header().Get("Location") if !assert.NotEmpty(t, location) { return @@ -82,7 +82,7 @@ func TestOAuth2(t *testing.T) { req := httptest.NewRequest("GET", "/?redirect="+url.QueryEscape(uri.String()), nil) res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil, nil)(nil).ServeHTTP(res, req) location := res.Header().Get("Location") if !assert.NotEmpty(t, location) { return @@ -97,7 +97,7 @@ func TestOAuth2(t *testing.T) { req := httptest.NewRequest("GET", "/?code=something", nil) res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil, nil)(nil).ServeHTTP(res, req) require.Equal(t, http.StatusBadRequest, res.Result().StatusCode) }) t.Run("NoStateCookie", func(t *testing.T) { @@ -105,7 +105,7 @@ func TestOAuth2(t *testing.T) { req := httptest.NewRequest("GET", "/?code=something&state=test", nil) res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil, nil)(nil).ServeHTTP(res, req) require.Equal(t, http.StatusUnauthorized, res.Result().StatusCode) }) t.Run("MismatchedState", func(t *testing.T) { @@ -117,7 +117,7 @@ func TestOAuth2(t *testing.T) { }) res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil, nil)(nil).ServeHTTP(res, req) require.Equal(t, http.StatusUnauthorized, res.Result().StatusCode) }) t.Run("ExchangeCodeAndState", func(t *testing.T) { @@ -133,7 +133,7 @@ func TestOAuth2(t *testing.T) { }) res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil, nil)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { state := httpmw.OAuth2(r) require.Equal(t, "/dashboard", state.Redirect) })).ServeHTTP(res, req) @@ -144,7 +144,7 @@ func TestOAuth2(t *testing.T) { res := httptest.NewRecorder() tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline, oauth2.SetAuthURLParam("foo", "bar")) authOpts := map[string]string{"foo": "bar"} - httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, authOpts)(nil).ServeHTTP(res, req) + httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, authOpts, nil)(nil).ServeHTTP(res, req) location := res.Header().Get("Location") // Ideally we would also assert that the location contains the query params // we set in the auth URL but this would essentially be testing the oauth2 package. @@ -160,7 +160,7 @@ func TestOAuth2(t *testing.T) { httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{ Secure: true, SameSite: "none", - }, nil)(nil).ServeHTTP(res, req) + }, nil, nil)(nil).ServeHTTP(res, req) found := false for _, cookie := range res.Result().Cookies() { diff --git a/coderd/promoauth/oauth2.go b/coderd/promoauth/oauth2.go index a89875cb75508..6ab54b604f5ec 100644 --- a/coderd/promoauth/oauth2.go +++ b/coderd/promoauth/oauth2.go @@ -11,6 +11,13 @@ import ( "golang.org/x/oauth2" ) +type Oauth2PKCEChallengeMethod string + +const ( + PKCEChallengeMethodSha256 Oauth2PKCEChallengeMethod = "S256" + PKCEChallengeMethodNone Oauth2PKCEChallengeMethod = "" +) + type Oauth2Source string const ( diff --git a/coderd/userauth.go b/coderd/userauth.go index 91472996737aa..388c067257e98 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -1172,6 +1172,15 @@ type OIDCConfig struct { IconURL string // SignupsDisabledText is the text do display on the static error page. SignupsDisabledText string + PKCEMethods []promoauth.Oauth2PKCEChallengeMethod +} + +// PKCESupported is to prevent nil pointer dereference. +func (o *OIDCConfig) PKCESupported() []promoauth.Oauth2PKCEChallengeMethod { + if o == nil { + return nil + } + return o.PKCEMethods } // @Summary OpenID Connect Callback diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 86fe30bf3c0a8..ab57e946b4440 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -1017,6 +1017,10 @@ func TestUserOAuth2Github(t *testing.T) { Name: "oauth_state", Value: "somestate", }) + req.AddCookie(&http.Cookie{ + Name: codersdk.OAuth2PKCEChallenge, + Value: oauth2.GenerateVerifier(), + }) require.NoError(t, err) res, err = client.HTTPClient.Do(req) require.NoError(t, err) @@ -2460,6 +2464,10 @@ func oauth2Callback(t *testing.T, client *codersdk.Client, opts ...func(*http.Re Name: codersdk.OAuth2StateCookie, Value: state, }) + req.AddCookie(&http.Cookie{ + Name: codersdk.OAuth2PKCEChallenge, + Value: oauth2.GenerateVerifier(), + }) res, err := client.HTTPClient.Do(req) require.NoError(t, err) t.Cleanup(func() { diff --git a/codersdk/client.go b/codersdk/client.go index 72dd7ac4b64f4..b93aeb80a0bdc 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -38,6 +38,8 @@ const ( SessionTokenHeader = "Coder-Session-Token" // OAuth2StateCookie is the name of the cookie that stores the oauth2 state. OAuth2StateCookie = "oauth_state" + + OAuth2PKCEChallenge = "oauth_pkce_challenge" // OAuth2RedirectCookie is the name of the cookie that stores the oauth2 redirect. OAuth2RedirectCookie = "oauth_redirect" diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 0dd082ab5eebc..2c7ebb21c7b5a 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -766,7 +766,8 @@ type ExternalAuthConfig struct { // DisplayName is shown in the UI to identify the auth config. 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" yaml:"display_icon"` + DisplayIcon string `json:"display_icon" yaml:"display_icon"` + CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported" yaml:"code_challenge_methods_supported"` } type ProvisionerConfig struct { diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 8b811480d5289..7407bf40b712c 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -396,27 +396,28 @@ func TestExternalAuthYAMLConfig(t *testing.T) { 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", - RevokeURL: "https://example.com/revoke", - 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", - MCPURL: "https://api.githubcopilot.com/mcp/", - MCPToolAllowRegex: ".*", - MCPToolDenyRegex: "create_gist", + 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", + RevokeURL: "https://example.com/revoke", + 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", + MCPURL: "https://api.githubcopilot.com/mcp/", + MCPToolAllowRegex: ".*", + MCPToolDenyRegex: "create_gist", + CodeChallengeMethodsSupported: []string{"S256"}, } // Input the github section twice for testing a slice of configs. diff --git a/codersdk/externalauth.go b/codersdk/externalauth.go index 48c4781605d07..70c0060a73736 100644 --- a/codersdk/externalauth.go +++ b/codersdk/externalauth.go @@ -94,14 +94,15 @@ type ExternalAuthLink struct { // ExternalAuthLinkProvider are the static details of a provider. type ExternalAuthLinkProvider struct { - ID string `json:"id"` - Type string `json:"type"` - Device bool `json:"device"` - DisplayName string `json:"display_name"` - DisplayIcon string `json:"display_icon"` - AllowRefresh bool `json:"allow_refresh"` - AllowValidate bool `json:"allow_validate"` - SupportsRevocation bool `json:"supports_revocation"` + ID string `json:"id"` + Type string `json:"type"` + Device bool `json:"device"` + DisplayName string `json:"display_name"` + DisplayIcon string `json:"display_icon"` + AllowRefresh bool `json:"allow_refresh"` + AllowValidate bool `json:"allow_validate"` + SupportsRevocation bool `json:"supports_revocation"` + CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"` } type ExternalAuthAppInstallation struct { diff --git a/codersdk/testdata/githubcfg.yaml b/codersdk/testdata/githubcfg.yaml index c5e61baa030c4..838d8f0c2ee71 100644 --- a/codersdk/testdata/githubcfg.yaml +++ b/codersdk/testdata/githubcfg.yaml @@ -24,3 +24,5 @@ externalAuthProviders: regex: ^https://example.com/.*$ display_name: GitHub display_icon: /static/icons/github.svg + code_challenge_methods_supported: + - S256 diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index 3ea0180ae1454..224e1c043fa33 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -260,6 +260,9 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "app_installations_url": "string", "auth_url": "string", "client_id": "string", + "code_challenge_methods_supported": [ + "string" + ], "device_code_url": "string", "device_flow": true, "display_icon": "string", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index bd00d79c4b40b..76e9318a99f7f 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -2944,6 +2944,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "app_installations_url": "string", "auth_url": "string", "client_id": "string", + "code_challenge_methods_supported": [ + "string" + ], "device_code_url": "string", "device_flow": true, "display_icon": "string", @@ -3467,6 +3470,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "app_installations_url": "string", "auth_url": "string", "client_id": "string", + "code_challenge_methods_supported": [ + "string" + ], "device_code_url": "string", "device_flow": true, "display_icon": "string", @@ -4202,6 +4208,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "app_installations_url": "string", "auth_url": "string", "client_id": "string", + "code_challenge_methods_supported": [ + "string" + ], "device_code_url": "string", "device_flow": true, "display_icon": "string", @@ -4224,21 +4233,22 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -|-------------------------|---------|----------|--------------|-----------------------------------------------------------------------------------------| -| `app_install_url` | string | false | | | -| `app_installations_url` | string | false | | | -| `auth_url` | string | false | | | -| `client_id` | string | false | | | -| `device_code_url` | string | false | | | -| `device_flow` | boolean | false | | | -| `display_icon` | string | false | | Display icon is a URL to an icon to display in the UI. | -| `display_name` | string | false | | Display name is shown in the UI to identify the auth config. | -| `id` | string | false | | ID is a unique identifier for the auth config. It defaults to `type` when not provided. | -| `mcp_tool_allow_regex` | string | false | | | -| `mcp_tool_deny_regex` | string | false | | | -| `mcp_url` | string | false | | | -| `no_refresh` | boolean | false | | | +| Name | Type | Required | Restrictions | Description | +|------------------------------------|-----------------|----------|--------------|-----------------------------------------------------------------------------------------| +| `app_install_url` | string | false | | | +| `app_installations_url` | string | false | | | +| `auth_url` | string | false | | | +| `client_id` | string | false | | | +| `code_challenge_methods_supported` | array of string | false | | | +| `device_code_url` | string | false | | | +| `device_flow` | boolean | false | | | +| `display_icon` | string | false | | Display icon is a URL to an icon to display in the UI. | +| `display_name` | string | false | | Display name is shown in the UI to identify the auth config. | +| `id` | string | false | | ID is a unique identifier for the auth config. It defaults to `type` when not provided. | +| `mcp_tool_allow_regex` | string | false | | | +| `mcp_tool_deny_regex` | string | false | | | +| `mcp_url` | string | false | | | +| `no_refresh` | boolean | false | | | |`regex`|string|false||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.| |`revoke_url`|string|false||| @@ -13931,6 +13941,9 @@ None "app_installations_url": "string", "auth_url": "string", "client_id": "string", + "code_challenge_methods_supported": [ + "string" + ], "device_code_url": "string", "device_flow": true, "display_icon": "string", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6cb14744035cc..53a8aa658d113 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2001,6 +2001,7 @@ export interface ExternalAuthConfig { * DisplayIcon is a URL to an icon to display in the UI. */ readonly display_icon: string; + readonly code_challenge_methods_supported: readonly string[]; } // From codersdk/externalauth.go @@ -2050,6 +2051,7 @@ export interface ExternalAuthLinkProvider { readonly allow_refresh: boolean; readonly allow_validate: boolean; readonly supports_revocation: boolean; + readonly code_challenge_methods_supported: readonly string[]; } // From codersdk/externalauth.go @@ -3043,6 +3045,9 @@ export interface OAuth2GithubConfig { readonly enterprise_base_url: string; } +// From codersdk/client.go +export const OAuth2PKCEChallenge = "oauth_pkce_challenge"; + // From codersdk/oauth2.go /** * OAuth2ProtectedResourceMetadata represents RFC 9728 OAuth 2.0 Protected Resource Metadata diff --git a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx index a558918e71165..a37c490fc0b77 100644 --- a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx @@ -27,6 +27,7 @@ const meta: Meta = { mcp_url: "", mcp_tool_allow_regex: "", mcp_tool_deny_regex: "", + code_challenge_methods_supported: ["S256"], }, ], }, diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index bb65146e3afc3..d180c383a4069 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -4502,6 +4502,7 @@ export const MockGithubExternalProvider: TypesGen.ExternalAuthLinkProvider = { allow_refresh: true, allow_validate: true, supports_revocation: false, + code_challenge_methods_supported: ["S256"], }; export const MockGithubAuthLink: TypesGen.ExternalAuthLink = {