fix: sign out without re-authenticating and revoke the stored token

.github/workflows/ci.yaml

@@ -41,7 +41,9 @@ jobs:
           go run github.com/mxschmitt/playwright-go/cmd/playwright install
           chromium --with-deps
       - run: make lint
-      - run: go test -race ./...
+      # cmd/cloudx tests run against live staging, which go test cannot track,
+      # so a cached pass would hide a break on the staging side.
+      - run: go test -race -count=1 ./...
         env:
           ORY_RATE_LIMIT_HEADER: ${{ secrets.ORY_RATE_LIMIT_HEADER }}
           ORY_CONSOLE_URL: https://console.staging.ory.dev

cmd/cloudx/client/auth.go

@@ -273,32 +273,69 @@ func redirectErr(w http.ResponseWriter, r *http.Request, err, desc string) {
 	http.Redirect(w, r, location.String(), http.StatusFound)
 }
 
+// SignOut revokes the stored OAuth2 token and removes it from the local
+// configuration. It reads the configuration file directly instead of going
+// through GetAuthenticatedConfig, because signing out must not require a valid
+// session: an expired or invalid one would otherwise start a new login, and
+// RFC 7009 servers accept revocation requests for tokens that are already
+// invalid.
 func (h *CommandHelper) SignOut(ctx context.Context) error {
-	config, err := h.GetAuthenticatedConfig(ctx)
-	if err != nil {
+	config, err := readConfig(h.configLocation)
+	if stderrors.Is(err, ErrNoConfig) {
+		return nil
+	} else if err != nil {
 		return err
 	}
-	if config.AccessToken == nil {
-		return h.ClearConfig()
+	if config.AccessToken != nil {
+		if err := revokeToken(ctx, config.AccessToken); err != nil {
+			_, _ = fmt.Fprintf(h.VerboseErrWriter, "failed to revoke access token, signing out locally only: %v\n", err)
+		}
 	}
+	return h.ClearConfig()
+}
+
+// revokeToken revokes the refresh token, which also revokes the access tokens
+// issued with it. Tokens obtained without the offline_access scope have no
+// refresh token, so the access token is revoked instead.
+func revokeToken(ctx context.Context, token *oauth2.Token) error {
+	value, hint := token.RefreshToken, "refresh_token"
+	if value == "" {
+		value, hint = token.AccessToken, "access_token"
+	}
+	if value == "" {
+		return nil
+	}
+
 	client := oauth2ClientConfig()
 	revoke, err := url.Parse(client.Endpoint.AuthURL)
 	if err != nil {
 		return err
 	}
 	revoke.Path = "/oauth2/revoke"
-	res, err := http.PostForm(revoke.String(), url.Values{
-		"client_id": []string{client.ClientID},
-		"token":     []string{config.AccessToken.RefreshToken}, // this also revokes the associated access token
-	})
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, revoke.String(), strings.NewReader(url.Values{
+		"client_id":       {client.ClientID},
+		"token":           {value},
+		"token_type_hint": {hint},
+	}.Encode()))
 	if err != nil {
-		_, _ = fmt.Fprintf(h.VerboseErrWriter, "failed to revoke access token: %v\n", err)
-	} else {
-		defer res.Body.Close()
-		if res.StatusCode < 200 || res.StatusCode > 299 {
-			body, _ := io.ReadAll(res.Body)
-			_, _ = fmt.Fprintf(h.VerboseErrWriter, "failed to revoke access token: %v\n", string(body))
-		}
+		return err
 	}
-	return h.ClearConfig()
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	// Honor the same context HTTP client that the oauth2 package uses for the
+	// token exchange and refresh requests against this server.
+	httpClient := http.DefaultClient
+	if c, ok := ctx.Value(oauth2.HTTPClient).(*http.Client); ok && c != nil {
+		httpClient = c
+	}
+	res, err := httpClient.Do(req)
+	if err != nil {
+		return err
+	}
+	defer res.Body.Close()
+	if res.StatusCode < 200 || res.StatusCode > 299 {
+		body, _ := io.ReadAll(io.LimitReader(res.Body, 4096))
+		return fmt.Errorf("unexpected status %d: %s", res.StatusCode, body)
+	}
+	return nil
 }

cmd/cloudx/client/auth_test.go

@@ -0,0 +1,148 @@
+// Copyright © 2026 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package client_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/url"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/oauth2"
+
+	"github.com/ory/cli/cmd/cloudx/client"
+	"github.com/ory/cli/cmd/cloudx/testhelpers"
+)
+
+type roundTripFunc func(*http.Request) (*http.Response, error)
+
+func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
+
+// revokeRecorder answers OAuth2 token revocation requests with the given
+// status and fails the test on any other request, such as a token refresh.
+func revokeRecorder(t *testing.T, status int) (*http.Client, *[]url.Values) {
+	var revoked []url.Values
+	return &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
+		res := &http.Response{
+			StatusCode: status,
+			Header:     http.Header{},
+			Body:       io.NopCloser(strings.NewReader("")),
+			Request:    r,
+		}
+		if r.Method != http.MethodPost || r.URL.Path != "/oauth2/revoke" {
+			t.Errorf("unexpected request: %s %s", r.Method, r.URL)
+			res.StatusCode = http.StatusBadRequest
+			return res, nil
+		}
+		if assert.NoError(t, r.ParseForm()) {
+			revoked = append(revoked, r.PostForm)
+		}
+		return res, nil
+	})}, &revoked
+}
+
+func TestSignOut(t *testing.T) {
+	expired := time.Now().Add(-time.Hour)
+
+	writeConfig := func(t *testing.T, config any) string {
+		location := testhelpers.NewConfigFile(t)
+		raw, err := json.Marshal(config)
+		require.NoError(t, err)
+		require.NoError(t, os.WriteFile(location, raw, 0600))
+		return location
+	}
+
+	signOut := func(t *testing.T, location string, status int, opts ...client.CommandHelperOption) (revoked []url.Values, stderr string, err error) {
+		httpClient, rec := revokeRecorder(t, status)
+		ctx := context.WithValue(context.Background(), oauth2.HTTPClient, httpClient)
+		out := bytes.Buffer{}
+		h, err := client.NewCommandHelper(ctx, append([]client.CommandHelperOption{
+			client.WithConfigLocation(location),
+			client.WithVerboseErrWriter(&out),
+			client.WithOpenBrowserHook(func(string) error { return testhelpers.ErrAuthFlowTriggered }),
+		}, opts...)...)
+		require.NoError(t, err)
+		err = h.SignOut(ctx)
+		return *rec, out.String(), err
+	}
+
+	t.Run("revokes the refresh token of an expired session without signing in", func(t *testing.T) {
+		location := writeConfig(t, client.Config{
+			Version:     client.ConfigVersion,
+			AccessToken: &oauth2.Token{AccessToken: "access", RefreshToken: "refresh", Expiry: expired},
+		})
+
+		revoked, _, err := signOut(t, location, http.StatusOK)
+		require.NoError(t, err)
+
+		assert.Equal(t, []url.Values{{
+			"client_id":       {"ory-cli"},
+			"token":           {"refresh"},
+			"token_type_hint": {"refresh_token"},
+		}}, revoked)
+		assert.Nil(t, testhelpers.ReadConfig(t, location).AccessToken)
+	})
+
+	t.Run("revokes the access token if there is no refresh token", func(t *testing.T) {
+		location := writeConfig(t, client.Config{
+			Version:     client.ConfigVersion,
+			AccessToken: &oauth2.Token{AccessToken: "access", Expiry: expired},
+		})
+
+		revoked, _, err := signOut(t, location, http.StatusOK)
+		require.NoError(t, err)
+
+		assert.Equal(t, []url.Values{{
+			"client_id":       {"ory-cli"},
+			"token":           {"access"},
+			"token_type_hint": {"access_token"},
+		}}, revoked)
+		assert.Nil(t, testhelpers.ReadConfig(t, location).AccessToken)
+	})
+
+	t.Run("signs out locally if revocation fails", func(t *testing.T) {
+		location := writeConfig(t, client.Config{
+			Version:     client.ConfigVersion,
+			AccessToken: &oauth2.Token{AccessToken: "access", RefreshToken: "refresh", Expiry: time.Now().Add(time.Hour)},
+		})
+
+		revoked, stderr, err := signOut(t, location, http.StatusServiceUnavailable)
+		require.NoError(t, err)
+
+		assert.Len(t, revoked, 1)
+		assert.Contains(t, stderr, "failed to revoke")
+		assert.Nil(t, testhelpers.ReadConfig(t, location).AccessToken)
+	})
+
+	t.Run("does nothing without a config file", func(t *testing.T) {
+		location := testhelpers.NewConfigFile(t)
+
+		revoked, _, err := signOut(t, location, http.StatusOK)
+		require.NoError(t, err)
+
+		assert.Empty(t, revoked)
+		assert.NoFileExists(t, location)
+	})
+
+	t.Run("clears a legacy config", func(t *testing.T) {
+		location := writeConfig(t, map[string]string{"version": "v0alpha0", "session_token": "legacy"})
+
+		revoked, _, err := signOut(t, location, http.StatusOK, client.WithNoConfirm(true))
+		require.NoError(t, err)
+
+		assert.Empty(t, revoked)
+		raw, err := os.ReadFile(location)
+		require.NoError(t, err)
+		assert.NotContains(t, string(raw), "legacy")
+		assert.Equal(t, client.ConfigVersion, testhelpers.ReadConfig(t, location).Version)
+	})
+}

