diff --git a/cmd/cloudx/client/api_key.go b/cmd/cloudx/client/api_key.go index 34a2e341..470523ca 100644 --- a/cmd/cloudx/client/api_key.go +++ b/cmd/cloudx/client/api_key.go @@ -10,7 +10,6 @@ import ( "time" cloud "github.com/ory/client-go" - "github.com/ory/x/cmdx" ) // CreateProjectAPIKey creates a project API key. If expiresIn is greater than @@ -88,12 +87,20 @@ func (h *CommandHelper) TemporaryAPIKey(ctx context.Context, name string, expire return *h.projectAPIKey, noop, nil } - // For all other projects, except the playground, we need to authenticate. - if err := h.Authenticate(ctx); errors.Is(err, ErrNoConfigQuiet) { - _, _ = fmt.Fprintf(h.VerboseErrWriter, "Because you are not authenticated, the Ory CLI can not configure your project automatically. You can still use the Ory Proxy / Ory Tunnel, but complex flows such as Social Sign In will not work. Remove the `--quiet` flag or run `ory auth login` to authenticate.") - return "", noop, nil - } else if errors.Is(err, ErrNotAuthenticated) { - ok, err := cmdx.AskScannerForConfirmation("To support complex flows such as Social Sign In, the Ory CLI can configure your project automatically. To do so, you need to be signed in. Do you want to sign in?", h.Stdin, h.VerboseErrWriter) + // For all other projects, except the playground, we need to authenticate. We + // may only *check* here: Authenticate performs the interactive sign-in, so + // calling it up front would sign the user in without ever asking them, and + // would make both branches below unreachable. + switch err := h.checkAuthenticated(ctx); { + case err == nil: + // Already signed in, nothing to do. + case errors.Is(err, ErrNoConfig), errors.Is(err, ErrNotAuthenticated), errors.Is(err, ErrReauthenticate): + if h.isQuiet { + _, _ = fmt.Fprintf(h.VerboseErrWriter, "Because you are not authenticated, the Ory CLI can not configure your project automatically. You can still use the Ory Proxy / Ory Tunnel, but complex flows such as Social Sign In will not work. Remove the `--quiet` flag or run `ory auth login` to authenticate.") + return "", noop, nil + } + + ok, err := h.Confirm("To support complex flows such as Social Sign In, the Ory CLI can configure your project automatically. To do so, you need to be signed in. Do you want to sign in?") if err != nil { return "", noop, err } @@ -106,7 +113,7 @@ func (h *CommandHelper) TemporaryAPIKey(ctx context.Context, name string, expire if err := h.Authenticate(ctx); err != nil { return "", noop, err } - } else if err != nil { + default: return "", noop, err } diff --git a/cmd/cloudx/client/command_helper.go b/cmd/cloudx/client/command_helper.go index a36d0264..07f6ac47 100644 --- a/cmd/cloudx/client/command_helper.go +++ b/cmd/cloudx/client/command_helper.go @@ -314,6 +314,25 @@ func (h *CommandHelper) determineProjectID(ctx context.Context, config *Config) return nil } +// Confirm asks the user to confirm message, returning true if they agree. +// +// When --yes is set the answer is yes without prompting. Every interactive +// confirmation in the cloudx commands must go through here, otherwise --yes +// silently does nothing and unattended use (CI, scripts) blocks forever on a +// prompt nobody can answer. +func (h *CommandHelper) Confirm(message string) (bool, error) { + if h.noConfirm { + return true, nil + } + if h.isQuiet { + // VerboseErrWriter is io.Discard in quiet mode, so prompting would block + // on stdin without the user ever seeing the question. Fail loudly instead + // of hanging. + return false, errors.New("can not ask for confirmation when --quiet is set, use --yes to confirm automatically") + } + return cmdx.AskScannerForConfirmation(message, h.Stdin, h.VerboseErrWriter) +} + func (h *CommandHelper) ProjectID() (string, error) { if h.projectID == uuid.Nil { return "", ErrProjectNotSet diff --git a/cmd/cloudx/client/config.go b/cmd/cloudx/client/config.go index ff8632ac..23e3a231 100644 --- a/cmd/cloudx/client/config.go +++ b/cmd/cloudx/client/config.go @@ -96,10 +96,14 @@ func (h *CommandHelper) getConfig() (*Config, error) { } _, _ = fmt.Fprintln(h.VerboseErrWriter, "Thanks for upgrading! You will now be prompted to log in to the Ory CLI through the Ory Console.") - _, _ = fmt.Fprintln(h.VerboseErrWriter, "Press enter to continue...") - _, err := h.Stdin.ReadString('\n') - if err != nil && err != io.EOF { - return nil, fmt.Errorf("unable to read from stdin: %w", err) + // --yes must skip this dialog as well, otherwise unattended use + // blocks on a prompt nobody can answer. + if !h.noConfirm { + _, _ = fmt.Fprintln(h.VerboseErrWriter, "Press enter to continue...") + _, err := h.Stdin.ReadString('\n') + if err != nil && err != io.EOF { + return nil, fmt.Errorf("unable to read from stdin: %w", err) + } } } fallthrough diff --git a/cmd/cloudx/client/confirm_test.go b/cmd/cloudx/client/confirm_test.go new file mode 100644 index 00000000..71ac5506 --- /dev/null +++ b/cmd/cloudx/client/confirm_test.go @@ -0,0 +1,130 @@ +// Copyright © 2026 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "bufio" + "bytes" + "context" + "errors" + "io" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// refusingReader fails the test if anything tries to read from it. Reading +// stdin is exactly what --yes and --quiet have to avoid: in CI there is nobody +// to answer, so the command would block forever. +type refusingReader struct{ t *testing.T } + +func (r refusingReader) Read([]byte) (int, error) { + r.t.Error("stdin must not be read without an interactive confirmation") + return 0, io.EOF +} + +// TestConfirm covers https://github.com/ory/cli/issues/219: --yes was parsed +// and stored but never read, so it did not skip any prompt. +func TestConfirm(t *testing.T) { + t.Run("case=--yes answers yes without prompting", func(t *testing.T) { + var out bytes.Buffer + h := &CommandHelper{ + noConfirm: true, + Stdin: bufio.NewReader(refusingReader{t}), + VerboseErrWriter: &out, + } + + ok, err := h.Confirm("do you want to sign in?") + require.NoError(t, err) + assert.True(t, ok) + assert.Empty(t, out.String(), "no prompt should be shown when --yes is set") + }) + + for _, tc := range []struct { + name string + input string + want bool + }{ + {name: "case=prompts and accepts yes", input: "y\n", want: true}, + {name: "case=prompts and accepts no", input: "n\n", want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + h := &CommandHelper{ + noConfirm: false, + Stdin: bufio.NewReader(strings.NewReader(tc.input)), + VerboseErrWriter: &out, + } + + ok, err := h.Confirm("do you want to sign in?") + require.NoError(t, err) + assert.Equal(t, tc.want, ok) + assert.Contains(t, out.String(), "do you want to sign in?") + }) + } + + t.Run("case=--quiet does not block on stdin", func(t *testing.T) { + h := &CommandHelper{ + isQuiet: true, + Stdin: bufio.NewReader(refusingReader{t}), + VerboseErrWriter: io.Discard, + } + + ok, err := h.Confirm("do you want to sign in?") + assert.Error(t, err) + assert.False(t, ok) + }) +} + +// TestTemporaryAPIKeyConfirmation guards the wiring of the confirmation: the +// prompt must actually be reachable. Gating on Authenticate instead of +// checkAuthenticated performs the sign-in instead of checking for it, which +// makes the prompt (and with it --yes) dead code. +func TestTemporaryAPIKeyConfirmation(t *testing.T) { + errBrowserOpened := errors.New("browser opened") + + newHelper := func(t *testing.T, stdin io.Reader, out io.Writer, opts ...CommandHelperOption) *CommandHelper { + h, err := NewCommandHelper(context.Background(), append([]CommandHelperOption{ + WithConfigLocation(filepath.Join(t.TempDir(), "config.json")), + WithStdin(stdin), + WithVerboseErrWriter(out), + WithOpenBrowserHook(func(string) error { return errBrowserOpened }), + }, opts...)...) + require.NoError(t, err) + return h + } + + t.Run("case=asks before signing in", func(t *testing.T) { + var out bytes.Buffer + h := newHelper(t, strings.NewReader("n\n"), &out) + + key, cleanup, err := h.TemporaryAPIKey(context.Background(), "test", 0) + require.NoError(t, err) + require.NoError(t, cleanup()) + assert.Empty(t, key) + assert.Contains(t, out.String(), "Do you want to sign in?") + }) + + t.Run("case=--yes skips the prompt and signs in", func(t *testing.T) { + var out bytes.Buffer + h := newHelper(t, refusingReader{t}, &out, WithNoConfirm(true)) + + _, _, err := h.TemporaryAPIKey(context.Background(), "test", 0) + assert.ErrorIs(t, err, errBrowserOpened) + }) + + t.Run("case=--quiet continues without an API key", func(t *testing.T) { + var out bytes.Buffer + h := newHelper(t, refusingReader{t}, &out, WithQuiet(true)) + + key, cleanup, err := h.TemporaryAPIKey(context.Background(), "test", 0) + require.NoError(t, err) + require.NoError(t, cleanup()) + assert.Empty(t, key) + assert.Contains(t, out.String(), "Remove the `--quiet` flag") + }) +}