From fdce6286ab451b20ff39dfe5ad2a3e8b82daeb0f Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 11 Nov 2026 19:11:24 +0100 Subject: [PATCH 1/5] fix: let `ory auth` finish when other programs load the login URLs Programs on the machine that fetch the pages the browser visits, such as the link previews of macOS, can request the URLs of the browser login without the browser's cookies. Ory Network then redirects them to the CLI's callback with an error, and the CLI took the first callback as the result. `ory auth` failed with "No CSRF value available in the session cookie" or "The consent verifier has already been used". A second request to the callback made the CLI panic with "close of closed channel" and redeem the code a second time. The browser now opens http://127.0.0.1:/start. Every request to it begins a login attempt with its own state and PKCE verifier, and sets a session cookie holding the state. The PKCE verifier stays in the CLI. A callback completes the login only if its state matches the cookie and belongs to an attempt. Other callbacks get an error page and the login keeps waiting. The first accepted callback redeems the code, and repeated callbacks get the same answer. In each browser, only the attempt opened last can complete. `ory auth --no-browser` prints the sign-in link without opening a browser. Co-authored-by: Claude --- cmd/cloudx/auth.go | 6 +- cmd/cloudx/client/auth.go | 240 +++++++++---- cmd/cloudx/client/auth_browser_test.go | 464 +++++++++++++++++++++++++ cmd/cloudx/client/command_helper.go | 8 + 4 files changed, 646 insertions(+), 72 deletions(-) create mode 100644 cmd/cloudx/client/auth_browser_test.go diff --git a/cmd/cloudx/auth.go b/cmd/cloudx/auth.go index 9fc24954..64886657 100644 --- a/cmd/cloudx/auth.go +++ b/cmd/cloudx/auth.go @@ -8,14 +8,17 @@ import ( "github.com/ory/cli/cmd/cloudx/client" "github.com/ory/x/cmdx" + "github.com/ory/x/flagx" ) +const flagNoBrowser = "no-browser" + func NewAuthCmd() *cobra.Command { cmd := &cobra.Command{ Use: "auth", Short: "Create a new Ory Network account or sign in to an existing account.", RunE: func(cmd *cobra.Command, args []string) error { - h, err := client.NewCobraCommandHelper(cmd) + h, err := client.NewCobraCommandHelper(cmd, client.WithNoBrowser(flagx.MustGetBool(cmd, flagNoBrowser))) if err != nil { return err } @@ -33,6 +36,7 @@ func NewAuthCmd() *cobra.Command { client.RegisterYesFlag(cmd.PersistentFlags()) cmdx.RegisterNoiseFlags(cmd.PersistentFlags()) cmdx.RegisterFormatFlags(cmd.Flags()) + cmd.Flags().Bool(flagNoBrowser, false, "Print the sign-in link instead of opening it in the browser.") cmd.AddCommand(NewLogoutCmd()) return cmd } diff --git a/cmd/cloudx/client/auth.go b/cmd/cloudx/client/auth.go index 4b38756a..e5d33ff9 100644 --- a/cmd/cloudx/client/auth.go +++ b/cmd/cloudx/client/auth.go @@ -13,6 +13,7 @@ import ( "net/url" "slices" "strings" + "sync" "time" "github.com/gofrs/uuid" @@ -173,91 +174,188 @@ func (h *CommandHelper) loginOAuth2(ctx context.Context) (*Config, error) { return config, nil } -func (h *CommandHelper) oAuth2DanceWithServer(ctx context.Context, client *oauth2.Config) (token *oauth2.Token, err error) { - var ( - state = randx.MustString(32, randx.AlphaNum) - pkceVerifier = oauth2.GenerateVerifier() - serverErr = make(chan error) - serverToken = make(chan *oauth2.Token) - ) +func (h *CommandHelper) oAuth2DanceWithServer(ctx context.Context, client *oauth2.Config) (*oauth2.Token, error) { l, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { return nil, fmt.Errorf("failed to allocate port for OAuth2 callback handler, try again later: %w", err) } - client.RedirectURL = fmt.Sprintf("http://%s/callback", l.Addr().String()) - - srv := http.Server{ - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // for retries the user has to start from the beginning - defer close(serverErr) - defer close(serverToken) - - ctx := r.Context() - if err := r.ParseForm(); err != nil { - redirectErr(w, r, "parse form", "An error occurred during CLI authentication. Please try again") - serverErr <- fmt.Errorf("failed to parse form: %w", err) - return - } - if s := r.Form.Get("state"); s != state { - redirectErr(w, r, "state mismatch", "An error occurred during CLI authentication. Please try again") - serverErr <- fmt.Errorf("state mismatch: expected %q, got %q", state, s) - return - } - if r.Form.Has("error") { - e, d := r.Form.Get("error"), r.Form.Get("error_description") - redirectErr(w, r, e, d) - serverErr <- fmt.Errorf("upsteam error: %s: %s", e, d) - return - } - code := r.Form.Get("code") - if code == "" { - redirectErr(w, r, "missing code", "An error occurred during CLI authentication. Please try again") - serverErr <- fmt.Errorf("missing code") - return - } - t, err := client.Exchange( - ctx, - code, - oauth2.VerifierOption(pkceVerifier), + client.RedirectURL = fmt.Sprintf("http://%s/callback", l.Addr()) + + login := &loopbackLogin{ + // Cookies are not separated by port, so concurrent logins need distinct + // names. + cookieName: fmt.Sprintf("ory_cli_login_%d", l.Addr().(*net.TCPAddr).Port), + authURL: func(state, pkceVerifier string) string { + return client.AuthCodeURL(state, + oauth2.S256ChallengeOption(pkceVerifier), + oauth2.SetAuthURLParam("scope", "offline_access email profile"), + oauth2.SetAuthURLParam("response_type", "code"), + oauth2.SetAuthURLParam("prompt", "login consent"), + oauth2.SetAuthURLParam("audience", CloudConsoleURL("api").String()), ) - if err != nil { - redirectErr(w, r, "token exchange", "An error occurred during the OAuth2 token exchange") - serverErr <- fmt.Errorf("failed OAuth2 token exchange: %w", err) - return - } - redirectOK(w, r) - serverToken <- t - }), + }, + exchange: func(code, pkceVerifier string) (*oauth2.Token, error) { + return client.Exchange(ctx, code, oauth2.VerifierOption(pkceVerifier)) + }, + pkceVerifiers: make(map[string]string), + done: make(chan struct{}), + } + + handler := login.handler() + if h.wrapLoopbackHandler != nil { + handler = h.wrapLoopbackHandler(handler) } + srv := http.Server{Handler: handler, ReadHeaderTimeout: 10 * time.Second} go func() { _ = srv.Serve(l) }() defer srv.Close() - - u := client.AuthCodeURL(state, - oauth2.S256ChallengeOption(pkceVerifier), - oauth2.SetAuthURLParam("scope", "offline_access email profile"), - oauth2.SetAuthURLParam("response_type", "code"), - oauth2.SetAuthURLParam("prompt", "login consent"), - oauth2.SetAuthURLParam("audience", CloudConsoleURL("api").String()), - ) - if err := h.openBrowserHook(u); err != nil { - return nil, err + defer func() { + // Shutdown lets the browser receive the redirect it waits for. + shutdownCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second) + defer cancel() + _ = srv.Shutdown(shutdownCtx) + }() + + startURL := fmt.Sprintf("http://%s/start", l.Addr()) + if h.noBrowser { + _, _ = fmt.Fprintln(h.VerboseErrWriter, "Open the below page in a browser on this computer to complete your login to Ory Network:") + } else { + if err := h.openBrowserHook(startURL); err != nil { + return nil, err + } + _, _ = fmt.Fprintln(h.VerboseErrWriter, `A browser should have opened for you to complete your login to Ory Network. +If no browser opened, visit the below page to continue:`) } - _, _ = fmt.Fprintf(h.VerboseErrWriter, - `A browser should have opened for you to complete your login to Ory Network. -If no browser opened, visit the below page to continue: - - %s - -`, u) + _, _ = fmt.Fprintf(h.VerboseErrWriter, "\n\t\t%s\n\n", startURL) select { case <-ctx.Done(): return nil, ctx.Err() - case token := <-serverToken: - return token, nil - case err := <-serverErr: - return nil, err + case <-login.done: + return login.result.token, login.result.err + } +} + +// loopbackLogin is the CLI's side of the browser login: it sends the browser +// to the authorization server and receives the callback with the result. +// +// Every request to the start URL begins an attempt with its own state and PKCE +// verifier, and sets a cookie holding the state. A callback completes the +// login only if its state matches the cookie and belongs to an attempt. Other +// programs on the machine, such as link previews that fetch the pages the +// browser visits, may request the start and callback URLs too. They get +// attempts of their own or are rejected, and leave the login running. +type loopbackLogin struct { + // All attempts share the cookie name, so in each browser only the latest + // attempt can complete, as with the authorization server's CSRF cookie. + cookieName string + authURL func(state, pkceVerifier string) string + exchange func(code, pkceVerifier string) (*oauth2.Token, error) + + mu sync.Mutex + // pkceVerifiers are keyed by state. + pkceVerifiers map[string]string + + once sync.Once + // done is closed after the first callback from a browser has set result. + done chan struct{} + result loopbackLoginResult +} + +const maxLoginAttempts = 32 + +type loopbackLoginResult struct { + token *oauth2.Token + err error + // browserError and browserErrorDescription are shown in the browser if + // err is set. + browserError, browserErrorDescription string +} + +func (l *loopbackLogin) handler() http.Handler { + mux := http.NewServeMux() + mux.HandleFunc("GET /start", l.start) + mux.HandleFunc("GET /callback", l.callback) + return mux +} + +func (l *loopbackLogin) start(w http.ResponseWriter, r *http.Request) { + state := randx.MustString(32, randx.AlphaNum) + pkceVerifier := oauth2.GenerateVerifier() + l.mu.Lock() + full := len(l.pkceVerifiers) >= maxLoginAttempts + if !full { + l.pkceVerifiers[state] = pkceVerifier + } + l.mu.Unlock() + if full { + http.Error(w, "This login was opened too many times. Run the command again.", http.StatusTooManyRequests) + return + } + + http.SetCookie(w, l.cookie(state, 0)) + http.Redirect(w, r, l.authURL(state, pkceVerifier), http.StatusFound) +} + +func (l *loopbackLogin) callback(w http.ResponseWriter, r *http.Request) { + pkceVerifier, ok := l.pkceVerifierOf(r) + if !ok { + http.Error(w, "This login was started in another browser window. Continue in the window where you last opened the login link, or run the command again.", http.StatusForbidden) + return + } + http.SetCookie(w, l.cookie("", -1)) + + // A repeated callback waits for the first one and gets the same answer. + l.once.Do(func() { + l.result = l.complete(r, pkceVerifier) + close(l.done) + }) + if l.result.err != nil { + redirectErr(w, r, l.result.browserError, l.result.browserErrorDescription) + return + } + redirectOK(w, r) +} + +func (l *loopbackLogin) cookie(value string, maxAge int) *http.Cookie { + return &http.Cookie{ + Name: l.cookieName, + Value: value, + Path: "/callback", + MaxAge: maxAge, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + } +} + +func (l *loopbackLogin) pkceVerifierOf(r *http.Request) (string, bool) { + state := r.URL.Query().Get("state") + if c, err := r.Cookie(l.cookieName); err != nil || c.Value != state { + return "", false + } + l.mu.Lock() + defer l.mu.Unlock() + pkceVerifier, ok := l.pkceVerifiers[state] + return pkceVerifier, ok +} + +func (l *loopbackLogin) complete(r *http.Request, pkceVerifier string) loopbackLoginResult { + const tryAgain = "An error occurred during CLI authentication. Please try again" + if err := r.ParseForm(); err != nil { + return loopbackLoginResult{err: fmt.Errorf("failed to parse form: %w", err), browserError: "parse form", browserErrorDescription: tryAgain} + } + if r.Form.Has("error") { + e, d := r.Form.Get("error"), r.Form.Get("error_description") + return loopbackLoginResult{err: fmt.Errorf("upstream error: %s: %s", e, d), browserError: e, browserErrorDescription: d} + } + code := r.Form.Get("code") + if code == "" { + return loopbackLoginResult{err: stderrors.New("missing code"), browserError: "missing code", browserErrorDescription: tryAgain} + } + token, err := l.exchange(code, pkceVerifier) + if err != nil { + return loopbackLoginResult{err: fmt.Errorf("failed OAuth2 token exchange: %w", err), browserError: "token exchange", browserErrorDescription: "An error occurred during the OAuth2 token exchange"} } + return loopbackLoginResult{token: token} } func redirectOK(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/cloudx/client/auth_browser_test.go b/cmd/cloudx/client/auth_browser_test.go new file mode 100644 index 00000000..61084425 --- /dev/null +++ b/cmd/cloudx/client/auth_browser_test.go @@ -0,0 +1,464 @@ +// Copyright © 2026 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "context" + "crypto/rand" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "net/http/cookiejar" + "net/http/httptest" + "net/url" + "slices" + "strings" + "sync" + "testing" + "time" + + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" +) + +const linkPreviewUserAgent = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/601.2.4 (KHTML, like Gecko) Version/9.0.1 Safari/601.2.4 facebookexternalhit/1.1 Facebot Twitterbot/1.0" + +type fakeAuthorizationHooks struct { + // Each hook runs while the browser waits for the answer to its request, so + // whatever the hook sends reaches the CLI before the browser continues. + beforeCallback, duringRedemption func(callbackURL string) +} + +// fakeAuthorizationServer signs in every browser at once: it answers an +// authorization request with a redirect to the client that carries a code, or +// an error if access is denied. Link previews cannot sign in and get the login +// page instead. Like Ory Network, it revokes the token issued for a code that +// is redeemed twice. It also serves the Console pages the CLI redirects the +// browser to. +type fakeAuthorizationServer struct { + t *testing.T + URL string + deny bool + hooks fakeAuthorizationHooks + + mu sync.Mutex + // flows are keyed by their code. + flows map[string]*fakeFlow + revoked bool +} + +type fakeFlow struct { + authorizeRequest url.Values + callbackURL string + redemptions int +} + +func newFakeAuthorizationServer(t *testing.T, deny bool, hooks fakeAuthorizationHooks) *fakeAuthorizationServer { + f := &fakeAuthorizationServer{t: t, deny: deny, hooks: hooks, flows: map[string]*fakeFlow{}} + + mux := http.NewServeMux() + mux.HandleFunc("GET /oauth2/auth", f.authorize) + mux.HandleFunc("POST /oauth2/token", f.token) + mux.Handle("GET /cli-auth-success", fakePage("Signed in")) + mux.Handle("GET /error", fakePage("Error")) + + srv := httptest.NewUnstartedServer(mux) + // Browsers treat localhost and 127.0.0.1 as different sites, as they do + // Ory Network and the CLI's callback server. + f.URL = fmt.Sprintf("http://localhost:%d", srv.Listener.Addr().(*net.TCPAddr).Port) + srv.Start() + t.Cleanup(srv.Close) + return f +} + +func fakePage(body string) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, _ = fmt.Fprintf(w, "%s", body) + } +} + +func (f *fakeAuthorizationServer) authorize(w http.ResponseWriter, r *http.Request) { + if r.UserAgent() == linkPreviewUserAgent { + fakePage("Sign in")(w, r) + return + } + q := r.URL.Query() + assert.Equal(f.t, "ory-cli", q.Get("client_id")) + assert.Equal(f.t, "code", q.Get("response_type")) + assert.Equal(f.t, "S256", q.Get("code_challenge_method")) + assert.NotEmpty(f.t, q.Get("code_challenge")) + assert.NotEmpty(f.t, q.Get("state")) + + params := url.Values{"state": {q.Get("state")}} + if f.deny { + params.Set("error", "access_denied") + params.Set("error_description", "The resource owner denied the request") + } else { + params.Set("code", rand.Text()) + } + callback := q.Get("redirect_uri") + "?" + params.Encode() + if !f.deny { + f.mu.Lock() + f.flows[params.Get("code")] = &fakeFlow{authorizeRequest: q, callbackURL: callback} + f.mu.Unlock() + } + + if f.hooks.beforeCallback != nil { + f.hooks.beforeCallback(callback) + } + http.Redirect(w, r, callback, http.StatusSeeOther) +} + +func (f *fakeAuthorizationServer) token(w http.ResponseWriter, r *http.Request) { + tokenError := func(err, description string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{"error": err, "error_description": description}) + } + if !assert.NoError(f.t, r.ParseForm()) { + tokenError("invalid_request", "unparsable form") + return + } + + f.mu.Lock() + flow, ok := f.flows[r.PostForm.Get("code")] + valid := ok && + r.PostForm.Get("grant_type") == "authorization_code" && + r.PostForm.Get("client_id") == flow.authorizeRequest.Get("client_id") && + r.PostForm.Get("redirect_uri") == flow.authorizeRequest.Get("redirect_uri") && + oauth2.S256ChallengeFromVerifier(r.PostForm.Get("code_verifier")) == flow.authorizeRequest.Get("code_challenge") + var reused bool + if valid { + flow.redemptions++ + reused = flow.redemptions > 1 + f.revoked = f.revoked || reused + } + f.mu.Unlock() + + switch { + case !valid: + tokenError("invalid_grant", "The provided authorization grant is invalid.") + return + case reused: + tokenError("invalid_grant", "The authorization code has already been used.") + return + } + if f.hooks.duringRedemption != nil { + f.hooks.duringRedemption(flow.callbackURL) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "access-token", + "refresh_token": "refresh-token", + "token_type": "bearer", + "expires_in": 3600, + "scope": "offline_access email profile", + }) +} + +func (f *fakeAuthorizationServer) redeemed() (redemptions int, revoked bool) { + f.mu.Lock() + defer f.mu.Unlock() + for _, flow := range f.flows { + redemptions += flow.redemptions + } + return redemptions, f.revoked +} + +// errorCallback returns the callback that Ory Network sends for the same state +// when a link preview repeats one of the browser's requests to it. +func errorCallback(t *testing.T, callbackURL string) string { + u, err := url.Parse(callbackURL) + if !assert.NoError(t, err) { + return "" + } + u.RawQuery = url.Values{ + "error": {"request_forbidden"}, + "error_description": {"The request is not allowed. No CSRF value available in the session cookie."}, + "state": {u.Query().Get("state")}, + }.Encode() + return u.String() +} + +// linkPreview fetches a URL the way the link previews of macOS do: like the +// browser, but without its cookies and following redirects. A jar lets it keep +// the cookies it receives itself. +func linkPreview(t *testing.T, jar http.CookieJar) func(string) { + c := &http.Client{Jar: jar, Timeout: 10 * time.Second} + return func(u string) { + req, err := http.NewRequest(http.MethodGet, u, nil) + if err != nil { + t.Errorf("link preview request: %v", err) + return + } + req.Header.Set("User-Agent", linkPreviewUserAgent) + res, err := c.Do(req) + if err != nil { + t.Logf("link preview of %s: %v", u, err) + return + } + t.Logf("link preview of %s ended at %s with %d", u, res.Request.URL, res.StatusCode) + _, _ = io.Copy(io.Discard, res.Body) + _ = res.Body.Close() + } +} + +// signInWithBrowser opens the URL the CLI gives the browser and waits for the +// page the CLI sends the browser to at the end. +func signInWithBrowser(page playwright.Page, startURL string, deny bool) error { + if _, err := page.Goto(startURL); err != nil { + return err + } + landing, landingText := "/cli-auth-success", "Signed in" + if deny { + landing, landingText = "/error", "Error" + } + // Page.WaitForURL can miss a load event that fires while it starts + // waiting, so this waits for the content of the page instead. + if err := page.GetByText(landingText, playwright.PageGetByTextOptions{Exact: new(true)}).WaitFor(); err != nil { + return fmt.Errorf("the browser did not reach %s but stayed at %s: %w", landing, page.URL(), err) + } + if u, err := url.Parse(page.URL()); err != nil || u.Path != landing { + return fmt.Errorf("the browser is at %s instead of %s", page.URL(), landing) + } + return nil +} + +// browserLoginCookies returns the cookies the CLI set to recognize the browser. +func browserLoginCookies(browserContext playwright.BrowserContext) ([]playwright.Cookie, error) { + cookies, err := browserContext.Cookies() + if err != nil { + return nil, err + } + return slices.DeleteFunc(cookies, func(c playwright.Cookie) bool { + return !strings.HasPrefix(c.Name, "ory_cli_login_") + }), nil +} + +func TestOAuth2LoginInBrowser(t *testing.T) { + pw, err := playwright.Run() + require.NoError(t, err) + t.Cleanup(func() { _ = pw.Stop() }) + browser, err := pw.Chromium.Launch(playwright.BrowserTypeLaunchOptions{Headless: new(true)}) + require.NoError(t, err) + t.Cleanup(func() { _ = browser.Close() }) + + type setup struct { + hooks fakeAuthorizationHooks + // beforeBrowser runs before the browser opens the start URL. + beforeBrowser func(startURL string) + wrapLoopbackHandler func(http.Handler) http.Handler + check func(t *testing.T) + } + + for _, tc := range []struct { + name string + deny bool + setup func(t *testing.T, browserContext playwright.BrowserContext) setup + }{ + { + name: "signs in with a session cookie", + setup: func(t *testing.T, browserContext playwright.BrowserContext) setup { + duringLogin := make(chan []playwright.Cookie, 1) + return setup{ + hooks: fakeAuthorizationHooks{beforeCallback: func(string) { + cookies, err := browserLoginCookies(browserContext) + assert.NoError(t, err) + duringLogin <- cookies + }}, + check: func(t *testing.T) { + var cookies []playwright.Cookie + select { + case cookies = <-duringLogin: + default: + require.Fail(t, "the cookies were not read during the login") + } + require.Len(t, cookies, 1) + assert.EqualValues(t, -1, cookies[0].Expires, "the cookie must not outlive the browser session") + assert.Equal(t, "/callback", cookies[0].Path) + assert.True(t, cookies[0].HttpOnly) + assert.Equal(t, playwright.SameSiteAttributeLax, cookies[0].SameSite) + }, + } + }, + }, + { + name: "fails when the user denies access", + deny: true, + }, + { + name: "ignores an error callback caused by a link preview", + setup: func(t *testing.T, _ playwright.BrowserContext) setup { + preview := linkPreview(t, nil) + return setup{hooks: fakeAuthorizationHooks{beforeCallback: func(callback string) { + preview(errorCallback(t, callback)) + }}} + }, + }, + { + name: "ignores a link preview of the callback", + setup: func(t *testing.T, _ playwright.BrowserContext) setup { + return setup{hooks: fakeAuthorizationHooks{duringRedemption: linkPreview(t, nil)}} + }, + }, + { + name: "signs in after a link preview opened the login link first", + setup: func(t *testing.T, _ playwright.BrowserContext) setup { + return setup{beforeBrowser: linkPreview(t, nil)} + }, + }, + { + name: "ignores a link preview with the cookie of its own login attempt", + setup: func(t *testing.T, _ playwright.BrowserContext) setup { + jar, err := cookiejar.New(nil) + require.NoError(t, err) + preview := linkPreview(t, jar) + return setup{ + beforeBrowser: preview, + hooks: fakeAuthorizationHooks{beforeCallback: func(callback string) { + preview(errorCallback(t, callback)) + }}, + } + }, + }, + { + name: "ignores the callback of an older tab in the same browser", + setup: func(t *testing.T, browserContext playwright.BrowserContext) setup { + return setup{beforeBrowser: func(startURL string) { + // The request API of a browser context shares its cookies, + // as the tabs of a browser do. + tabs := browserContext.Request() + older, err := tabs.Get(startURL, playwright.APIRequestContextGetOptions{MaxRedirects: new(0)}) + require.NoError(t, err) + _, err = tabs.Get(startURL, playwright.APIRequestContextGetOptions{MaxRedirects: new(0)}) + require.NoError(t, err) + res, err := tabs.Get(older.Headers()["location"]) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.Status(), "the older tab must not end the login") + }} + }, + }, + { + name: "redeems the code once when the browser repeats the callback", + setup: func(t *testing.T, browserContext playwright.BrowserContext) setup { + type navigation struct { + landed string + err error + } + repeated := make(chan navigation, 1) + callbacks := make(chan struct{}, 2) + return setup{ + wrapLoopbackHandler: func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/callback" { + callbacks <- struct{}{} + } + next.ServeHTTP(w, r) + }) + }, + hooks: fakeAuthorizationHooks{duringRedemption: func(callback string) { + // The browser's first callback started this redemption. + <-callbacks + page, err := browserContext.NewPage() + if !assert.NoError(t, err) { + return + } + // Chromium's HTTP cache holds a request until an earlier + // one for the same URL is answered; routing it bypasses + // the cache, so both reach the CLI at the same time. + err = page.Route(func(u string) bool { return u == callback }, func(r playwright.Route) { + _ = r.Continue() + }) + if !assert.NoError(t, err) { + return + } + go func() { + _, err := page.Goto(callback) + repeated <- navigation{landed: page.URL(), err: err} + }() + select { + case <-callbacks: + case <-time.After(10 * time.Second): + t.Error("the repeated callback did not reach the CLI") + } + }}, + check: func(t *testing.T) { + select { + case n := <-repeated: + assert.NoError(t, n.err) + assert.Regexp(t, `/cli-auth-success$`, n.landed) + case <-time.After(10 * time.Second): + t.Error("the repeated callback was not answered") + } + }, + } + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + browserContext, err := browser.NewContext() + require.NoError(t, err) + t.Cleanup(func() { _ = browserContext.Close() }) + page, err := browserContext.NewPage() + require.NoError(t, err) + page.SetDefaultTimeout(10_000) + + var s setup + if tc.setup != nil { + s = tc.setup(t, browserContext) + } + authorizationServer := newFakeAuthorizationServer(t, tc.deny, s.hooks) + t.Setenv(ConsoleURLKey, authorizationServer.URL) + + signedIn := make(chan error, 1) + h := &CommandHelper{ + VerboseErrWriter: io.Discard, + openBrowserHook: func(uri string) error { + if s.beforeBrowser != nil { + s.beforeBrowser(uri) + } + go func() { signedIn <- signInWithBrowser(page, uri, tc.deny) }() + return nil + }, + wrapLoopbackHandler: s.wrapLoopbackHandler, + } + conf := oauth2ClientConfig() + conf.Endpoint.AuthURL = authorizationServer.URL + "/oauth2/auth" + conf.Endpoint.TokenURL = authorizationServer.URL + "/oauth2/token" + + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + token, err := h.oAuth2DanceWithServer(ctx, conf) + + redemptions, revoked := authorizationServer.redeemed() + if tc.deny { + assert.ErrorContains(t, err, "access_denied") + assert.Nil(t, token) + assert.Zero(t, redemptions) + } else if assert.NoError(t, err) { + assert.Equal(t, "access-token", token.AccessToken) + assert.Equal(t, 1, redemptions, "the code must be redeemed exactly once") + assert.False(t, revoked, "the issued token must not be revoked") + } + select { + case err := <-signedIn: + assert.NoError(t, err) + case <-time.After(20 * time.Second): + t.Error("the browser did not finish the login") + } + if s.check != nil { + s.check(t) + } + + remaining, err := browserLoginCookies(browserContext) + require.NoError(t, err) + assert.Empty(t, remaining, "the callback must clear the cookie") + }) + } +} diff --git a/cmd/cloudx/client/command_helper.go b/cmd/cloudx/client/command_helper.go index a25b912c..6cdc68b9 100644 --- a/cmd/cloudx/client/command_helper.go +++ b/cmd/cloudx/client/command_helper.go @@ -48,10 +48,12 @@ type ( projectID, workspaceID uuid.UUID configLocation string noConfirm, isQuiet bool + noBrowser bool workspaceFromConfig bool VerboseErrWriter io.Writer Stdin *bufio.Reader openBrowserHook func(string) error + wrapLoopbackHandler func(http.Handler) http.Handler } helperOptionsContextKey struct{} CommandHelperOption func(*CommandHelper) @@ -119,6 +121,12 @@ func WithWorkspaceAPIKey(apiKey string) CommandHelperOption { } } +func WithNoBrowser(noBrowser bool) CommandHelperOption { + return func(h *CommandHelper) { + h.noBrowser = noBrowser + } +} + func WithOpenBrowserHook(openBrowser func(string) error) CommandHelperOption { return func(h *CommandHelper) { h.openBrowserHook = openBrowser From 156f6aa39898f32420fe879ee3c753c89bdeb4f6 Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 11 Nov 2026 19:54:23 +0100 Subject: [PATCH 2/5] fix: begin `ory auth` logins only from the link the CLI gives The start URL carries a nonce generated for each login, and /start answers 404 without it. The CLI no longer passes the start URL to the program that opens the browser. It writes a page that only the current user can read and that forwards the browser to the start URL, opens that page, and removes it when the login ends. Co-authored-by: Claude --- cmd/cloudx/client/auth.go | 69 ++++++++++++++++++++++++-- cmd/cloudx/client/auth_browser_test.go | 52 ++++++++++++++++--- 2 files changed, 112 insertions(+), 9 deletions(-) diff --git a/cmd/cloudx/client/auth.go b/cmd/cloudx/client/auth.go index e5d33ff9..bf30bd18 100644 --- a/cmd/cloudx/client/auth.go +++ b/cmd/cloudx/client/auth.go @@ -4,13 +4,18 @@ package client import ( + "bytes" "context" + "crypto/subtle" stderrors "errors" "fmt" + "html/template" "io" "net" "net/http" "net/url" + "os" + "path/filepath" "slices" "strings" "sync" @@ -181,7 +186,9 @@ func (h *CommandHelper) oAuth2DanceWithServer(ctx context.Context, client *oauth } client.RedirectURL = fmt.Sprintf("http://%s/callback", l.Addr()) + nonce := randx.MustString(32, randx.AlphaNum) login := &loopbackLogin{ + startNonce: nonce, // Cookies are not separated by port, so concurrent logins need distinct // names. cookieName: fmt.Sprintf("ory_cli_login_%d", l.Addr().(*net.TCPAddr).Port), @@ -215,15 +222,17 @@ func (h *CommandHelper) oAuth2DanceWithServer(ctx context.Context, client *oauth _ = srv.Shutdown(shutdownCtx) }() - startURL := fmt.Sprintf("http://%s/start", l.Addr()) + startURL := fmt.Sprintf("http://%s/start?nonce=%s", l.Addr(), nonce) if h.noBrowser { _, _ = fmt.Fprintln(h.VerboseErrWriter, "Open the below page in a browser on this computer to complete your login to Ory Network:") } else { - if err := h.openBrowserHook(startURL); err != nil { + removePage, err := h.openBrowserPrivately(startURL) + if err != nil { return nil, err } + defer removePage() _, _ = fmt.Fprintln(h.VerboseErrWriter, `A browser should have opened for you to complete your login to Ory Network. -If no browser opened, visit the below page to continue:`) +If it did not open or does not show the login, visit the below page to continue:`) } _, _ = fmt.Fprintf(h.VerboseErrWriter, "\n\t\t%s\n\n", startURL) @@ -235,6 +244,53 @@ If no browser opened, visit the below page to continue:`) } } +var redirectPage = template.Must(template.New("").Parse(` + + + + +Ory CLI + +Continue to log in to Ory Network + +`)) + +// openBrowserPrivately opens the URL through a page that only the current user +// can read, because every user of the computer can see the arguments of the +// program that opens the browser. The returned function removes the page. +func (h *CommandHelper) openBrowserPrivately(target string) (removePage func(), _ error) { + dir, err := os.MkdirTemp("", "ory-cli-login-") + if err != nil { + return nil, fmt.Errorf("failed to create a directory for the login page: %w", err) + } + removePage = func() { _ = os.RemoveAll(dir) } + + var page bytes.Buffer + if err := redirectPage.Execute(&page, target); err != nil { + removePage() + return nil, fmt.Errorf("failed to render the login page: %w", err) + } + path := filepath.Join(dir, "login.html") + if err := os.WriteFile(path, page.Bytes(), 0o600); err != nil { + removePage() + return nil, fmt.Errorf("failed to write the login page: %w", err) + } + if err := h.openBrowserHook(fileURL(path)); err != nil { + removePage() + return nil, err + } + return removePage, nil +} + +func fileURL(path string) string { + p := filepath.ToSlash(path) + if !strings.HasPrefix(p, "/") { + // Windows paths start with the drive letter. + p = "/" + p + } + return (&url.URL{Scheme: "file", Path: p}).String() +} + // loopbackLogin is the CLI's side of the browser login: it sends the browser // to the authorization server and receives the callback with the result. // @@ -245,6 +301,9 @@ If no browser opened, visit the below page to continue:`) // browser visits, may request the start and callback URLs too. They get // attempts of their own or are rejected, and leave the login running. type loopbackLogin struct { + // Every user of the computer can connect to the port, so only requests + // with the nonce from the start URL can begin an attempt. + startNonce string // All attempts share the cookie name, so in each browser only the latest // attempt can complete, as with the authorization server's CSRF cookie. cookieName string @@ -279,6 +338,10 @@ func (l *loopbackLogin) handler() http.Handler { } func (l *loopbackLogin) start(w http.ResponseWriter, r *http.Request) { + if subtle.ConstantTimeCompare([]byte(r.URL.Query().Get("nonce")), []byte(l.startNonce)) != 1 { + http.NotFound(w, r) + return + } state := randx.MustString(32, randx.AlphaNum) pkceVerifier := oauth2.GenerateVerifier() l.mu.Lock() diff --git a/cmd/cloudx/client/auth_browser_test.go b/cmd/cloudx/client/auth_browser_test.go index 61084425..5c57bd93 100644 --- a/cmd/cloudx/client/auth_browser_test.go +++ b/cmd/cloudx/client/auth_browser_test.go @@ -8,12 +8,16 @@ import ( "crypto/rand" "encoding/json" "fmt" + "html" "io" "net" "net/http" "net/http/cookiejar" "net/http/httptest" "net/url" + "os" + "path/filepath" + "regexp" "slices" "strings" "sync" @@ -209,10 +213,28 @@ func linkPreview(t *testing.T, jar http.CookieJar) func(string) { } } -// signInWithBrowser opens the URL the CLI gives the browser and waits for the +// readLoginPage checks that only the current user can read the page the CLI +// opens in the browser, and returns its path and the start URL it leads to. +func readLoginPage(t *testing.T, pageURL string) (path, startURL string) { + u, err := url.Parse(pageURL) + require.NoError(t, err) + path = u.Path + for p, perm := range map[string]os.FileMode{path: 0o600, filepath.Dir(path): 0o700} { + info, err := os.Stat(p) + require.NoError(t, err) + assert.Equal(t, perm, info.Mode().Perm(), p) + } + page, err := os.ReadFile(path) + require.NoError(t, err) + link := regexp.MustCompile(`href="([^"]+)"`).FindSubmatch(page) + require.NotNil(t, link, "%s", page) + return path, html.UnescapeString(string(link[1])) +} + +// signInWithBrowser opens the page the CLI gives the browser and waits for the // page the CLI sends the browser to at the end. -func signInWithBrowser(page playwright.Page, startURL string, deny bool) error { - if _, err := page.Goto(startURL); err != nil { +func signInWithBrowser(page playwright.Page, pageURL string, deny bool) error { + if _, err := page.Goto(pageURL); err != nil { return err } landing, landingText := "/cli-auth-success", "Signed in" @@ -313,6 +335,20 @@ func TestOAuth2LoginInBrowser(t *testing.T) { return setup{beforeBrowser: linkPreview(t, nil)} }, }, + { + name: "refuses to begin a login without the nonce", + setup: func(t *testing.T, _ playwright.BrowserContext) setup { + return setup{beforeBrowser: func(startURL string) { + u, err := url.Parse(startURL) + require.NoError(t, err) + u.RawQuery = "" + res, err := http.Get(u.String()) + require.NoError(t, err) + _ = res.Body.Close() + assert.Equal(t, http.StatusNotFound, res.StatusCode) + }} + }, + }, { name: "ignores a link preview with the cookie of its own login attempt", setup: func(t *testing.T, _ playwright.BrowserContext) setup { @@ -417,13 +453,16 @@ func TestOAuth2LoginInBrowser(t *testing.T) { t.Setenv(ConsoleURLKey, authorizationServer.URL) signedIn := make(chan error, 1) + var pagePath string h := &CommandHelper{ VerboseErrWriter: io.Discard, - openBrowserHook: func(uri string) error { + openBrowserHook: func(pageURL string) error { + var startURL string + pagePath, startURL = readLoginPage(t, pageURL) if s.beforeBrowser != nil { - s.beforeBrowser(uri) + s.beforeBrowser(startURL) } - go func() { signedIn <- signInWithBrowser(page, uri, tc.deny) }() + go func() { signedIn <- signInWithBrowser(page, pageURL, tc.deny) }() return nil }, wrapLoopbackHandler: s.wrapLoopbackHandler, @@ -435,6 +474,7 @@ func TestOAuth2LoginInBrowser(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) defer cancel() token, err := h.oAuth2DanceWithServer(ctx, conf) + assert.NoFileExists(t, pagePath, "the login page must be removed") redemptions, revoked := authorizationServer.redeemed() if tc.deny { From df3a5c59918e493a575b8ca301eefa046f93f55e Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 11 Nov 2026 21:08:17 +0100 Subject: [PATCH 3/5] test: skip the browser login test on Windows readLoginPage takes the page's path from its file URL as a Unix path and checks Unix file modes. CI runs the tests on Linux only. Co-authored-by: Claude --- cmd/cloudx/client/auth_browser_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/cloudx/client/auth_browser_test.go b/cmd/cloudx/client/auth_browser_test.go index 5c57bd93..0aabe23e 100644 --- a/cmd/cloudx/client/auth_browser_test.go +++ b/cmd/cloudx/client/auth_browser_test.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "regexp" + "runtime" "slices" "strings" "sync" @@ -264,6 +265,9 @@ func browserLoginCookies(browserContext playwright.BrowserContext) ([]playwright } func TestOAuth2LoginInBrowser(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("readLoginPage expects a Unix path in the file URL and Unix file modes.") + } pw, err := playwright.Run() require.NoError(t, err) t.Cleanup(func() { _ = pw.Stop() }) From b961aa7d97c07c0f93c3e6b77d01258cc971a59e Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 11 Nov 2026 23:20:37 +0100 Subject: [PATCH 4/5] test: fail the login when the CLI does not accept the callback The CLI keeps waiting after it rejects a callback, so a login that never reached the CLI ran into the 10-minute test timeout without saying why. The consent helper now waits for the CLI's answer to the callback and fails with the page or the CLI's error. The trace now ends after the callback, so a failure in CI shows it. Co-authored-by: Claude --- cmd/cloudx/testhelpers/testhelpers.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/cmd/cloudx/testhelpers/testhelpers.go b/cmd/cloudx/testhelpers/testhelpers.go index bb0cd5f6..28710695 100644 --- a/cmd/cloudx/testhelpers/testhelpers.go +++ b/cmd/cloudx/testhelpers/testhelpers.go @@ -582,8 +582,18 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai } time.Sleep(time.Second) - // accept consent - require.NoError(t, page.Locator(`button:has-text("Allow")`).Click()) + // The CLI keeps waiting after it rejects a callback, so a failed login + // has to fail here and not at the test timeout. + callback, err := page.ExpectResponse(isLoopbackCallback, func() error { + return page.Locator(`button:has-text("Allow")`).Click() + }) + if err != nil { + require.FailNowf(t, "the browser did not return to the CLI", "%s\n\nThe browser ended up at %s.", err, page.URL()) + } + if callback.Status() != http.StatusFound { + body, _ := callback.Text() + require.FailNowf(t, "the CLI rejected the callback", "%d %s", callback.Status(), body) + } t.Logf("consent successful") @@ -591,6 +601,11 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai } } +func isLoopbackCallback(rawURL string) bool { + u, err := url.Parse(rawURL) + return err == nil && u.Hostname() == "127.0.0.1" && u.Path == "/callback" +} + var ( tracesDir string // tracesPackage is the package under test, used to keep the traces of From 82d0b4f429eebd69adba087c9c8ef63cb1131bac Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Thu, 12 Nov 2026 16:01:00 +0100 Subject: [PATCH 5/5] refactor: open the login start URL in the browser directly The CLI passes the start URL to the program that opens the browser again, instead of writing a temporary page that forwards to it. The browser login test runs on Windows again. Co-authored-by: Claude --- cmd/cloudx/client/auth.go | 57 +------------------------- cmd/cloudx/client/auth_browser_test.go | 40 +++--------------- 2 files changed, 7 insertions(+), 90 deletions(-) diff --git a/cmd/cloudx/client/auth.go b/cmd/cloudx/client/auth.go index bf30bd18..afcaecdd 100644 --- a/cmd/cloudx/client/auth.go +++ b/cmd/cloudx/client/auth.go @@ -4,18 +4,14 @@ package client import ( - "bytes" "context" "crypto/subtle" stderrors "errors" "fmt" - "html/template" "io" "net" "net/http" "net/url" - "os" - "path/filepath" "slices" "strings" "sync" @@ -226,13 +222,11 @@ func (h *CommandHelper) oAuth2DanceWithServer(ctx context.Context, client *oauth if h.noBrowser { _, _ = fmt.Fprintln(h.VerboseErrWriter, "Open the below page in a browser on this computer to complete your login to Ory Network:") } else { - removePage, err := h.openBrowserPrivately(startURL) - if err != nil { + if err := h.openBrowserHook(startURL); err != nil { return nil, err } - defer removePage() _, _ = fmt.Fprintln(h.VerboseErrWriter, `A browser should have opened for you to complete your login to Ory Network. -If it did not open or does not show the login, visit the below page to continue:`) +If no browser opened, visit the below page to continue:`) } _, _ = fmt.Fprintf(h.VerboseErrWriter, "\n\t\t%s\n\n", startURL) @@ -244,53 +238,6 @@ If it did not open or does not show the login, visit the below page to continue: } } -var redirectPage = template.Must(template.New("").Parse(` - - - - -Ory CLI - -Continue to log in to Ory Network - -`)) - -// openBrowserPrivately opens the URL through a page that only the current user -// can read, because every user of the computer can see the arguments of the -// program that opens the browser. The returned function removes the page. -func (h *CommandHelper) openBrowserPrivately(target string) (removePage func(), _ error) { - dir, err := os.MkdirTemp("", "ory-cli-login-") - if err != nil { - return nil, fmt.Errorf("failed to create a directory for the login page: %w", err) - } - removePage = func() { _ = os.RemoveAll(dir) } - - var page bytes.Buffer - if err := redirectPage.Execute(&page, target); err != nil { - removePage() - return nil, fmt.Errorf("failed to render the login page: %w", err) - } - path := filepath.Join(dir, "login.html") - if err := os.WriteFile(path, page.Bytes(), 0o600); err != nil { - removePage() - return nil, fmt.Errorf("failed to write the login page: %w", err) - } - if err := h.openBrowserHook(fileURL(path)); err != nil { - removePage() - return nil, err - } - return removePage, nil -} - -func fileURL(path string) string { - p := filepath.ToSlash(path) - if !strings.HasPrefix(p, "/") { - // Windows paths start with the drive letter. - p = "/" + p - } - return (&url.URL{Scheme: "file", Path: p}).String() -} - // loopbackLogin is the CLI's side of the browser login: it sends the browser // to the authorization server and receives the callback with the result. // diff --git a/cmd/cloudx/client/auth_browser_test.go b/cmd/cloudx/client/auth_browser_test.go index 0aabe23e..0cbcbda1 100644 --- a/cmd/cloudx/client/auth_browser_test.go +++ b/cmd/cloudx/client/auth_browser_test.go @@ -8,17 +8,12 @@ import ( "crypto/rand" "encoding/json" "fmt" - "html" "io" "net" "net/http" "net/http/cookiejar" "net/http/httptest" "net/url" - "os" - "path/filepath" - "regexp" - "runtime" "slices" "strings" "sync" @@ -214,28 +209,10 @@ func linkPreview(t *testing.T, jar http.CookieJar) func(string) { } } -// readLoginPage checks that only the current user can read the page the CLI -// opens in the browser, and returns its path and the start URL it leads to. -func readLoginPage(t *testing.T, pageURL string) (path, startURL string) { - u, err := url.Parse(pageURL) - require.NoError(t, err) - path = u.Path - for p, perm := range map[string]os.FileMode{path: 0o600, filepath.Dir(path): 0o700} { - info, err := os.Stat(p) - require.NoError(t, err) - assert.Equal(t, perm, info.Mode().Perm(), p) - } - page, err := os.ReadFile(path) - require.NoError(t, err) - link := regexp.MustCompile(`href="([^"]+)"`).FindSubmatch(page) - require.NotNil(t, link, "%s", page) - return path, html.UnescapeString(string(link[1])) -} - -// signInWithBrowser opens the page the CLI gives the browser and waits for the +// signInWithBrowser opens the URL the CLI gives the browser and waits for the // page the CLI sends the browser to at the end. -func signInWithBrowser(page playwright.Page, pageURL string, deny bool) error { - if _, err := page.Goto(pageURL); err != nil { +func signInWithBrowser(page playwright.Page, startURL string, deny bool) error { + if _, err := page.Goto(startURL); err != nil { return err } landing, landingText := "/cli-auth-success", "Signed in" @@ -265,9 +242,6 @@ func browserLoginCookies(browserContext playwright.BrowserContext) ([]playwright } func TestOAuth2LoginInBrowser(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("readLoginPage expects a Unix path in the file URL and Unix file modes.") - } pw, err := playwright.Run() require.NoError(t, err) t.Cleanup(func() { _ = pw.Stop() }) @@ -457,16 +431,13 @@ func TestOAuth2LoginInBrowser(t *testing.T) { t.Setenv(ConsoleURLKey, authorizationServer.URL) signedIn := make(chan error, 1) - var pagePath string h := &CommandHelper{ VerboseErrWriter: io.Discard, - openBrowserHook: func(pageURL string) error { - var startURL string - pagePath, startURL = readLoginPage(t, pageURL) + openBrowserHook: func(startURL string) error { if s.beforeBrowser != nil { s.beforeBrowser(startURL) } - go func() { signedIn <- signInWithBrowser(page, pageURL, tc.deny) }() + go func() { signedIn <- signInWithBrowser(page, startURL, tc.deny) }() return nil }, wrapLoopbackHandler: s.wrapLoopbackHandler, @@ -478,7 +449,6 @@ func TestOAuth2LoginInBrowser(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) defer cancel() token, err := h.oAuth2DanceWithServer(ctx, conf) - assert.NoFileExists(t, pagePath, "the login page must be removed") redemptions, revoked := authorizationServer.redeemed() if tc.deny {