From 08f52e96c8348e61175cea4a4fab036c0f287c78 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 10 Dec 2025 15:15:21 -0600 Subject: [PATCH 01/18] feat: oauth2 client to use pkce in auth/exchange flow Used in coder primary auth --- coderd/httpmw/oauth2.go | 17 +++++++++++++++-- codersdk/client.go | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index 28e6400c8a5a4..f18aeed66b6e4 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -133,7 +133,14 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg HttpOnly: true, })) - http.Redirect(rw, r, config.AuthCodeURL(state, opts...), http.StatusTemporaryRedirect) + var verifier = oauth2.GenerateVerifier() + http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{ + Name: codersdk.OAuth2PKCEChallenge, + Value: verifier, + Path: "/", + HttpOnly: true, + })) + http.Redirect(rw, r, config.AuthCodeURL(state, append(opts, oauth2.S256ChallengeOption(verifier))...), http.StatusTemporaryRedirect) return } @@ -163,7 +170,13 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg redirect = stateRedirect.Value } - oauthToken, err := config.Exchange(ctx, code) + exchangeOpts := []oauth2.AuthCodeOption{} + pkceChallenge, err := r.Cookie(codersdk.OAuth2PKCEChallenge) + if err == nil { + 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/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" From 40283ea3835472b262f2fefa3cc84a725f2d4266 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 10 Dec 2025 15:41:16 -0600 Subject: [PATCH 02/18] force pkce --- coderd/httpmw/oauth2.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index f18aeed66b6e4..eaf101c099cfc 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" @@ -133,14 +134,18 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg HttpOnly: true, })) + authOpts := slices.Clone(opts) var 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, append(opts, oauth2.S256ChallengeOption(verifier))...), http.StatusTemporaryRedirect) + + http.Redirect(rw, r, config.AuthCodeURL(state, authOpts...), http.StatusTemporaryRedirect) return } @@ -170,13 +175,15 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg redirect = stateRedirect.Value } - exchangeOpts := []oauth2.AuthCodeOption{} pkceChallenge, err := r.Cookie(codersdk.OAuth2PKCEChallenge) - if err == nil { - exchangeOpts = append(exchangeOpts, oauth2.VerifierOption(pkceChallenge.Value)) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "PKCE challenge must be provided.", + }) + return } - oauthToken, err := config.Exchange(ctx, code, exchangeOpts...) + oauthToken, err := config.Exchange(ctx, code, oauth2.VerifierOption(pkceChallenge.Value)) if err != nil { errorCode := http.StatusInternalServerError detail := err.Error() From 5eb6e9f60dc795cb034a47108673b28989004792 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 10 Dec 2025 16:37:32 -0600 Subject: [PATCH 03/18] chore: pkce defaults --- cli/server.go | 10 ++ coderd/coderd.go | 8 +- coderd/externalauth/externalauth.go | 74 +++++++++----- .../externalauth_internal_test.go | 99 ++++++++++--------- coderd/httpmw/oauth2.go | 46 +++++---- coderd/promoauth/oauth2.go | 7 ++ coderd/userauth.go | 1 + codersdk/deployment.go | 5 +- 8 files changed, 157 insertions(+), 93 deletions(-) diff --git a/cli/server.go b/cli/server.go index e8e2d24de1873..4938e66a8d9e4 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,8 @@ 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(), + // We only support S256 PKCE challenge method. + PKCEMethods: pkceSupport.CodeChallengeMethodsSupported, }, nil } diff --git a/coderd/coderd.go b/coderd/coderd.go index b356f372dc56c..ddbe4fe424f57 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.PKCEMethods), ) r.Get("/", api.userOIDC) }) diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index f33a9d36700b8..e29c0ab574d77 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -102,7 +102,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. @@ -800,9 +801,11 @@ 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. - copyDefaultSettings(config, codersdk.ExternalAuthConfig{}) + // Defaults apply to any provider that doesn't have specific defaults. + copyDefaultSettings(config, codersdk.ExternalAuthConfig{ + // PKCE should always be enabled by default. + CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, + }) return } } @@ -856,6 +859,9 @@ func copyDefaultSettings(config *codersdk.ExternalAuthConfig, defaults codersdk. // This is a key emoji. config.DisplayIcon = "/emojis/1f511.png" } + if config.CodeChallengeMethodsSupported == nil { + config.CodeChallengeMethodsSupported = defaults.CodeChallengeMethodsSupported + } } // gitHubDefaults returns default config values for GitHub. @@ -869,9 +875,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 +893,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 +932,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 +956,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 +973,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 +1010,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 +1045,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 +1085,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 +1096,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 +1107,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..64e709252747a 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)}, }, }, } diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index eaf101c099cfc..1d05a9e86f8db 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -41,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() @@ -135,15 +141,17 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg })) authOpts := slices.Clone(opts) - var verifier = oauth2.GenerateVerifier() - authOpts = append(authOpts, oauth2.S256ChallengeOption(verifier)) - - http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{ - Name: codersdk.OAuth2PKCEChallenge, - Value: verifier, - Path: "/", - HttpOnly: true, - })) + if sha256PKCESupported { + var 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 @@ -175,15 +183,19 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg redirect = stateRedirect.Value } - 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 := 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, oauth2.VerifierOption(pkceChallenge.Value)) + oauthToken, err := config.Exchange(ctx, code, exchangeOpts...) if err != nil { errorCode := http.StatusInternalServerError detail := err.Error() 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..92c333a64108e 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -1172,6 +1172,7 @@ type OIDCConfig struct { IconURL string // SignupsDisabledText is the text do display on the static error page. SignupsDisabledText string + PKCEMethods []promoauth.Oauth2PKCEChallengeMethod } // @Summary OpenID Connect Callback diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 0dd082ab5eebc..d9032b99c1e6e 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 { @@ -1672,7 +1673,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Value: &c.DERP.Config.BlockDirect, Group: &deploymentGroupNetworkingDERP, YAML: "blockDirect", Annotations: serpent.Annotations{}. - Mark(annotationExternalProxies, "true"), + Mark(annotationExternalProxies, "true"), }, { Name: "DERP Force WebSockets", From 53fc86478143da6daa43024a11d12723cf09727c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 10 Dec 2025 17:04:09 -0600 Subject: [PATCH 04/18] frontend test --- site/src/api/typesGenerated.ts | 4 ++++ .../ExternalAuthSettingsPageView.stories.tsx | 1 + 2 files changed, 5 insertions(+) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6cb14744035cc..ce58b6296bf88 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 @@ -3043,6 +3044,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..6c2d5a36624d0 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"] }, ], }, From ead92c8cba03f2ab95ddfd0d2bba9cea88cb1564 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 10:07:20 -0600 Subject: [PATCH 05/18] chore: sane defaults and make gen --- cli/server.go | 2 + coderd/coderd.go | 2 +- coderd/externalauth.go | 18 ++++--- coderd/externalauth/externalauth.go | 50 ++++++++++--------- .../externalauth_internal_test.go | 42 ++++++++++++++++ coderd/httpmw/oauth2_test.go | 18 +++---- coderd/userauth.go | 8 +++ codersdk/externalauth.go | 17 ++++--- site/src/api/typesGenerated.ts | 1 + site/src/testHelpers/entities.ts | 1 + 10 files changed, 109 insertions(+), 50 deletions(-) diff --git a/cli/server.go b/cli/server.go index 4938e66a8d9e4..78d4d1dadd7a2 100644 --- a/cli/server.go +++ b/cli/server.go @@ -2771,6 +2771,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/coderd.go b/coderd/coderd.go index ddbe4fe424f57..29a678a6ba669 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1298,7 +1298,7 @@ func New(options *Options) *API { }) r.Route("/oidc/callback", func(r chi.Router) { r.Use( - httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, oidcAuthURLParams, options.OIDCConfig.PKCEMethods), + httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, oidcAuthURLParams, options.OIDCConfig.PKCESupported()), ) r.Get("/", api.userOIDC) }) 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 e29c0ab574d77..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" ) @@ -724,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 { @@ -801,11 +803,8 @@ func applyDefaultsToConfig(config *codersdk.ExternalAuthConfig) { copyDefaultSettings(config, azureDevopsEntraDefaults(config)) return default: - // Defaults apply to any provider that doesn't have specific defaults. - copyDefaultSettings(config, codersdk.ExternalAuthConfig{ - // PKCE should always be enabled by default. - CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodSha256)}, - }) + // Global defaults are specified at the end of the `copyDefaultSettings` function. + copyDefaultSettings(config, codersdk.ExternalAuthConfig{}) return } } @@ -847,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 == "" { @@ -860,7 +862,7 @@ func copyDefaultSettings(config *codersdk.ExternalAuthConfig, defaults codersdk. config.DisplayIcon = "/emojis/1f511.png" } if config.CodeChallengeMethodsSupported == nil { - config.CodeChallengeMethodsSupported = defaults.CodeChallengeMethodsSupported + config.CodeChallengeMethodsSupported = []string{string(promoauth.PKCEChallengeMethodSha256)} } } diff --git a/coderd/externalauth/externalauth_internal_test.go b/coderd/externalauth/externalauth_internal_test.go index 64e709252747a..f88299412eb82 100644 --- a/coderd/externalauth/externalauth_internal_test.go +++ b/coderd/externalauth/externalauth_internal_test.go @@ -196,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/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/userauth.go b/coderd/userauth.go index 92c333a64108e..388c067257e98 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -1175,6 +1175,14 @@ type OIDCConfig struct { 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 // @ID openid-connect-callback // @Security CoderSessionToken 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/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ce58b6296bf88..53a8aa658d113 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2051,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 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 = { From 0be0bb2b413599eca5c8bcfaf23a4b562ef75f4d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 10:23:53 -0600 Subject: [PATCH 06/18] make some tests use pkce --- coderd/coderdtest/oidctest/idp.go | 58 ++++++++++++++++++++++-- coderd/externalauth/externalauth_test.go | 15 +++--- coderd/externalauth_test.go | 19 ++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index d5215b9964a14..0d2a5eee34c2d 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". @@ -790,9 +808,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 +937,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 +994,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 +1044,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/externalauth_test.go b/coderd/externalauth/externalauth_test.go index 670d1cbf1123b..4bc4d59836fc2 100644 --- a/coderd/externalauth/externalauth_test.go +++ b/coderd/externalauth/externalauth_test.go @@ -792,7 +792,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 +800,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" From 303ea30315f8d1614b6a3f6502adf7e8086c8add Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 10:28:04 -0600 Subject: [PATCH 07/18] fmt --- cli/server.go | 3 +-- coderd/httpmw/oauth2.go | 2 +- codersdk/deployment.go | 2 +- .../ExternalAuthSettingsPageView.stories.tsx | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cli/server.go b/cli/server.go index 78d4d1dadd7a2..dbc35141e83b9 100644 --- a/cli/server.go +++ b/cli/server.go @@ -214,8 +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(), - // We only support S256 PKCE challenge method. - PKCEMethods: pkceSupport.CodeChallengeMethodsSupported, + PKCEMethods: pkceSupport.CodeChallengeMethodsSupported, }, nil } diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index 1d05a9e86f8db..9cd8c38d0eedd 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -142,7 +142,7 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg authOpts := slices.Clone(opts) if sha256PKCESupported { - var verifier = oauth2.GenerateVerifier() + verifier := oauth2.GenerateVerifier() authOpts = append(authOpts, oauth2.S256ChallengeOption(verifier)) http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{ diff --git a/codersdk/deployment.go b/codersdk/deployment.go index d9032b99c1e6e..2c7ebb21c7b5a 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1673,7 +1673,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Value: &c.DERP.Config.BlockDirect, Group: &deploymentGroupNetworkingDERP, YAML: "blockDirect", Annotations: serpent.Annotations{}. - Mark(annotationExternalProxies, "true"), + Mark(annotationExternalProxies, "true"), }, { Name: "DERP Force WebSockets", diff --git a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx index 6c2d5a36624d0..a37c490fc0b77 100644 --- a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx @@ -27,7 +27,7 @@ const meta: Meta = { mcp_url: "", mcp_tool_allow_regex: "", mcp_tool_deny_regex: "", - code_challenge_methods_supported: ["S256"] + code_challenge_methods_supported: ["S256"], }, ], }, From 7af6c3a81b9f5e29a4e2612dea2e49fa44190342 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 11:14:38 -0600 Subject: [PATCH 08/18] fix tests --- coderd/userauth_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) 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() { From 7f11ab74c98d2bd5a0912c6cae9ea619cafd383e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 17:25:41 +0000 Subject: [PATCH 09/18] make gen --- coderd/apidoc/docs.go | 6 +++++ coderd/apidoc/swagger.json | 6 +++++ docs/reference/api/general.md | 3 +++ docs/reference/api/schemas.md | 43 +++++++++++++++++++++++------------ 4 files changed, 43 insertions(+), 15 deletions(-) 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/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", From 65b08c8c150aaedd882c24ec903b6a1524c2a859 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 11:29:12 -0600 Subject: [PATCH 10/18] fix tests --- coderd/coderdtest/oidctest/idp.go | 8 +++++++- coderd/externalauth/externalauth_test.go | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index 0d2a5eee34c2d..6eb5bc000003d 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -774,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) diff --git a/coderd/externalauth/externalauth_test.go b/coderd/externalauth/externalauth_test.go index 4bc4d59836fc2..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} }, }, }) From c6b3b24ff3b9082139caf356e382fab72187a2fa Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 11 Dec 2025 11:31:48 -0600 Subject: [PATCH 11/18] fix tests --- codersdk/deployment_test.go | 43 ++++++++++++++++---------------- codersdk/testdata/githubcfg.yaml | 2 ++ 2 files changed, 24 insertions(+), 21 deletions(-) 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/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 From 37aa4c0937146a2ed8e739a29d7f52c187d203b2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 11:31:40 -0600 Subject: [PATCH 12/18] PR feedback --- coderd/coderd.go | 3 +-- coderd/coderdtest/oidctest/idp.go | 10 +++++++--- coderd/externalauth/externalauth.go | 12 ++++++------ coderd/userauth.go | 4 ++++ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 29a678a6ba669..8552effd01c64 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -24,7 +24,6 @@ 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" @@ -1291,7 +1290,7 @@ func New(options *Options) *API { r.Route("/github", func(r chi.Router) { r.Use( // Github supports PKCE S256 - httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil, []promoauth.Oauth2PKCEChallengeMethod{promoauth.PKCEChallengeMethodSha256}), + httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil, options.GithubOAuth2Config.PKCESupported()), ) r.Get("/callback", api.userOAuth2Github) }) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index 6eb5bc000003d..fb8125e097ce5 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -169,7 +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 + pkce bool // TODO(Emyrk): Implement for refresh token flow as well // externalProviderID is optional to match the provider in coderd for // redirectURLs. externalProviderID string @@ -1054,13 +1054,17 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { if f.pkce { challenge, ok := f.codeToChallengeMap.Load(code) if !ok { - httpError(rw, http.StatusBadRequest, xerrors.New("code challenge not found for code")) + httpError(rw, http.StatusBadRequest, xerrors.New("pkce: challenge not found for code")) return } codeVerifier := values.Get("code_verifier") + if codeVerifier == "" { + httpError(rw, http.StatusBadRequest, xerrors.New("pkce: missing code_verifier")) + return + } expecter := oauth2.S256ChallengeFromVerifier(codeVerifier) if challenge != expecter { - httpError(rw, http.StatusBadRequest, xerrors.New("invalid code verifier")) + httpError(rw, http.StatusBadRequest, xerrors.New("pkce: invalid code verifier")) return } } diff --git a/coderd/externalauth/externalauth.go b/coderd/externalauth/externalauth.go index d730d8cc7d7dd..2957efd69849e 100644 --- a/coderd/externalauth/externalauth.go +++ b/coderd/externalauth/externalauth.go @@ -895,7 +895,7 @@ 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 + // TODO: Investigate if 'S256' is accepted and PKCE is supported CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, } // Bitbucket servers will have some base url, e.g. https://bitbucket.coder.com. @@ -975,7 +975,7 @@ 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 + // TODO: Investigate if 'S256' is accepted and PKCE is supported CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, } // Artifactory servers will have some base url, e.g. https://jfrog.coder.com. @@ -1047,7 +1047,7 @@ 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 + // TODO: Investigate if 'S256' is accepted and PKCE is supported CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, } // The tenant ID is required for urls and is in the auth url. @@ -1087,7 +1087,7 @@ 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 + // TODO: Investigate if 'S256' is accepted and PKCE is supported CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, codersdk.EnhancedExternalAuthProviderBitBucketCloud: { @@ -1098,7 +1098,7 @@ 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 + // TODO: Investigate if 'S256' is accepted and PKCE is supported CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, codersdk.EnhancedExternalAuthProviderSlack: { @@ -1109,7 +1109,7 @@ 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 + // TODO: Investigate if 'S256' is accepted and PKCE is supported CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)}, }, } diff --git a/coderd/userauth.go b/coderd/userauth.go index 388c067257e98..adf150461e840 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -770,6 +770,10 @@ type GithubOAuth2Config struct { DefaultProviderConfigured bool } +func (*GithubOAuth2Config) PKCESupported() []promoauth.Oauth2PKCEChallengeMethod { + return []promoauth.Oauth2PKCEChallengeMethod{promoauth.PKCEChallengeMethodSha256} +} + func (c *GithubOAuth2Config) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { if !c.DeviceFlowEnabled { return c.OAuth2Config.Exchange(ctx, code, opts...) From 263b92504deaeb792f1c53c8b646c1baaed2961a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 11:34:11 -0600 Subject: [PATCH 13/18] rename cookie from challenge to verifier --- coderd/httpmw/oauth2.go | 6 +++--- coderd/userauth_test.go | 4 ++-- codersdk/client.go | 6 ++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index 9cd8c38d0eedd..0e2b99e9073d5 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -146,7 +146,7 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg authOpts = append(authOpts, oauth2.S256ChallengeOption(verifier)) http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{ - Name: codersdk.OAuth2PKCEChallenge, + Name: codersdk.OAuth2PKCEVerifier, Value: verifier, Path: "/", HttpOnly: true, @@ -185,14 +185,14 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg exchangeOpts := make([]oauth2.AuthCodeOption, 0) if sha256PKCESupported { - pkceChallenge, err := r.Cookie(codersdk.OAuth2PKCEChallenge) + pkceVerifier, err := r.Cookie(codersdk.OAuth2PKCEVerifier) 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)) + exchangeOpts = append(exchangeOpts, oauth2.VerifierOption(pkceVerifier.Value)) } oauthToken, err := config.Exchange(ctx, code, exchangeOpts...) diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index ab57e946b4440..53db08aa11b07 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -1018,7 +1018,7 @@ func TestUserOAuth2Github(t *testing.T) { Value: "somestate", }) req.AddCookie(&http.Cookie{ - Name: codersdk.OAuth2PKCEChallenge, + Name: codersdk.OAuth2PKCEVerifier, Value: oauth2.GenerateVerifier(), }) require.NoError(t, err) @@ -2465,7 +2465,7 @@ func oauth2Callback(t *testing.T, client *codersdk.Client, opts ...func(*http.Re Value: state, }) req.AddCookie(&http.Cookie{ - Name: codersdk.OAuth2PKCEChallenge, + Name: codersdk.OAuth2PKCEVerifier, Value: oauth2.GenerateVerifier(), }) res, err := client.HTTPClient.Do(req) diff --git a/codersdk/client.go b/codersdk/client.go index b93aeb80a0bdc..e1591f90fa89d 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -38,8 +38,10 @@ const ( SessionTokenHeader = "Coder-Session-Token" // OAuth2StateCookie is the name of the cookie that stores the oauth2 state. OAuth2StateCookie = "oauth_state" - - OAuth2PKCEChallenge = "oauth_pkce_challenge" + // OAuth2PKCEVerifier is the name of the cookie that stores the oauth2 PKCE + // verifier. This is the raw verifier that when hashed, will match the challenge + // sent in the initial oauth2 request. + OAuth2PKCEVerifier = "oauth_pkce_verifier" // OAuth2RedirectCookie is the name of the cookie that stores the oauth2 redirect. OAuth2RedirectCookie = "oauth_redirect" From 3e780535c5f6bcd04f0cc23b33c7f141a4996080 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 11:36:20 -0600 Subject: [PATCH 14/18] Add some comments --- coderd/coderdtest/oidctest/idp.go | 1 + codersdk/deployment.go | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index fb8125e097ce5..8d9276d18ab46 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -1071,6 +1071,7 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { // Always invalidate the code after it is used. f.codeToStateMap.Delete(code) + f.codeToChallengeMap.Delete(code) idTokenClaims, ok := f.getClaims(f.stateToIDTokenClaims, stateStr) if !ok { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 2c7ebb21c7b5a..7e823a30526b3 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -766,7 +766,9 @@ 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 lists the PKCE code challenge methods + // The only one supported by Coder is "S256" CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported" yaml:"code_challenge_methods_supported"` } From ef3a3d3a03d530de4dfa52a8dc80cb29315af0bc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 11:38:54 -0600 Subject: [PATCH 15/18] make gen --- coderd/apidoc/docs.go | 1 + coderd/apidoc/swagger.json | 1 + docs/reference/api/schemas.md | 32 ++++++++++++++++---------------- site/src/api/typesGenerated.ts | 11 ++++++++++- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 8b69721d35206..764d63b09a082 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -14632,6 +14632,7 @@ const docTemplate = `{ "type": "string" }, "code_challenge_methods_supported": { + "description": "CodeChallengeMethodsSupported lists the PKCE code challenge methods\nThe only one supported by Coder is \"S256\"", "type": "array", "items": { "type": "string" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 0c927eee90bd9..6cce3c98a41d8 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -13209,6 +13209,7 @@ "type": "string" }, "code_challenge_methods_supported": { + "description": "CodeChallengeMethodsSupported lists the PKCE code challenge methods\nThe only one supported by Coder is \"S256\"", "type": "array", "items": { "type": "string" diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 76e9318a99f7f..24152eca466a4 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -4233,22 +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 | | | -| `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 | | | +| 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 | | Code challenge methods supported lists the PKCE code challenge methods The only one supported by Coder is "S256" | +| `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||| diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 53a8aa658d113..2e19baa407a32 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2001,6 +2001,10 @@ export interface ExternalAuthConfig { * DisplayIcon is a URL to an icon to display in the UI. */ readonly display_icon: string; + /** + * CodeChallengeMethodsSupported lists the PKCE code challenge methods + * The only one supported by Coder is "S256" + */ readonly code_challenge_methods_supported: readonly string[]; } @@ -3046,7 +3050,12 @@ export interface OAuth2GithubConfig { } // From codersdk/client.go -export const OAuth2PKCEChallenge = "oauth_pkce_challenge"; +/** + * OAuth2PKCEVerifier is the name of the cookie that stores the oauth2 PKCE + * verifier. This is the raw verifier that when hashed, will match the challenge + * sent in the initial oauth2 request. + */ +export const OAuth2PKCEVerifier = "oauth_pkce_verifier"; // From codersdk/oauth2.go /** From 82fdc0f28cb8c13dc2d357a3dae5658ddd0e898b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 11:42:52 -0600 Subject: [PATCH 16/18] docs: added docs to external auth page --- docs/admin/external-auth/index.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/admin/external-auth/index.md b/docs/admin/external-auth/index.md index 634a22b23c9bb..2bfa1df7ffc50 100644 --- a/docs/admin/external-auth/index.md +++ b/docs/admin/external-auth/index.md @@ -133,6 +133,34 @@ You must add the SSH key to your Git provider. - [GitHub](https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account#adding-a-new-ssh-key-to-your-account) - [GitLab](https://docs.gitlab.com/user/ssh/#add-an-ssh-key-to-your-gitlab-account) +## + +## PKCE Support + +[PKCE (Proof Key for Code Exchange)](https://datatracker.ietf.org/doc/html/rfc7636) is an OAuth 2.0 +security extension that prevents authorization code interception attacks. Coder supports PKCE when +acting as an OAuth client to external identity providers. + +### OIDC Providers + +For OIDC providers, PKCE support is automatically detected from the provider's +`/.well-known/openid-configuration` endpoint. If the provider advertises support for PKCE, +Coder will use it automatically. No manual configuration is required. + +### OAuth Providers (External Auth) + +For OAuth providers configured via external authentication, Coder will usually assume PKCE support is available with "S256" as the code challenge method. + +Manual configuration is available to override any default behavior. + +```env +# Enable PKCE with S256 (recommended when supported) +CODER_EXTERNAL_AUTH_0_PKCE_METHODS="S256" + +# Disable PKCE entirely +CODER_EXTERNAL_AUTH_0_PKCE_METHODS="none" +``` + ## Git-provider specific env variables ### Azure DevOps From a753ff434693f9b5232bdd6328c12c6584ba2026 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 11:43:54 -0600 Subject: [PATCH 17/18] remove oidc from docs, since it is auto --- docs/admin/external-auth/index.md | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/docs/admin/external-auth/index.md b/docs/admin/external-auth/index.md index 2bfa1df7ffc50..8b50692722b6c 100644 --- a/docs/admin/external-auth/index.md +++ b/docs/admin/external-auth/index.md @@ -141,17 +141,8 @@ You must add the SSH key to your Git provider. security extension that prevents authorization code interception attacks. Coder supports PKCE when acting as an OAuth client to external identity providers. -### OIDC Providers - -For OIDC providers, PKCE support is automatically detected from the provider's -`/.well-known/openid-configuration` endpoint. If the provider advertises support for PKCE, -Coder will use it automatically. No manual configuration is required. - -### OAuth Providers (External Auth) - -For OAuth providers configured via external authentication, Coder will usually assume PKCE support is available with "S256" as the code challenge method. - -Manual configuration is available to override any default behavior. +Coder will usually assume PKCE support is available with "S256" as the code challenge method. Manual +configuration is available to override any default behavior. ```env # Enable PKCE with S256 (recommended when supported) From 675b528c87072c07ec617bdbc0a492d4a53ca29b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 12 Dec 2025 12:28:04 -0600 Subject: [PATCH 18/18] PR feedback --- codersdk/deployment.go | 5 ++-- docs/admin/external-auth/index.md | 2 -- docs/reference/api/schemas.md | 44 +++++++++++++++---------------- site/src/api/typesGenerated.ts | 2 +- 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 7e823a30526b3..99442f21da4a1 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -768,7 +768,7 @@ type ExternalAuthConfig struct { // DisplayIcon is a URL to an icon to display in the UI. DisplayIcon string `json:"display_icon" yaml:"display_icon"` // CodeChallengeMethodsSupported lists the PKCE code challenge methods - // The only one supported by Coder is "S256" + // The only one supported by Coder is "S256". CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported" yaml:"code_challenge_methods_supported"` } @@ -1674,8 +1674,7 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Env: "CODER_BLOCK_DIRECT", Value: &c.DERP.Config.BlockDirect, Group: &deploymentGroupNetworkingDERP, - YAML: "blockDirect", Annotations: serpent.Annotations{}. - Mark(annotationExternalProxies, "true"), + YAML: "blockDirect", Annotations: serpent.Annotations{}.Mark(annotationExternalProxies, "true"), }, { Name: "DERP Force WebSockets", diff --git a/docs/admin/external-auth/index.md b/docs/admin/external-auth/index.md index 8b50692722b6c..61d1b5816b18c 100644 --- a/docs/admin/external-auth/index.md +++ b/docs/admin/external-auth/index.md @@ -133,8 +133,6 @@ You must add the SSH key to your Git provider. - [GitHub](https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account#adding-a-new-ssh-key-to-your-account) - [GitLab](https://docs.gitlab.com/user/ssh/#add-an-ssh-key-to-your-gitlab-account) -## - ## PKCE Support [PKCE (Proof Key for Code Exchange)](https://datatracker.ietf.org/doc/html/rfc7636) is an OAuth 2.0 diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 24152eca466a4..d2a4fa3728822 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -4233,29 +4233,29 @@ 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 | | | -| `code_challenge_methods_supported` | array of string | false | | Code challenge methods supported lists the PKCE code challenge methods The only one supported by Coder is "S256" | -| `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. +| 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 | | Code challenge methods supported lists the PKCE code challenge methods The only one supported by Coder is "S256". | +| `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||| -|`scopes`|array of string|false||| -|`token_url`|string|false||| -|`type`|string|false||Type is the type of external auth config.| -|`validate_url`|string|false||| +|`revoke_url`|string|false|| | +|`scopes`|array of string|false|| | +|`token_url`|string|false|| | +|`type`|string|false|| Type is the type of external auth config. | +|`validate_url`|string|false|| | ## codersdk.ExternalAuthDevice diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 2e19baa407a32..fd3e4ca7ddb45 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2003,7 +2003,7 @@ export interface ExternalAuthConfig { readonly display_icon: string; /** * CodeChallengeMethodsSupported lists the PKCE code challenge methods - * The only one supported by Coder is "S256" + * The only one supported by Coder is "S256". */ readonly code_challenge_methods_supported: readonly string[]; }