Set AuthStyle to InParams for public PKCE OAuth clients

pkg/auth/oauth/flow.go

@@ -117,15 +117,26 @@ func NewFlow(config *Config) (*Flow, error) {
 		redirectURL = fmt.Sprintf("http://localhost:%d/callback", port)
 	}
 
+	// Public clients (no secret) must use AuthStyleInParams: strict OAuth 2.1 servers
+	// (e.g. Datadog) reject Basic Auth for token_endpoint_auth_method=none clients and
+	// consume the single-use auth code in doing so, causing a retry to fail with
+	// invalid_grant. Confidential clients use AutoDetect so servers that mandate
+	// client_secret_basic are not broken.
+	authStyle := oauth2.AuthStyleInParams
+	if config.ClientSecret != "" {
+		authStyle = oauth2.AuthStyleAutoDetect
+	}
+
 	// Create OAuth2 config
 	oauth2Config := &oauth2.Config{
 		ClientID:     config.ClientID,
 		ClientSecret: config.ClientSecret,
 		RedirectURL:  redirectURL,
 		Scopes:       config.Scopes,
 		Endpoint: oauth2.Endpoint{
-			AuthURL:  config.AuthURL,
-			TokenURL: config.TokenURL,
+			AuthURL:   config.AuthURL,
+			TokenURL:  config.TokenURL,
+			AuthStyle: authStyle,
 		},
 	}
 

pkg/auth/oauth/flow_test.go

@@ -13,6 +13,7 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"os"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -1204,3 +1205,117 @@ func TestProcessToken_ResourceTokenSourceSelection(t *testing.T) {
 		assert.Equal(t, "access-token", gotToken.AccessToken)
 	})
 }
+
+// TestAuthStyleInParams_StrictPublicClientServer verifies that the OAuth flow
+// uses AuthStyleInParams (sending client_id in the POST body) rather than
+// AuthStyleAutoDetect. This is critical for public PKCE clients because:
+//
+//  1. AutoDetect tries HTTP Basic Auth first (Authorization: Basic base64(client_id:))
+//  2. Strict OAuth 2.1 servers (e.g., Datadog) reject Basic Auth for public clients
+//     registered with token_endpoint_auth_method=none
+//  3. The authorization code is single-use — consumed by the rejected first attempt
+//  4. The retry with client_id in POST body gets "invalid_grant" because the code
+//     was already burned
+func TestAuthStyleInParams_StrictPublicClientServer(t *testing.T) {
+	t.Parallel()
+
+	// Simulate a strict OAuth 2.1 server that rejects Basic Auth for public clients.
+	// This mirrors Datadog's behavior: public clients (token_endpoint_auth_method=none)
+	// MUST send client_id in the POST body, not via Authorization header.
+	var requestCount atomic.Int32
+	tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requestCount.Add(1)
+
+		err := r.ParseForm()
+		require.NoError(t, err)
+
+		// Reject requests that use Basic Auth header — strict public client enforcement
+		if _, _, ok := r.BasicAuth(); ok {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusUnauthorized)
+			json.NewEncoder(w).Encode(map[string]string{
+				"error":             "invalid_client",
+				"error_description": "Basic auth not allowed for public clients",
+			})
+			return
+		}
+
+		// Require client_id in POST body (AuthStyleInParams)
+		clientID := r.Form.Get("client_id")
+		if clientID == "" {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusBadRequest)
+			json.NewEncoder(w).Encode(map[string]string{
+				"error":             "invalid_request",
+				"error_description": "client_id required in request body",
+			})
+			return
+		}
+
+		assert.Equal(t, "test-public-client", clientID)
+		assert.Equal(t, "authorization_code", r.Form.Get("grant_type"))
+		assert.Equal(t, "test-auth-code", r.Form.Get("code"))
+
+		// Verify PKCE verifier is present
+		assert.NotEmpty(t, r.Form.Get("code_verifier"), "PKCE code_verifier should be present")
+
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]interface{}{
+			"access_token":  "dd-access-token",
+			"token_type":    "Bearer",
+			"expires_in":    3600,
+			"refresh_token": "dd-refresh-token",
+		})
+	}))
+	defer tokenServer.Close()
+
+	config := &Config{
+		ClientID:     "test-public-client",
+		ClientSecret: "", // Public client — no secret
+		AuthURL:      "https://example.com/auth",
+		TokenURL:     tokenServer.URL,
+		UsePKCE:      true,
+	}
+
+	flow, err := NewFlow(config)
+	require.NoError(t, err)
+
+	// Verify the endpoint is configured with AuthStyleInParams
+	assert.Equal(t, oauth2.AuthStyleInParams, flow.oauth2Config.Endpoint.AuthStyle,
+		"Endpoint must use AuthStyleInParams to avoid burning auth codes with Basic Auth probes")
+
+	tokenChan := make(chan *oauth2.Token, 1)
+	errorChan := make(chan error, 1)
+
+	handler := flow.handleCallback(tokenChan, errorChan)
+
+	// Simulate the OAuth callback with a valid auth code
+	values := url.Values{}
+	values.Set("code", "test-auth-code")
+	values.Set("state", flow.state)
+
+	req := httptest.NewRequest("GET", "/callback?"+values.Encode(), nil)
+	w := httptest.NewRecorder()
+
+	handler.ServeHTTP(w, req)
+
+	// The exchange should succeed on the first attempt
+	select {
+	case token := <-tokenChan:
+		require.NotNil(t, token)
+		assert.Equal(t, "dd-access-token", token.AccessToken)
+		assert.Equal(t, "dd-refresh-token", token.RefreshToken)
+	case err := <-errorChan:
+		t.Fatalf("token exchange failed: %v\n"+
+			"This likely means AuthStyleInParams is not set — the oauth2 library "+
+			"tried Basic Auth first, the strict server rejected it, and the auth "+
+			"code was consumed by the failed attempt", err)
+	case <-time.After(5 * time.Second):
+		t.Fatal("timeout waiting for token exchange")
+	}
+
+	// The server should have received exactly one request (no retry needed)
+	assert.Equal(t, int32(1), requestCount.Load(),
+		"strict server should receive exactly one request when AuthStyleInParams is used; "+
+			"multiple requests indicate AuthStyleAutoDetect is trying Basic Auth first")
+}