cmd/cloudx/testhelpers/testhelpers.go

@@ -468,38 +468,74 @@ func redactInZip(path, secret string) error {
 	return os.Rename(tmp.Name(), path)
 }
 
-// submitPasswordForm submits the filled-in login form and fails immediately if
-// Ory Network did not accept it.
+// submitLoginStep submits the login form with the given method and fails
+// immediately if Ory Network did not accept it.
 //
-// It exists because the consent screen is the next thing the caller waits for.
-// On a refusal the page never leaves the form, so an unchecked failure surfaces
-// 30 seconds later as a missing `Allow` button, pointing at the consent screen
-// instead of at the reason it never rendered: rejected credentials (400), 429
-// when a CI run drives more logins from one IP than Ory Network allows, or a 5xx.
-func submitPasswordForm(t testing.TB, page playwright.Page) {
+// It exists because every step is followed by a wait for the next screen. On a
+// refusal the page never leaves the form, so an unchecked failure surfaces 30
+// seconds later as a missing field or `Allow` button, pointing at that screen
+// instead of at the reason it never rendered: rejected credentials, 429 when a
+// CI run drives more logins from one IP than Ory Network allows, or a 5xx.
+func submitLoginStep(t testing.TB, page playwright.Page, method string) {
 	// The login UI submits the flow with fetch, so this POST is the only request
 	// to this path; the flow itself was created before the page rendered.
 	isLoginSubmission := func(r playwright.Response) bool {
 		return r.Request().Method() == http.MethodPost && strings.Contains(r.URL(), "/self-service/login")
 	}
 
 	resp, err := page.ExpectResponse(isLoginSubmission, func() error {
-		return page.Locator(`[type="submit"][name="method"][value="password"]`).Click()
+		return page.Locator(fmt.Sprintf(`[type="submit"][name="method"][value=%q]`, method)).Click()
 	})
-	require.NoError(t, err, "the login form was never submitted")
-
-	// A login that continues at the OAuth2 consent screen is answered with 422
-	// browser_location_change_required, the only use of 422 in self-service
-	// flows. The login UI navigates away on it, which can discard the body
-	// before it is read, so the status alone has to decide.
-	if resp.Status() < http.StatusBadRequest || resp.Status() == http.StatusUnprocessableEntity {
+	require.NoError(t, err, "the %s login step was never submitted", method)
+
+	switch resp.Status() {
+	case http.StatusUnprocessableEntity:
+		// A login that continues at the OAuth2 consent screen is answered with
+		// 422 browser_location_change_required, the only use of 422 in
+		// self-service flows. The login UI navigates away on it, which can
+		// discard the body before it is read, so the status alone has to decide.
 		return
+	case http.StatusBadRequest:
+		// Kratos answers 400 both when it refuses a step and when the flow
+		// continues at its next step, as after the identifier of an
+		// identifier-first login. The UI renders that step in place, so the
+		// body is still there to tell the two apart.
+		if body, err := resp.Body(); err == nil && isNextLoginStep(body) {
+			return
+		}
+	default:
+		if resp.Status() < http.StatusBadRequest {
+			return
+		}
 	}
 
 	body, _ := resp.Text()
 	require.FailNowf(t, "the login form was refused",
-		"POST %s\n%d %s\n%s\n\nThe consent screen never renders after this, so waiting for it would only time out.",
-		resp.URL(), resp.Status(), resp.StatusText(), body)
+		"POST %s (method %s)\n%d %s\n%s\n\nThe next login screen never renders after this, so waiting for it would only time out.",
+		resp.URL(), method, resp.Status(), resp.StatusText(), body)
+}
+
+// isNextLoginStep reports whether a 400 answer to a login submission is the
+// flow asking for its next step. Kratos answers a refusal with the flow too,
+// but adds messages of type error to it; other errors are not a flow at all.
+func isNextLoginStep(body []byte) bool {
+	var flow cloud.LoginFlow
+	if err := json.Unmarshal(body, &flow); err != nil {
+		return false
+	}
+	for _, m := range flow.Ui.Messages {
+		if m.Type == "error" {
+			return false
+		}
+	}
+	for _, n := range flow.Ui.Nodes {
+		for _, m := range n.Messages {
+			if m.Type == "error" {
+				return false
+			}
+		}
+	}
+	return true
 }
 
 func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, email, password string) func(uri string) error {
@@ -521,19 +557,23 @@ func PlaywrightAcceptConsentBrowserHook(t testing.TB, page playwright.Page, emai
 		_, err := page.Goto(uri)
 		require.NoError(t, err)
 
-		// The password field renders on a first login and when reconfirming an
-		// existing session, but only the first login asks for the identifier.
-		passwordInput := page.Locator(`[data-testid="ory/form/node/input/password"]`)
-		require.NoError(t, passwordInput.WaitFor())
-		identifierInput := page.Locator(`[data-testid="ory/form/node/input/identifier"]`)
+		// Console login is identifier-first: a first login asks for the email on
+		// its own screen and only then for the password, while reconfirming an
+		// existing session starts at the password. The email stays in the form
+		// as a hidden field after its step, so only visible fields count.
+		identifierInput := page.Locator(`[data-testid="ory/form/node/input/identifier"]:visible`)
+		passwordInput := page.Locator(`[data-testid="ory/form/node/input/password"]:visible`)
+		require.NoError(t, identifierInput.Or(passwordInput).First().WaitFor())
 		if visible, err := identifierInput.IsVisible(); err == nil && visible {
 			t.Logf("logging in")
 			require.NoError(t, identifierInput.Fill(email))
+			submitLoginStep(t, page, "identifier_first")
+			require.NoError(t, passwordInput.WaitFor())
 		} else {
 			t.Logf("reconfirming password")
 		}
 		require.NoError(t, passwordInput.Fill(password))
-		submitPasswordForm(t, page)
+		submitLoginStep(t, page, "password")
 
 		// we wait here for the button +1s because there is some console bug that can lead to form submissions before the form action is correctly set
 		if err := page.Locator(`button:has-text("Allow")`).WaitFor(); err != nil {