mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
fix(coderd/oauth2provider): support client_secret_basic client auth (#21793)
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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 \
|
||||
|
||||
Reference in New Issue
Block a user