…ngth

tr -d "=+/" deleted every '+' and '/' character that happened to appear
in the base64 output instead of translating them to the URL-safe
alphabet, so cut -c -43 truncated a string that was often already
short. Roughly 70% of runs produced a verifier below the 43-character
floor coderd/oauth2provider now enforces (#28003), so the manual and
scripted OAuth2 flows these scripts drive failed token exchange
intermittently.

Use tr '+/' '-_' | tr -d '=' instead: translating first and then
stripping the single padding character is deterministic, since 32
random bytes always base64-encode to a fixed length. This always
yields exactly 43 characters, so the cut is no longer needed.

extractAuthorizeParams only checked code_challenge for non-emptiness, so
a malformed value (wrong length, disallowed characters, an arbitrarily
large blob) was persisted verbatim and only surfaced as a failure at
token exchange, with an error that misleadingly names code_verifier
instead of the parameter that was actually invalid.

RFC 7636 gives code_verifier and code_challenge the same ABNF, so reuse
the existing bounds check rather than adding a second one: rename
ValidPKCEVerifier to ValidPKCEFormat and validate code_challenge against
it in extractAuthorizeParams, rejecting a malformed value with
invalid_request at the authorization request per RFC 7636 §4.4.1.

TestExtractAuthorizeParams_Scopes used a 14-character placeholder
code_challenge that the new check now correctly rejects; lengthened it
to a valid value since that test only exercises scope parsing.

…_verifier

A malformed code_verifier (wrong length or disallowed characters) and a
well-formed verifier that simply fails the PKCE hash comparison both
returned the same error: invalid_grant, "The PKCE code verifier is
invalid." A client that sent a too-short verifier had no way to tell
that apart from a genuine hash mismatch, would re-check its SHA-256
computation, find nothing wrong, and retry the same bad verifier
indefinitely since invalid_grant conventionally signals "retry."

RFC 6749 §5.2 assigns a malformed parameter to invalid_request; RFC 7636
§4.6 reserves invalid_grant for the comparison failure specifically. Move
the code_verifier format check out of authorizationCodeGrant and into
extractTokenRequest, which already owns syntax validation for this grant
type, so the two failure modes return distinct, spec-accurate errors.

Several existing tests sent an empty or placeholder code_verifier
incidental to what they were actually testing (client_secret
requirements, scope parsing, malformed-code handling); updated them to
use a valid-length value so they still reach the behavior under test.

…f PKCE hash mismatch

InvalidCodeVerifier ("wrong-verifier", 14 chars) was rejected on length
before VerifyPKCE ever ran, so no test exercised the token endpoint's
hash-comparison branch end to end; TestVerifyPKCE unit-tests the
function, but nothing proved the endpoint still calls it.

Lengthen InvalidCodeVerifier to a well-formed but wrong 43-character
value so it again reaches the hash comparison. Add MalformedCodeVerifier
and a new test asserting the length-rejection path returns
invalid_request, now that the previous commit gives it a distinct error
from the hash-mismatch invalid_grant case.

The code was deleted only inside the success-path transaction, so every
PKCE rejection (errInvalidPKCE) left it live in the database. RFC 6749
§10.5 requires authorization codes to be single-use; without that, an
attacker holding a leaked code (the exact threat PKCE defends against,
since codes and challenges land in browser history, referrer headers, and
proxy logs) could retry the token endpoint with different code_verifier
guesses for the entire 10-minute code lifetime, unthrottled. The
43-character length floor bounds guess format, not entropy.

Add revokeOAuth2CodeOnPKCEFailure, called from both PKCE rejection paths
in authorizationCodeGrant. It deletes the code using the same system
authz context already used for reads in this function; a deletion
failure is noted on the request's log line rather than changing the
response, since surfacing it as a different error would let a caller
distinguish delete success from failure, itself a new oracle.

Added TestOAuth2PKCEFailureConsumesCode to verify the code is
unredeemable, even with the correct verifier, once a PKCE mismatch has
occurred.

Tighten the ValidPKCEFormat doc comment and correct a false claim
(CRF-8, CRF-10). The rationale restated the same threat model across
three separate rhetorical framings, and claimed PKCE is the only
client authentication some clients have, which is false today since
authorizationCodeGrant validates a client secret before PKCE ever
runs; that claim only becomes true once #27873 adds public clients.
Trim the paragraph to a single concrete why and note the caveat.

Delete four boundary-case comments in pkce_test.go (CRF-9). Each one
restated the case name and the strings.Repeat literal beside it; the
RFC provenance already lives on ValidPKCEFormat's doc comment and the
pkceVerifierMinLength/pkceVerifierMaxLength constants, so the comments
carried no information and would drift if either constant changed.

Replace an em-dash with a comma in a comment inside the block this
PR's PKCE-failure handling touches (CRF-2), per the repo's no-emdash
rule. It survived lint because the check scans only changed lines by
default, and this comment was pre-existing context rather than a line
this PR added.

Fix the PKCE example in docs/admin/integrations/oauth2-provider.md
(CRF-7). tr -d "=+/" deleted reserved base64 characters instead of
translating them to the URL-safe alphabet, so the example computed a
code_challenge that failed to verify roughly 74% of the time. Also
strip the newline openssl base64 inserts at its default 64-column
wrap, which the 96-byte verifier example crosses; the prior
cut -c1-128 never merged the wrapped lines back together either.

The PKCE Flow section showed how to generate a code_verifier and
code_challenge but never stated the bound now enforced server-side:
43 to 128 characters from the unreserved set [A-Za-z0-9-._~] (RFC
7636 §4.1). A value outside these bounds returns invalid_request,
at the token endpoint for code_verifier and at the authorization
endpoint for code_challenge.

isValidCustomScheme required a literal "." in the scheme for a public
client's redirect URI, so vscode://, jetbrains://, and cursor:// all
400'd while the identical schemes passed for a confidential client
through the separate, more permissive validateScheme. Native and CLI
apps, the population public clients exist for, register those exact
schemes with their OS.

Removed the extra restriction: validateScheme already blocks the
schemes that are actually dangerous in a redirect context, and RFC
8252 section 7.1 only recommends reverse-domain notation rather than
requiring it. PKCE, not the scheme's spelling, is what secures a
public client's redirect.

That removal also stopped rejecting mailto, tel, and sms for public
clients specifically, since validateScheme's dangerous-scheme
blocklist never covered them either. Those three hand off to a mail
client, dialer, or SMS app rather than returning control to the
client, so unlike vscode:// or jetbrains://, none of them can deliver
an authorization code. A public client's redirect URI scheme is its
only mechanism for regaining control, so they are rejected again here,
scoped specifically to public clients rather than folded into
validateScheme's blocklist, since they are harmless for a confidential
client's redirect.

Split out of #27873 to make that PR smaller to review. First in the
stack; the rest of the public-client work builds on this.

`isValidCustomScheme` required a literal `.` in the scheme for a public
client's redirect URI, so `vscode://`, `jetbrains://`, and `cursor://`
all 400'd while the identical schemes passed for a confidential client
through the separate, more permissive `validateScheme`. Native and CLI
apps, the population public clients exist for, register those exact
schemes with their OS.

Removed the extra restriction: `validateScheme` already blocks the
schemes that are actually dangerous in a redirect context, and RFC 8252
section 7.1 only recommends reverse-domain notation rather than
requiring it. PKCE, not the scheme's spelling, is what secures a public
client's redirect.

That removal also stopped rejecting `mailto`, `tel`, and `sms` for
public clients specifically, since `validateScheme`'s dangerous-scheme
blocklist never covered them either. Those three hand off to a mail
client, dialer, or SMS app rather than returning control to the
application that started the flow, so a public client registered with
one of them could never actually complete authorization. They are
rejected again here, scoped to public clients only because that is how
custom-scheme validation was already scoped before this change, not
because they are known to be safe for a confidential client's redirect;
confidential clients were never subject to any scheme-shape check beyond
`validateScheme` and remain so here.

Refs
https://linear.app/codercom/issue/ENG-3029/oauth2-support-public-client

…-vocabulary

UpdateClientConfiguration wrote ClientType: string(req.DetermineClientType())
on every PUT, recomputed from the request instead of read from storage.
ApplyDefaults() fills an omitted token_endpoint_auth_method with
client_secret_basic, so a public client's PUT that only touched an
unrelated field (e.g. redirect_uris) silently converted it to
confidential, since DetermineClientType() can now return "public" where
it previously always returned "confidential".

A client's type is fixed at registration; RFC 7592 §2.2 permits
rejecting metadata the server will not accept. UpdateClientConfiguration
now rejects a PUT that would move a client between public and
confidential with 400 invalid_client_metadata, and carries the stored
client_type through verbatim rather than re-deriving it. A legacy row
whose stored client_type and token_endpoint_auth_method already
disagree can still manage itself, as long as the update does not also
ask to change the auth method.

ClientTypeFor(), extracted as its own function in the previous commit,
had exactly one caller and no second one materialized, so it is
inlined back into DetermineClientType().

Remove the function headers from the public-client token exchange and
client-secret requirement tests, whose names and subtest names already say
what they cover, and cut the inline explanations to the facts that are not
in the assertion messages.

Comments only, no behavior change.

The two call sites explained the same code_verifier length floor at
different lengths.

Comments only, no behavior change.

Comments only, no behavior change.

The field is not gated on Dynamic Client Registration, so advertising
none does not mean a client can register a public client. Point at
registration_endpoint, which is gated, for that.