coder · BobbyHo · Sep 18, 2026 · Sep 18, 2026 · Sep 18, 2026 · Sep 18, 2026
diff --git a/coderd/oauth2_test.go b/coderd/oauth2_test.go
@@ -1342,6 +1342,15 @@ func TestOAuth2ProviderRevokeClientAuthentication(t *testing.T) {
 		require.True(t, works(), "a refused revocation must not end the session")
 	}
 
+	requireInvalidRequest := func(t *testing.T, status int, oauthErr codersdk.OAuth2Error, wantDescription string, works func() bool) {
+		t.Helper()
+
+		require.Equal(t, http.StatusBadRequest, status)
+		require.Equal(t, codersdk.OAuth2ErrorCodeInvalidRequest, oauthErr.Error)
+		require.Contains(t, oauthErr.ErrorDescription, wantDescription)
+		require.True(t, works(), "a refused revocation must not end the session")
+	}
+
 	t.Run("MissingSecret", func(t *testing.T) {
 		t.Parallel()
 		ctx := testutil.Context(t, testutil.WaitLong)
@@ -1438,10 +1447,78 @@ func TestOAuth2ProviderRevokeClientAuthentication(t *testing.T) {
 		status, _, oauthErr := postRevoke(ctx, t, userClient, form, func(r *http.Request) {
 			r.SetBasicAuth(apps.Default.ID.String(), secret.ClientSecretFull)
 		})
-		require.Equal(t, http.StatusBadRequest, status)
-		require.Equal(t, codersdk.OAuth2ErrorCodeInvalidRequest, oauthErr.Error)
-		require.Contains(t, oauthErr.ErrorDescription, "Conflicting client credentials")
-		require.True(t, works(), "a refused revocation must not end the session")
+		requireInvalidRequest(t, status, oauthErr, "Conflicting client credentials", works)
+	})
+
+	t.Run("SecretInQueryString", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+		userClient, refreshToken, works := newSession(ctx, t)
+
+		form := url.Values{}
+		form.Set("token", refreshToken)
+		form.Set("client_id", apps.Default.ID.String())
+		status, _, oauthErr := postRevoke(ctx, t, userClient, form, func(r *http.Request) {
+			q := r.URL.Query()
+			q.Set("client_secret", secret.ClientSecretFull)
+			r.URL.RawQuery = q.Encode()
+		})
+		requireInvalidRequest(t, status, oauthErr, "client_secret", works)
+	})
+
+	// A correct secret in the body does not excuse a copy in the URL.
+	t.Run("SecretInQueryStringAndBody", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+		userClient, refreshToken, works := newSession(ctx, t)
+
+		form := url.Values{}
+		form.Set("token", refreshToken)
+		form.Set("client_id", apps.Default.ID.String())
+		form.Set("client_secret", secret.ClientSecretFull)
+		status, _, oauthErr := postRevoke(ctx, t, userClient, form, func(r *http.Request) {
+			q := r.URL.Query()
+			q.Set("client_secret", secret.ClientSecretFull)
+			r.URL.RawQuery = q.Encode()
+		})
+		requireInvalidRequest(t, status, oauthErr, "client_secret", works)
+	})
+
+	// RFC 6749 §3.2: a valueless parameter is the omitted case, so ?client_secret=
+	// leaks nothing and must not cost a client that authenticated in the body.
+	t.Run("EmptySecretInQueryString", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+		userClient, refreshToken, works := newSession(ctx, t)
+
+		form := url.Values{}
+		form.Set("token", refreshToken)
+		form.Set("client_id", apps.Default.ID.String())
+		form.Set("client_secret", secret.ClientSecretFull)
+		status, _, _ := postRevoke(ctx, t, userClient, form, func(r *http.Request) {
+			q := r.URL.Query()
+			q.Set("client_secret", "")
+			r.URL.RawQuery = q.Encode()
+		})
+		require.Equal(t, http.StatusOK, status)
+		require.False(t, works(), "the revocation must end the session")
+	})
+
+	// An empty first value must not hide a real one behind it.
+	t.Run("EmptyAndRealSecretInQueryString", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+		userClient, refreshToken, works := newSession(ctx, t)
+
+		form := url.Values{}
+		form.Set("token", refreshToken)
+		form.Set("client_id", apps.Default.ID.String())
+		status, _, oauthErr := postRevoke(ctx, t, userClient, form, func(r *http.Request) {
+			q := r.URL.Query()
+			q["client_secret"] = []string{"", secret.ClientSecretFull}
+			r.URL.RawQuery = q.Encode()
+		})
+		requireInvalidRequest(t, status, oauthErr, "URL query string", works)
 	})
 }
 

diff --git a/coderd/oauth2provider/revoke.go b/coderd/oauth2provider/revoke.go
@@ -78,9 +78,17 @@ func RevokeToken(db database.Store, logger slog.Logger) http.HandlerFunc {
 			return
 		}
 
