mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 17:57:54 -10:00
* fix(oauth): add missing api.connectors scopes for ChatGPT OAuth preset * fix(oauth): add missing authorize params for ChatGPT OAuth (id_token_add_organizations, codex_cli_simplified_flow, originator) * fix(oauth): use localhost + /auth/callback for ChatGPT redirect URI (matches Codex CLI registration) * fix(oauth): use fixed port 1455 + localhost/auth/callback for ChatGPT (matches opencode + Codex CLI client registration) * chore: gofmt chatgpt.go * refactor(oauth): use RedirectURIWithHost for ChatGPT callback; drop dead Port() RedirectURIWithHost and Port() were added but never called — chatgpt.go hardcoded "http://localhost:1455/auth/callback" as a literal, duplicating both the port and path. Use RedirectURIWithHost("localhost", "/auth/callback") so the redirect_uri derives its port from the listener (single source of truth via the new chatgptCallbackPort constant), and remove the unused Port() method. Also make the fixed-port bind failure actionable: ChatGPT's client registration requires exactly port 1455, so we can't fall back to a random port — but if 1455 is already bound (a prior Zero/Codex login still open) the error now says so instead of surfacing a raw OS bind error. Tests: RedirectURIWithHost host/path formatting, and that the listener captures the code on the /auth/callback path (not just /callback) — both were previously uncovered. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix(oauth): restore api.connectors scopes for ChatGPT preset (review fix) The fixed-port commit accidentally reverted internal/oauth/presets.go back to only openid/profile/email/offline_access, dropping the api.connectors.read and api.connectors.invoke scopes this PR exists to add — so authorize URLs still omitted them and the authorize_hydra_invalid_request failure was not actually fixed (caught in review by anandh8x). Restore both connector scopes in the chatgpt preset and pin them with tests: - TestResolveConfigChatGPTPreset now asserts both connector scopes are present (not just allowed). - New TestChatGPTAuthorizeURLIncludesConnectorScopes inspects the generated authorize URL and asserts the scope param carries api.connectors.read and api.connectors.invoke, so a future revert is caught end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix(oauth): read chatgpt_account_id from the nested OpenAI auth claim (P0) extractChatGPTAccountID read chatgpt_account_id as a TOP-LEVEL id_token claim, but auth.openai.com namespaces it under the "https://api.openai.com/auth" claim object (the Codex CLI reads only from there). Against a real token the top-level lookup returned "", so token.Account stayed empty → the codex provider omitted the required chatgpt-account-id header on every request → Cloudflare/backend 401. The login itself succeeded (bearer minted), so the failure was silent: the user authenticated fine, then every Codex API call 401'd. The unit tests passed only because the fixture put the claim at the top level, masking the bug. Read the nested path first (id_token[ns][chatgpt_account_id]), matching codex; keep a bare top-level claim as a forward-compat fallback only. Tests now build the fixture with the nested shape a real token uses, plus regressions: a top-level decoy must not shadow the nested value, and the top-level fallback still works when the namespace is absent. Found by an adversarial audit against the codex reference (same OAuth client). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
125 lines
3.3 KiB
Go
125 lines
3.3 KiB
Go
package oauth
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net/http"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
func TestLoopbackCapturesCode(t *testing.T) {
|
|
l, err := NewLoopbackListener("the-state")
|
|
if err != nil {
|
|
t.Fatalf("NewLoopbackListener: %v", err)
|
|
}
|
|
defer l.Close()
|
|
if !strings.HasPrefix(l.RedirectURI(), "http://127.0.0.1:") {
|
|
t.Fatalf("redirect URI not loopback: %q", l.RedirectURI())
|
|
}
|
|
go func() {
|
|
_, _ = http.Get(l.RedirectURI() + "?code=auth-code&state=the-state")
|
|
}()
|
|
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
|
|
defer cancel()
|
|
code, err := l.Wait(ctx)
|
|
if err != nil {
|
|
t.Fatalf("Wait: %v", err)
|
|
}
|
|
if code != "auth-code" {
|
|
t.Fatalf("code = %q, want auth-code", code)
|
|
}
|
|
}
|
|
|
|
func TestNewLoopbackListenerRejectsEmptyState(t *testing.T) {
|
|
if _, err := NewLoopbackListener(" "); err == nil {
|
|
t.Fatal("an empty CSRF state must be rejected (fail closed)")
|
|
}
|
|
}
|
|
|
|
func TestRedirectURIWithHostUsesGivenHostAndPath(t *testing.T) {
|
|
l, err := NewLoopbackListener("the-state")
|
|
if err != nil {
|
|
t.Fatalf("NewLoopbackListener: %v", err)
|
|
}
|
|
defer l.Close()
|
|
got := l.RedirectURIWithHost("localhost", "/auth/callback")
|
|
if !strings.HasPrefix(got, "http://localhost:") {
|
|
t.Fatalf("redirect URI host = %q, want localhost", got)
|
|
}
|
|
if !strings.HasSuffix(got, "/auth/callback") {
|
|
t.Fatalf("redirect URI path = %q, want /auth/callback suffix", got)
|
|
}
|
|
}
|
|
|
|
func TestLoopbackCapturesCodeOnAuthCallbackPath(t *testing.T) {
|
|
// The ChatGPT flow uses /auth/callback, not /callback; the listener must
|
|
// capture the code on that path too.
|
|
l, err := NewLoopbackListener("the-state")
|
|
if err != nil {
|
|
t.Fatalf("NewLoopbackListener: %v", err)
|
|
}
|
|
defer l.Close()
|
|
uri := l.RedirectURIWithHost("127.0.0.1", "/auth/callback")
|
|
go func() {
|
|
_, _ = http.Get(uri + "?code=auth-code&state=the-state")
|
|
}()
|
|
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
|
|
defer cancel()
|
|
code, err := l.Wait(ctx)
|
|
if err != nil {
|
|
t.Fatalf("Wait: %v", err)
|
|
}
|
|
if code != "auth-code" {
|
|
t.Fatalf("code = %q, want auth-code", code)
|
|
}
|
|
}
|
|
|
|
func TestLoopbackRejectsStateMismatch(t *testing.T) {
|
|
l, err := NewLoopbackListener("expected-state")
|
|
if err != nil {
|
|
t.Fatalf("NewLoopbackListener: %v", err)
|
|
}
|
|
defer l.Close()
|
|
go func() {
|
|
_, _ = http.Get(l.RedirectURI() + "?code=x&state=WRONG")
|
|
}()
|
|
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
|
|
defer cancel()
|
|
_, err = l.Wait(ctx)
|
|
if !errors.Is(err, ErrStateMismatch) {
|
|
t.Fatalf("Wait err = %v, want ErrStateMismatch", err)
|
|
}
|
|
}
|
|
|
|
func TestLoopbackTimesOut(t *testing.T) {
|
|
l, err := NewLoopbackListener("s")
|
|
if err != nil {
|
|
t.Fatalf("NewLoopbackListener: %v", err)
|
|
}
|
|
defer l.Close()
|
|
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond)
|
|
defer cancel()
|
|
if _, err := l.Wait(ctx); err == nil {
|
|
t.Fatal("Wait should time out cleanly")
|
|
}
|
|
}
|
|
|
|
func TestLoopbackProviderError(t *testing.T) {
|
|
l, err := NewLoopbackListener("s")
|
|
if err != nil {
|
|
t.Fatalf("NewLoopbackListener: %v", err)
|
|
}
|
|
defer l.Close()
|
|
go func() {
|
|
_, _ = http.Get(l.RedirectURI() + "?error=access_denied&error_description=nope&state=s")
|
|
}()
|
|
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
|
|
defer cancel()
|
|
_, err = l.Wait(ctx)
|
|
if err == nil || !strings.Contains(err.Error(), "access_denied") {
|
|
t.Fatalf("Wait err = %v, want provider error", err)
|
|
}
|
|
}
|