…e' into plat479-4-consolidate-redirects

# Conflicts:
#	coderd/oauth2provider/authorize.go
#	coderd/oauth2provider/authorize_test.go

RFC 6749 Appendix A restricts error_description to NQSCHAR (%x20-21 /
%x23-5B / %x5D-7E), excluding the double quote and the backslash, and
applies the rule to the decoded value, so percent-encoding on the wire
does not satisfy it. Every invalid_scope description names the offending
scope with %q, which puts a quote on both ends and escapes any quote
inside, so each one emitted today falls outside the permitted set. The
PKCE method rejection is the same shape, rendering the raw query value.

Sanitizing at errorURL rather than at each caller also covers the
branches this PR did not touch, since errorURL is the single point every
4.1.2.1 description passes through. Quotes become apostrophes rather
than disappearing, because they delimit the offending value and a reader
needs to see where it starts and ends. The backslash goes with the quote
it escaped. Anything outside printable ASCII becomes a space.

The shared redirect assertion in authorize_test.go now checks the
charset, so every branch reaching it is covered rather than only the
cases with a test of their own.

url.Parse returns a nil *url.URL alongside its error. RedirectURL
recorded the validation error and then fell through to the exact-match
comparison, calling String() on that nil. A redirect_uri of %00, %7F, or
:// reaches it on both authorize verbs and in the POST /oauth2/tokens
form body.

POST /oauth2/tokens takes no API key, so an unauthenticated caller who
knows a public client ID can make the server capture a stack trace and
write it to the log on demand. httpmw.Recover keeps the trace out of the
response, so the cost is log volume and wasted work rather than
disclosure.

Returning base is safe for both callers: p.Errors is non-empty by that
point, so extractAuthorizeParams returns before reading the value and
tokens.go discards it.

…callback

withQuery seeded its query from the registered callback's own, so an app
registered with code, error, error_description, or state in its callback
URL received that value back alongside the one the response set.
Registration validates the scheme and rejects fragments but says nothing
about the query, so such a callback is accepted.

The success path was the damaging one: with error= registered, a code
redirect carried both, and a client following 4.1.2.1 checks error first,
discards a valid code, and can never complete the flow. The four reserved
parameters are now cleared before set runs. The rest of the registered
query stays, which 3.1.2 requires.

The doc comment claimed the copy existed because one request builds
several destinations from the same callback. No path calls withQuery
twice; the reason is inherited from the aliasing bug this branch removed.
What the copy actually protects is String, which ProcessAuthorize records
on the code row from the same pointer codeURL would otherwise mutate.

Set, not Add, moves from errorURL to withQuery, where the state it
justifies is actually set.

The method was validated on POST only, so a request carrying
code_challenge_method=plain rendered the consent page, and the user
learned the request could never succeed only after clicking Allow. That
contradicts the invariant the handler's own comment states, and it is the
one post-extraction check left without parity after response_type was
brought to both verbs.

GET validates without defaulting an omitted method: only POST records the
method on the code row, and the validator accepts an empty value, so both
verbs accept the same set.

Delivering an authorization error to the client's callback puts the error
code and description in a Location header, which loggermw does not
record. A failed authorization therefore logged status_code=302, byte
identical to the successful one, and the diagnosis existed only in the
app's logs rather than Coder's. An operator handed "our login is broken"
lost the signal a 400 used to give them.

redirectAuthorizeError now logs once, which covers the invalid_scope
redirects that were already silent as well as the four this branch adds.
Info, not Warn: these are client errors, and one line per failed
authorization is in proportion to the request logging already emitted.
The app comes from the request context, which the route's middleware
guarantees, so the call sites pass only the logger they already hold.

… make

Five comments and one test message state things the code does not do.

validatedCallbackURL's doc says requiring the type is what keeps the error
redirects from becoming open redirects. Every consumer is in this package,
where a composite literal still compiles, so the caveat removed in
bf4c089 goes back: the guarantee holds across packages and not within
one.

redirectAuthorizeError said §4.1.2.1 requires informing the user "there",
meaning neither of the two places the sentence had just named, and gave
the RFC as the reason extractAuthorizeParams failures answer on Coder.
That reason is false for most of them: only a mismatched or unparsable
redirect_uri is untrusted, while a bad code_challenge, a bad resource, or
an excess parameter leaves a callback that was exact-matched. The real
reason is that the parser reports one verdict for every field at once.
UnparseableResponseTypeNotRedirected asserted the same false rationale,
and its own request omits redirect_uri, so the callback it describes as
untrusted is the registered one.

Both scheme-check comments enumerated the redirects that consume the URL,
a list this branch already outgrew. They now count nothing.

The POST response_type branch explained less than its GET twin, and the
GET branch did not record why an unsupported_response_type error goes in
the query when §4.2.2.1 puts implicit-grant errors in the fragment.

…evels

One value carried three names: the params field was redirectURL, the type
wraps a field named callback, and use sites read
params.redirectURL.callback, where redirectURL no longer holds a URL.

The field becomes callback and the wrapped URL becomes url, so the use
sites read params.callback.url and each name says what it is at its own
level. url as a field name is unambiguous alongside the imported package,
since field access is always qualified.

AuthorizeOAuth2AppExpectingRedirectError asserted the same six facts as
requireAuthorizeErrorRedirect, more weakly, and had drifted on arrival. It
compared Host and Path but not Scheme, which is the field
ValidateRedirectURIScheme and DangerousCallbackSchemeNotRedirected exist
to defend, and asserted only that error_description was non-empty, so its
one caller proved that some invalid_request came back rather than that the
PKCE branch produced it.

The exported helper and TestOAuth2PKCEPlainMethodRejected go away. What
that test covered beyond the in-package case is an explicit redirect_uri
rather than an omitted one, which is a real axis through the parser, so it
becomes two more sub-cases of InvalidPKCEMethodRedirected.

Three unrelated snags in the same file.

Two cases deferred a body close inside a loop, so the closes ran at
subtest exit rather than per iteration. A subtest per method closes per
iteration and puts the method in the failure name rather than in every
message.

The second seedApp closure shadowed the first by name while taking a
different arity and capturing a different db. It becomes
seedAppInCatalog, which is what it does.

cancelLinkFromConsentPage indexed and sliced twice and hand-checked the
-1 sentinel. strings.Cut returns the remainder and an ok bool in one
call.

GET can now answer a §4.1.2.1 failure with a 302, and the POST 302 may
carry an error instead of a code, so the generated reference told an
integrator to read every 302 as success and pull an empty `code` with no
error branch. Adds the 302 case to GET and rewords the POST one.

The advertised `response_type` enum still lists `token`. It is shared
with `response_types_supported`, so correcting it is a separate change.

…llback

`unsupported_response_type` and `invalid_request` for a bad
`code_challenge_method` reach the registered callback rather than
terminating on Coder, in the shape the invalid_scope section already
established. The Limitations bullet said requests "return"
`unsupported_response_type`, which was unambiguous only while the answer
was a Coder page.