+		if clientSecretInQuery(r) {
+			logger.Warn(ctx, "oauth2 revocation refused: client_secret in query string",
+				slog.F("client_id", app.ID.String()),
+				slog.F("app_name", app.Name))
+			httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, errMsgClientSecretInQuery)
+			return
+		}
+
 		req, err := extractRevocationRequest(r)
 		if errors.Is(err, errConflictingClientAuth) {
-			httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, "Conflicting client credentials between Authorization header and request body")
+			httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, errMsgConflictingClientAuth)
 			return
 		}
 		if err != nil {

diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go
@@ -26,6 +26,12 @@ import (
 	"github.com/coder/coder/v2/codersdk"
 )
 
+// Shared by the token and revocation endpoints.
+const (
+	errMsgClientSecretInQuery   = "client_secret was sent in the URL query string; send it in the request body or the Authorization header, and rotate the secret that was exposed" //nolint:gosec // G101: message text, not a hardcoded credential.
+	errMsgConflictingClientAuth = "Conflicting client credentials between Authorization header and request body"
+)
+
 var (
 	// errBadSecret means the user provided a bad secret.
 	errBadSecret = xerrors.New("Invalid client secret")
@@ -268,6 +274,21 @@ func mergeBasicClientAuth(r *http.Request, clientID, clientSecret string) (merge
 	return user, pass, nil
 }
 
+// clientSecretInQuery reports whether the request carries client_secret in the
+// query string. OAuth 2.1 §2.4.1 prohibits it there. The other parameters read
+// from the merged form carry no such prohibition and stay accepted; PLAT-660
+// tracks them.
+//
+// It reads the URL query rather than r.Form, which cannot tell a body value
+// from a query value. Nothing constrains where a caller places the check.
+//
+// RFC 6749 §3.2: a parameter sent without a value counts as omitted.
+func clientSecretInQuery(r *http.Request) bool {
+	return slices.ContainsFunc(r.URL.Query()["client_secret"], func(v string) bool {
+		return v != ""
+	})
+}
+
 // authenticateClient checks a client secret and confirms it belongs to the
 // app named by client_id. That id arrives unverified, so without the app
 // check a valid secret for one app could issue a token for another. It
@@ -314,6 +335,13 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime, logger slog.L
 		ctx := r.Context()
 		app := httpmw.OAuth2ProviderApp(r)
 
+		if clientSecretInQuery(r) {
+			logger.Warn(ctx, "oauth2 token request refused: client_secret in query string",
+				slog.F("app_id", app.ID))
+			writeTokenError(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, errMsgClientSecretInQuery)
+			return
+		}
+
 		primary, alternates, err := registeredRedirectURIs(app)
 		if err != nil {
 			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -334,7 +362,7 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime, logger slog.L
 				return
 			}
 			if errors.Is(err, errConflictingClientAuth) {
-				writeTokenError(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, "Conflicting client credentials between Authorization header and request body")
+				writeTokenError(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, errMsgConflictingClientAuth)
 				return
 			}
 

diff --git a/coderd/oauth2provider/tokens_test.go b/coderd/oauth2provider/tokens_test.go
@@ -689,17 +689,26 @@ func TestOAuth2RefreshClientAuthentication(t *testing.T) {
 		return status, header, body
 	}
 
-	// The token must survive the refusal and redeem on the next, correct
-	// attempt: nothing was consumed.
-	requireRefused := func(ctx context.Context, t *testing.T, app appWithSecret, refreshToken string, status int, body string) {
+	// A refusal must leave the grant untouched: the row is still there and the
+	// single-use refresh token still redeems on the next, correct attempt.
+	// Neither is visible from the error response the call site sees.
+	requireNothingConsumed := func(ctx context.Context, t *testing.T, app appWithSecret, refreshToken string) {
 		t.Helper()
 
-		requireTokenClientError(t, status, body)
 		_ = tokenRow(ctx, t, db, refreshToken)
-		status, body = postTokenRequest(ctx, t, client, refreshForm(app, refreshToken))
+		status, body := postTokenRequest(ctx, t, client, refreshForm(app, refreshToken))
 		requireTokenResponse(t, status, body)
 	}
 
+	// A client authentication failure is a 401 invalid_client that costs the
+	// grant nothing.
+	requireRefused := func(ctx context.Context, t *testing.T, app appWithSecret, refreshToken string, status int, body string) {
+		t.Helper()
+
+		requireTokenClientError(t, status, body)
+		requireNothingConsumed(ctx, t, app, refreshToken)
+	}
+
 	t.Run("MissingSecret", func(t *testing.T) {
 		t.Parallel()
 		ctx := testutil.Context(t, testutil.WaitLong)
@@ -793,6 +802,48 @@ func TestOAuth2RefreshClientAuthentication(t *testing.T) {
 		_ = tokenRow(ctx, t, db, refreshToken)
 	})
 
+	t.Run("SecretInQueryString", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		app := seedAppWithSecret(t, db, sql.NullString{})
+		refreshToken := seedRefreshToken(ctx, t, db, app, owner.UserID, "workspace:ssh")
+
+		form := refreshForm(app, refreshToken)
+		form.Del("client_secret")
+		status, _, body := postForm(ctx, t, form, func(r *http.Request) {
+			q := r.URL.Query()
+			q.Set("client_secret", app.ClientSecret)
+			r.URL.RawQuery = q.Encode()
+		})
+		desc := requireTokenError(t, status, body, codersdk.OAuth2ErrorCodeInvalidRequest)
+		require.Contains(t, desc, "URL query string")
+		requireNothingConsumed(ctx, t, app, refreshToken)
+	})
+
+	// A correct secret in the body does not excuse a copy in the URL. The copy
+	// leaves client_secret in r.Form twice, which extractTokenRequest already
+	// refuses as a repeated parameter with the same status and code, so the
+	// description is what distinguishes this rule from that one.
+	t.Run("SecretInQueryStringAndBody", func(t *testing.T) {
+		t.Parallel()
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		app := seedAppWithSecret(t, db, sql.NullString{})
+		refreshToken := seedRefreshToken(ctx, t, db, app, owner.UserID, "workspace:ssh")
+
+		status, _, body := postForm(ctx, t, refreshForm(app, refreshToken), func(r *http.Request) {
+			q := r.URL.Query()
+			q.Set("client_secret", app.ClientSecret)
+			r.URL.RawQuery = q.Encode()
+		})
+		desc := requireTokenError(t, status, body, codersdk.OAuth2ErrorCodeInvalidRequest)
+		require.Contains(t, desc, "URL query string")
+		// The body was already correct, so dropping the URL copy is the only
+		// change the retry makes: the copy alone caused the refusal.
+		requireNothingConsumed(ctx, t, app, refreshToken)
+	})
+
 	// A public client has no secret to check; the token's app binding and
 	// single-use rotation are what tie its refresh to the client.
 	t.Run("PublicClientNoSecret", func(t *testing.T) {

diff --git a/docs/admin/integrations/oauth2-provider.md b/docs/admin/integrations/oauth2-provider.md
@@ -53,7 +53,7 @@
 Those tokens keep authenticating to the regular Coder API while the OAuth2 refresh and revocation endpoints return 404.
 Treat the setting as a way to stop new authorizations rather than as a way to revoke access, and revoke the tokens or delete the application before you disable the provider.

 ## Creating OAuth2 Applications

 ### Method 1: Web UI

@@ -150,6 +150,8 @@
 
 Coder supports both secret-based methods for compatibility; existing integrations using `client_secret_post` do not need to change.
 
+Send `client_secret` in the request body or in the `Authorization` header. A request that puts `client_secret` in the URL query string is rejected with `invalid_request`, because OAuth 2.1 section 2.4.1 does not allow it there. This applies to both `POST /oauth2/tokens` and `POST /oauth2/revoke`. The rule covers `client_secret` only. Coder still reads `refresh_token`, `code`, and the revocation `token` from the query string, so send those in the request body as well.
+
 Public clients suit native, mobile, and CLI applications that cannot keep a secret confidential. Note the redirect URI restrictions below before choosing one.
 
 Opening a public client on the **OAuth2 Applications** page shows no client secrets section, since a public client has no secret to display or generate.
@@ -426,7 +428,7 @@
 This is also how you remove clients that registered themselves while dynamic client registration was enabled.
 Turning the setting off stops new registrations; it does not remove the ones already there.

 ## Testing and Development

 Coder provides comprehensive test scripts for OAuth2 development:

@@ -591,6 +593,28 @@
 revoked with it, and the client must authorize again. Public clients have no
 secret and never receive this error for omitting one.
 
+### "invalid_request" for `client_secret` in the query string
+
+`POST /oauth2/tokens` and `POST /oauth2/revoke` answer HTTP 400 with
+`error=invalid_request` when `client_secret` appears in the URL query string.
+OAuth 2.1 section 2.4.1 allows the secret in the request body or the
+`Authorization` header only. Send it as a form parameter or as HTTP Basic,
+following [Client Authentication Methods](#client-authentication-methods).
+
+A copy in the body does not excuse one in the URL: the request is refused on
+the query string alone, whatever the body holds. The refusal issues no token
+and revokes nothing, so the retry needs no new authorization.
+
+Rotate the secret that was in the URL. It is still valid, and a URL is
+recorded by reverse proxies, load balancers, CDN access logs, shell history,
+and client libraries. Coder does not log query strings, so an empty result
+when you search the Coder logs does not mean the secret stayed private.
+Deleting a secret also revokes the tokens issued under it, so the client has
+to authorize again.
+
+Earlier releases accepted the parameter in the query string. An integration
+that relied on that has to move it into the body or the header.
+
 ### "unsupported_response_type" returned to your callback
 
 Coder supports the authorization code flow only, so `response_type=code` is the single accepted value.