pkg/auth/oauth/resource_token_source.go

@@ -64,7 +64,7 @@ func (r *resourceTokenSource) refreshWithResource(ctx context.Context) (*oauth2.
 
 	// Use x/oauth2 internals via Exchange() so we get:
 	// - expires_in -> Expiry handling
-	// - defaulting to HTTP Basic auth (or auto-detection)
+	// - AuthStyle from the Config's Endpoint (AuthStyleInParams)
 	// - structured error parsing via oauth2.RetrieveError
 	//
 	// Note: Exchange() will include an empty "code" parameter in the body.

pkg/auth/remote/handler.go

@@ -250,14 +250,25 @@ func (h *Handler) tryRestoreFromCachedTokens(
 	// Resolve client credentials - prefer cached DCR credentials over config
 	clientID, clientSecret := h.resolveClientCredentials(ctx)
 
+	// Public clients (no secret) must use AuthStyleInParams: strict OAuth 2.1 servers
+	// (e.g. Datadog) reject Basic Auth for token_endpoint_auth_method=none clients and
+	// consume the single-use auth code in doing so. Confidential clients (DCR or
+	// statically configured) use AutoDetect so servers that mandate client_secret_basic
+	// are not broken.
+	authStyle := oauth2.AuthStyleInParams
+	if clientSecret != "" {
+		authStyle = oauth2.AuthStyleAutoDetect
+	}
+
 	// Build OAuth2 config for token refresh
 	oauth2Config := &oauth2.Config{
 		ClientID:     clientID,
 		ClientSecret: clientSecret,
 		Scopes:       scopes,
 		Endpoint: oauth2.Endpoint{
-			AuthURL:  h.config.AuthorizeURL,
-			TokenURL: h.config.TokenURL,
+			AuthURL:   h.config.AuthorizeURL,
+			TokenURL:  h.config.TokenURL,
+			AuthStyle: authStyle,
 		},
 	}
 

pkg/registry/auth/oauth_token_source.go

@@ -188,8 +188,9 @@ func (o *oauthTokenSource) buildOAuth2Config(ctx context.Context) (*oauth2.Confi
 		ClientSecret: oauthCfg.ClientSecret,
 		Scopes:       oauthCfg.Scopes,
 		Endpoint: oauth2.Endpoint{
-			AuthURL:  oauthCfg.AuthURL,
-			TokenURL: oauthCfg.TokenURL,
+			AuthURL:   oauthCfg.AuthURL,
+			TokenURL:  oauthCfg.TokenURL,
+			AuthStyle: oauth2.AuthStyleInParams,
 		},
 	}, nil
 }