opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat!: support PKCE in the oauth2 client's auth/exchange flow (#21215)

Browse Source 
**Breaking Change:** Existing oauth apps might now use PKCE. If an
unknown IdP type was being used, and it does not support PKCE, it will
break.

To fix, set the PKCE methods on the external auth to `none`
```
export CODER_EXTERNAL_AUTH_1_PKCE_METHODS=none
```
This commit is contained in:
Steven Masley
2025-12-15 17:41:47 +00:00
committed by GitHub
parent 3194bcfc9e
commit 8fefd91e4a
26 changed files with 473 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/apidoc/docs.go
Generated
+7
View File
@@ -14640,6 +14640,13 @@ const docTemplate = `{
"client_id": {
"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"
}
},
"device_code_url": {
"type": "string"
},
coderd/apidoc/swagger.json
Generated
+7
View File
@@ -13217,6 +13217,13 @@
"client_id": {
"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"
}
},
"device_code_url": {
"type": "string"
},
coderd/coderd.go
+4 -3
View File
@@ -941,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))
})
@@ -1290,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, options.GithubOAuth2Config.PKCESupported()),
)
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)
})
coderd/coderdtest/oidctest/idp.go
+66 -5
View File
@@ -169,6 +169,7 @@ type FakeIDP struct {
// clientID to be used by coderd
clientID string
clientSecret string
pkce bool // TODO(Emyrk): 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,8 +1050,28 @@ 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("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("pkce: invalid code verifier"))
return
}
}
// 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 {
coderd/externalauth.go
+10 -8
View File
@@ -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),
}
}
coderd/externalauth/externalauth.go
+67 -43
View File
@@ -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: 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.
// 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: 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.
// 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: 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.
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: Investigate if 'S256' is accepted and PKCE is supported
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: Investigate if 'S256' is accepted and PKCE is supported
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: Investigate if 'S256' is accepted and PKCE is supported
CodeChallengeMethodsSupported: []string{string(promoauth.PKCEChallengeMethodNone)},
},
}
coderd/externalauth/externalauth_internal_test.go
+96 -45
View File
@@ -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)
})
}
}
coderd/externalauth/externalauth_test.go
+9 -7
View File
@@ -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)
coderd/externalauth_test.go
+19
View File
@@ -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"
coderd/httpmw/oauth2.go
+35 -3
View File
@@ -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.OAuth2PKCEVerifier,
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 {
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(pkceVerifier.Value))
}
oauthToken, err := config.Exchange(ctx, code, exchangeOpts...)
if err != nil {
errorCode := http.StatusInternalServerError
detail := err.Error()
coderd/httpmw/oauth2_test.go
+9 -9
View File
@@ -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() {
coderd/promoauth/oauth2.go
+7
View File
@@ -11,6 +11,13 @@ import (
"golang.org/x/oauth2"
)
type Oauth2PKCEChallengeMethod string
const (
PKCEChallengeMethodSha256 Oauth2PKCEChallengeMethod = "S256"
PKCEChallengeMethodNone Oauth2PKCEChallengeMethod = ""
)
type Oauth2Source string
const (
coderd/userauth.go
+13
View File
@@ -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...)
@@ -1172,6 +1176,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
coderd/userauth_test.go
+8
View File
@@ -1017,6 +1017,10 @@ func TestUserOAuth2Github(t *testing.T) {
Name: "oauth_state",
Value: "somestate",
})
req.AddCookie(&http.Cookie{
Name: codersdk.OAuth2PKCEVerifier,
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.OAuth2PKCEVerifier,
Value: oauth2.GenerateVerifier(),
})
res, err := client.HTTPClient.Do(req)
require.NoError(t, err)
t.Cleanup(func() {