From dd6aec04d71056bd17d55720351311e8b00b4a61 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Mon, 2 Feb 2026 16:01:33 +0100 Subject: [PATCH] fix(coderd/oauth2provider): support client_secret_basic client auth (#21793) --- coderd/httpapi/httpapi.go | 6 + coderd/httpmw/oauth2.go | 7 ++ coderd/oauth2provider/metadata.go | 2 +- .../oauth2providertest/oauth2_test.go | 116 ++++++++++++++++++ coderd/oauth2provider/tokens.go | 39 +++++- docs/admin/integrations/oauth2-provider.md | 54 +++++++- 6 files changed, 216 insertions(+), 8 deletions(-) diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index c2dd81bbc3..2ee18ee0d8 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -503,6 +503,12 @@ func OneWayWebSocketEventSender(log slog.Logger) func(rw http.ResponseWriter, r // WriteOAuth2Error writes an OAuth2-compliant error response per RFC 6749. // This should be used for all OAuth2 endpoints (/oauth2/*) to ensure compliance. func WriteOAuth2Error(ctx context.Context, rw http.ResponseWriter, status int, errorCode codersdk.OAuth2ErrorCode, description string) { + // RFC 6749 §5.2: invalid_client SHOULD use 401 and MUST include a + // WWW-Authenticate response header. + if status == http.StatusUnauthorized && errorCode == codersdk.OAuth2ErrorCodeInvalidClient { + rw.Header().Set("WWW-Authenticate", `Basic realm="coder"`) + } + Write(ctx, rw, status, codersdk.OAuth2Error{ Error: errorCode, ErrorDescription: description, diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index a851299e66..5f12543887 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -329,6 +329,13 @@ func extractOAuth2ProviderAppBase(db database.Store, errWriter errorWriter) func paramAppID = r.Form.Get("client_id") } } + if paramAppID == "" { + // RFC 6749 §2.3.1: confidential clients may authenticate via + // HTTP Basic where the username is the client_id. + if user, _, ok := r.BasicAuth(); ok && user != "" { + paramAppID = user + } + } if paramAppID == "" { errWriter.writeMissingClientID(ctx, rw) return diff --git a/coderd/oauth2provider/metadata.go b/coderd/oauth2provider/metadata.go index 16749fe44c..53481a35d4 100644 --- a/coderd/oauth2provider/metadata.go +++ b/coderd/oauth2provider/metadata.go @@ -23,7 +23,7 @@ func GetAuthorizationServerMetadata(accessURL *url.URL) http.HandlerFunc { GrantTypesSupported: []codersdk.OAuth2ProviderGrantType{codersdk.OAuth2ProviderGrantTypeAuthorizationCode, codersdk.OAuth2ProviderGrantTypeRefreshToken}, CodeChallengeMethodsSupported: []codersdk.OAuth2PKCECodeChallengeMethod{codersdk.OAuth2PKCECodeChallengeMethodS256}, ScopesSupported: rbac.ExternalScopeNames(), - TokenEndpointAuthMethodsSupported: []codersdk.OAuth2TokenEndpointAuthMethod{codersdk.OAuth2TokenEndpointAuthMethodClientSecretPost}, + TokenEndpointAuthMethodsSupported: []codersdk.OAuth2TokenEndpointAuthMethod{codersdk.OAuth2TokenEndpointAuthMethodClientSecretBasic, codersdk.OAuth2TokenEndpointAuthMethodClientSecretPost}, } httpapi.Write(ctx, rw, http.StatusOK, metadata) } diff --git a/coderd/oauth2provider/oauth2providertest/oauth2_test.go b/coderd/oauth2provider/oauth2providertest/oauth2_test.go index 737acd1628..f3a5f514a5 100644 --- a/coderd/oauth2provider/oauth2providertest/oauth2_test.go +++ b/coderd/oauth2provider/oauth2providertest/oauth2_test.go @@ -1,13 +1,20 @@ package oauth2providertest_test import ( + "encoding/json" + "net/http" + "net/url" + "strings" "testing" + "time" "github.com/stretchr/testify/require" + "golang.org/x/oauth2" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" ) func TestOAuth2AuthorizationServerMetadata(t *testing.T) { @@ -42,6 +49,12 @@ func TestOAuth2AuthorizationServerMetadata(t *testing.T) { require.True(t, ok, "code_challenge_methods_supported should be an array") require.Contains(t, challengeMethods, "S256", "should support S256 PKCE method") + // Verify token endpoint auth methods + authMethods, ok := metadata["token_endpoint_auth_methods_supported"].([]any) + require.True(t, ok, "token_endpoint_auth_methods_supported should be an array") + require.Contains(t, authMethods, "client_secret_basic", "should support client_secret_basic token auth") + require.Contains(t, authMethods, "client_secret_post", "should support client_secret_post token auth") + // Verify endpoints are proper URLs authEndpoint, ok := metadata["authorization_endpoint"].(string) require.True(t, ok, "authorization_endpoint should be a string") @@ -186,6 +199,109 @@ func TestOAuth2WithoutPKCE(t *testing.T) { require.NotEmpty(t, token.RefreshToken, "should receive refresh token") } +func TestOAuth2TokenExchangeClientSecretBasic(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: false, + }) + _ = coderdtest.CreateFirstUser(t, client) + + app, clientSecret := oauth2providertest.CreateTestOAuth2App(t, client) + t.Cleanup(func() { + oauth2providertest.CleanupOAuth2App(t, client, app.ID) + }) + + state := oauth2providertest.GenerateState(t) + + authParams := oauth2providertest.AuthorizeParams{ + ClientID: app.ID.String(), + ResponseType: "code", + RedirectURI: oauth2providertest.TestRedirectURI, + State: state, + } + + code := oauth2providertest.AuthorizeOAuth2App(t, client, client.URL.String(), authParams) + require.NotEmpty(t, code, "should receive authorization code") + + ctx := testutil.Context(t, testutil.WaitLong) + data := url.Values{} + data.Set("grant_type", "authorization_code") + data.Set("code", code) + data.Set("redirect_uri", oauth2providertest.TestRedirectURI) + + req, err := http.NewRequestWithContext(ctx, "POST", client.URL.String()+"/oauth2/tokens", strings.NewReader(data.Encode())) + require.NoError(t, err, "failed to create token request") + + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(app.ID.String(), clientSecret) + + httpClient := &http.Client{Timeout: 10 * time.Second} + resp, err := httpClient.Do(req) + require.NoError(t, err, "failed to perform token request") + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, "unexpected status code") + + var tokenResp oauth2.Token + err = json.NewDecoder(resp.Body).Decode(&tokenResp) + require.NoError(t, err, "failed to decode token response") + + require.NotEmpty(t, tokenResp.AccessToken, "missing access token") + require.NotEmpty(t, tokenResp.RefreshToken, "missing refresh token") + require.Equal(t, "Bearer", tokenResp.TokenType, "unexpected token type") +} + +func TestOAuth2TokenExchangeClientSecretBasicInvalidSecret(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: false, + }) + _ = coderdtest.CreateFirstUser(t, client) + + app, clientSecret := oauth2providertest.CreateTestOAuth2App(t, client) + t.Cleanup(func() { + oauth2providertest.CleanupOAuth2App(t, client, app.ID) + }) + + state := oauth2providertest.GenerateState(t) + + authParams := oauth2providertest.AuthorizeParams{ + ClientID: app.ID.String(), + ResponseType: "code", + RedirectURI: oauth2providertest.TestRedirectURI, + State: state, + } + + code := oauth2providertest.AuthorizeOAuth2App(t, client, client.URL.String(), authParams) + require.NotEmpty(t, code, "should receive authorization code") + + ctx := testutil.Context(t, testutil.WaitLong) + data := url.Values{} + data.Set("grant_type", "authorization_code") + data.Set("code", code) + data.Set("redirect_uri", oauth2providertest.TestRedirectURI) + + wrongSecret := clientSecret + "x" + + req, err := http.NewRequestWithContext(ctx, "POST", client.URL.String()+"/oauth2/tokens", strings.NewReader(data.Encode())) + require.NoError(t, err, "failed to create token request") + + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(app.ID.String(), wrongSecret) + + httpClient := &http.Client{Timeout: 10 * time.Second} + resp, err := httpClient.Do(req) + require.NoError(t, err, "failed to perform token request") + defer resp.Body.Close() + + require.Equal(t, http.StatusUnauthorized, resp.StatusCode, "expected 401 status code") + require.Equal(t, `Basic realm="coder"`, resp.Header.Get("WWW-Authenticate"), "missing WWW-Authenticate header") + + oauth2providertest.RequireOAuth2Error(t, resp, oauth2providertest.OAuth2ErrorTypes.InvalidClient) +} + func TestOAuth2PKCEPlainMethodRejected(t *testing.T) { t.Parallel() diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go index 956f91a127..fcd031f8e5 100644 --- a/coderd/oauth2provider/tokens.go +++ b/coderd/oauth2provider/tokens.go @@ -34,6 +34,9 @@ var ( errInvalidPKCE = xerrors.New("invalid code_verifier") // errInvalidResource means the resource parameter validation failed. errInvalidResource = xerrors.New("invalid resource parameter") + // errConflictingClientAuth means the client provided credentials in both the + // request body and HTTP Basic, but they did not match. + errConflictingClientAuth = xerrors.New("conflicting client authentication") ) func extractTokenRequest(r *http.Request, callbackURL *url.URL) (codersdk.OAuth2TokenRequest, []codersdk.ValidationError, error) { @@ -52,7 +55,7 @@ func extractTokenRequest(r *http.Request, callbackURL *url.URL) (codersdk.OAuth2 case codersdk.OAuth2ProviderGrantTypeRefreshToken: p.RequiredNotEmpty("refresh_token") case codersdk.OAuth2ProviderGrantTypeAuthorizationCode: - p.RequiredNotEmpty("client_secret", "client_id", "code") + p.RequiredNotEmpty("code") } req := codersdk.OAuth2TokenRequest{ @@ -67,6 +70,35 @@ func extractTokenRequest(r *http.Request, callbackURL *url.URL) (codersdk.OAuth2 Scope: p.String(vals, "", "scope"), } + // RFC 6749 §2.3.1: confidential clients may authenticate via HTTP Basic. + if user, pass, ok := r.BasicAuth(); ok && user != "" { + if req.ClientID != "" && req.ClientID != user { + return codersdk.OAuth2TokenRequest{}, nil, errConflictingClientAuth + } + if req.ClientSecret != "" && req.ClientSecret != pass { + return codersdk.OAuth2TokenRequest{}, nil, errConflictingClientAuth + } + + req.ClientID = user + req.ClientSecret = pass + } + + // Grant-specific required checks that can be satisfied via HTTP Basic. + if req.GrantType == codersdk.OAuth2ProviderGrantTypeAuthorizationCode { + if req.ClientID == "" { + p.Errors = append(p.Errors, codersdk.ValidationError{ + Field: "client_id", + Detail: "Parameter \"client_id\" is required and cannot be empty", + }) + } + if req.ClientSecret == "" { + p.Errors = append(p.Errors, codersdk.ValidationError{ + Field: "client_secret", + Detail: "Parameter \"client_secret\" is required and cannot be empty", + }) + } + } + // Validate redirect URI - errors are added to p.Errors. _ = p.RedirectURL(vals, callbackURL, "redirect_uri") @@ -104,6 +136,11 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerF req, validationErrs, err := extractTokenRequest(r, callbackURL) if err != nil { + if errors.Is(err, errConflictingClientAuth) { + httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, "Conflicting client credentials between Authorization header and request body") + return + } + // Check for specific validation errors in priority order if slices.ContainsFunc(validationErrs, func(validationError codersdk.ValidationError) bool { return validationError.Field == "grant_type" diff --git a/docs/admin/integrations/oauth2-provider.md b/docs/admin/integrations/oauth2-provider.md index e526490429..e85055ec0c 100644 --- a/docs/admin/integrations/oauth2-provider.md +++ b/docs/admin/integrations/oauth2-provider.md @@ -69,6 +69,19 @@ curl -X POST \ ## Integration Patterns +### Client Authentication Methods + +Coder supports the following OAuth2 client authentication methods at the token endpoint (`/oauth2/tokens`): + +- `client_secret_basic` (recommended): HTTP Basic authentication (RFC 6749 §2.3.1). The username is `client_id` and the password is `client_secret`. +- `client_secret_post`: Form-based authentication where `client_id` and `client_secret` are sent in the request body. + +Coder supports both methods for compatibility; existing integrations using `client_secret_post` do not need to change. + +If you use Dynamic Client Registration (RFC 7591) and omit `token_endpoint_auth_method`, clients default to `client_secret_basic`. To request `client_secret_post`, set `token_endpoint_auth_method` to `client_secret_post` in the registration request. + +If client authentication fails, the token endpoint returns **HTTP 401** with an OAuth2 `invalid_client` error and a `WWW-Authenticate: Basic realm="coder"` response header. + ### Standard OAuth2 Flow 1. **Authorization Request**: Redirect users to Coder's authorization endpoint: @@ -81,7 +94,21 @@ curl -X POST \ state=random-string ``` -2. **Token Exchange**: Exchange the authorization code for an access token: +2. **Token Exchange**: Exchange the authorization code for an access token. + + **Option A: HTTP Basic authentication (`client_secret_basic`, recommended)** + + ```bash + curl -X POST \ + -u "$CLIENT_ID:$CLIENT_SECRET" \ + -H "Content-Type: application/x-www-form-urlencoded" \ + -d "grant_type=authorization_code" \ + -d "code=$AUTH_CODE" \ + -d "redirect_uri=https://yourapp.example.com/callback" \ + "$CODER_URL/oauth2/tokens" + ``` + + **Option B: Form parameters (`client_secret_post`)** ```bash curl -X POST \ @@ -101,9 +128,9 @@ curl -X POST \ "$CODER_URL/api/v2/users/me" ``` -### PKCE Flow (Public Clients) +### PKCE Flow (Recommended) -For mobile apps and single-page applications, use PKCE for enhanced security: +Use PKCE for enhanced security (recommended for both public and confidential clients): 1. Generate a code verifier and challenge: @@ -123,14 +150,16 @@ For mobile apps and single-page applications, use PKCE for enhanced security: redirect_uri=https://yourapp.example.com/callback ``` -3. Include the code verifier in the token exchange: +3. Include the code verifier in the token exchange (see [Client Authentication Methods](#client-authentication-methods)): ```bash curl -X POST \ + -u "$CLIENT_ID:$CLIENT_SECRET" \ + -H "Content-Type: application/x-www-form-urlencoded" \ -d "grant_type=authorization_code" \ -d "code=$AUTH_CODE" \ - -d "client_id=$CLIENT_ID" \ -d "code_verifier=$CODE_VERIFIER" \ + -d "redirect_uri=https://yourapp.example.com/callback" \ "$CODER_URL/oauth2/tokens" ``` @@ -147,7 +176,20 @@ These endpoints return server capabilities and endpoint URLs according to [RFC 8 ### Refresh Tokens -Refresh an expired access token: +Refresh an expired access token. + +**Option A: HTTP Basic authentication (`client_secret_basic`)** + +```bash +curl -X POST \ + -u "$CLIENT_ID:$CLIENT_SECRET" \ + -H "Content-Type: application/x-www-form-urlencoded" \ + -d "grant_type=refresh_token" \ + -d "refresh_token=$REFRESH_TOKEN" \ + "$CODER_URL/oauth2/tokens" +``` + +**Option B: Form parameters (`client_secret_post`)** ```bash curl -X POST \