Files
Vasanth T 547330197b fix(oauth): give every loopback callback server the bounded shutdown (#1079)
* fix(oauth): give every loopback callback server the bounded shutdown

The loopback OAuth callback server was written out three times: the shared
oauth.LoopbackListener, MCP's Login and OpenRouter's login. Each ended with
Shutdown under a one-second deadline, and Shutdown only closes idle
connections. A client that has sent part of a request header is not idle, so it
survived the deadline, and its goroutine and the Serve goroutine stayed alive
after the flow returned (#1028).

#1051 fixed that for LoopbackListener (a header read bound, force-close when
Shutdown runs out of time, and a join on Serve) and left the other two exactly
as they were, because each was its own copy.

The lifecycle now lives once, in oauth.CallbackServer, with #1051's behavior
unchanged: ReadHeaderTimeout of 5s, Shutdown within 1s, Close() on a missed
deadline, and Close returning only after Serve has. LoopbackListener uses it
through an unexported constructor that keeps its ConnState test hook, and MCP's
Login and OpenRouterLogin use StartCallbackServer, which both packages could
already import.

Each call site gets a regression test that stalls a raw client mid-header ahead
of the real callback and then requires the stalled connection to be closed once
the login returns. The ordering does the synchronising: the stalled client is
dialled before the real callback, Serve accepts in order, and the login cannot
finish until the real callback has been served, so no sleep is involved.

Fixes #1028

* test(oauth): pin the callback server's header bound and its join on Serve

Two of the three things CallbackServer does had no test that noticed their
removal, the same as when #1051 introduced them on LoopbackListener: the
five-second header read bound, and Close waiting for the Serve goroutine.

The header test stalls a client mid-header and never calls Close, so it can
only pass on ReadHeaderTimeout; the bound is a package var so the test can
shorten it. It synchronises on StateNew. Go reports StateActive only after a
request read returns, and a stalled header read returns only when the header
timeout fires, so a helper waiting for StateActive depended on the bound under
test and turned its removal into a SETUP INVALID instead of a failure.

The join test uses the case where the join matters: with no connections open,
Shutdown closes the listener and returns at once, so without the join Close can
return before Serve does. Without it the test failed 10 runs out of 10.
2026-10-12 09:33:10 +08:00

135 lines
4.8 KiB
Go

package oauth
import (
"context"
"errors"
"fmt"
"io"
"net"
"net/http"
"net/url"
"strings"
)
// LoopbackListener is a single-use loopback HTTP server that captures an OAuth
// redirect (?code=&state=) on 127.0.0.1 only: bind an OS-assigned loopback port,
// verify the CSRF state, hand back the code, then close.
type LoopbackListener struct {
listener net.Listener
state string
result chan callbackResult
server *CallbackServer
}
type callbackResult struct {
code string
err error
}
// NewLoopbackListener binds 127.0.0.1:0 (loopback only, OS-assigned port) and
// begins serving. state is the CSRF value the callback must echo back. Call
// RedirectURI to build the redirect_uri, then Wait for the code. Always Close.
func NewLoopbackListener(state string) (*LoopbackListener, error) {
return NewLoopbackListenerOnPort(state, 0)
}
// NewLoopbackListenerOnPort is like NewLoopbackListener but binds a specific
// port (0 = OS-assigned). Used by ChatGPT OAuth which requires a fixed
// redirect_uri of http://localhost:1455/auth/callback.
func NewLoopbackListenerOnPort(state string, port int) (*LoopbackListener, error) {
return newLoopbackListener(state, port, nil)
}
// newLoopbackListener is NewLoopbackListenerOnPort with an optional ConnState
// hook, set before the Serve goroutine starts so tests can synchronize on
// connection lifecycle events (e.g. a stalled request header being accepted)
// without racing the running server.
func newLoopbackListener(state string, port int, connState func(net.Conn, http.ConnState)) (*LoopbackListener, error) {
if strings.TrimSpace(state) == "" {
return nil, errors.New("oauth: loopback listener requires a non-empty CSRF state")
}
ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port))
if err != nil {
return nil, fmt.Errorf("oauth: start loopback redirect listener: %w", err)
}
l := &LoopbackListener{
listener: ln,
state: state,
result: make(chan callbackResult, 1),
}
l.server = startCallbackServer(ln, http.HandlerFunc(l.handle), connState)
return l, nil
}
// RedirectURI returns the http://127.0.0.1:<port>/callback redirect URI.
func (l *LoopbackListener) RedirectURI() string {
return fmt.Sprintf("http://%s/callback", l.listener.Addr().String())
}
// RedirectURIWithHost returns a redirect URI using the given host (e.g.
// "localhost") and path (e.g. "/auth/callback"). The listener still binds
// 127.0.0.1, but the OAuth client may require "localhost" in the redirect_uri
// (OpenAI's ChatGPT client registration does). The listener accepts both
// /callback and the given path.
func (l *LoopbackListener) RedirectURIWithHost(host, path string) string {
_, port, _ := net.SplitHostPort(l.listener.Addr().String())
return fmt.Sprintf("http://%s:%s%s", host, port, path)
}
func (l *LoopbackListener) handle(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/callback" && r.URL.Path != "/auth/callback" {
http.NotFound(w, r)
return
}
code, err := parseCallback(r.URL.Query(), l.state)
if err != nil {
w.WriteHeader(http.StatusBadRequest)
_, _ = io.WriteString(w, "Authorization failed. You may close this window.")
} else {
_, _ = io.WriteString(w, "Authorization complete. You may close this window.")
}
select {
case l.result <- callbackResult{code: code, err: err}:
default:
}
}
// Wait blocks until the callback arrives or ctx is done, returning the
// authorization code.
func (l *LoopbackListener) Wait(ctx context.Context) (string, error) {
select {
case res := <-l.result:
return res.code, res.err
case <-ctx.Done():
return "", fmt.Errorf("oauth: timed out waiting for authorization callback: %w", ctx.Err())
}
}
// Close shuts the listener down (bounded), idempotent. If a client leaves a
// connection open past the shutdown deadline (e.g. a stalled request header),
// Close forcibly closes it rather than leaking it past the end of the flow,
// and waits for the Serve goroutine to finish before returning. See
// CallbackServer.Close.
func (l *LoopbackListener) Close() {
l.server.Close()
}
// parseCallback validates the redirect query and returns the authorization code,
// rejecting a mismatched state (CSRF) and surfacing provider errors.
func parseCallback(values url.Values, wantState string) (string, error) {
if got := values.Get("state"); got != wantState {
return "", ErrStateMismatch
}
if providerErr := strings.TrimSpace(values.Get("error")); providerErr != "" {
if desc := strings.TrimSpace(values.Get("error_description")); desc != "" {
return "", fmt.Errorf("oauth: authorization server returned error %q: %s", providerErr, desc)
}
return "", fmt.Errorf("oauth: authorization server returned error %q", providerErr)
}
code := strings.TrimSpace(values.Get("code"))
if code == "" {
return "", errors.New("oauth: callback missing authorization code")
}
return code, nil
}