Skip to content

Add auth signup using login fallback flow#1310

Merged
Fab10-CircleCi merged 12 commits into
nextfrom
fabioramirez/signup-auth-login-fallback
May 15, 2026
Merged

Add auth signup using login fallback flow#1310
Fab10-CircleCi merged 12 commits into
nextfrom
fabioramirez/signup-auth-login-fallback

Conversation

@Fab10-CircleCi
Copy link
Copy Markdown

@Fab10-CircleCi Fab10-CircleCi commented May 14, 2026

Summary

This PR adds circleci auth signup to the next CLI auth flow.

The command starts a signup OAuth flow, opens the signup URL in the browser, then asks the user to return to the terminal after verifying their email. When the user presses Enter, the CLI first checks whether the original signup OAuth callback has already completed. If it has, the CLI exchanges that code and saves the token. If not, it falls back to the existing auth login browser flow.

This supports the temporary product/backend constraint where signup may not always complete the CLI auth handoff through the loopback callback.

Decision Context

  • Terminals can render printed URLs as clickable links, but the CLI cannot detect whether the user clicked one.
  • Because of that, the command opens the signup URL immediately instead of asking the user to press Enter just to open it.
  • The signup URL still includes the CLI loopback callback, so if signup does redirect back successfully, the CLI consumes that callback and avoids a second login flow.
  • Reused the existing login token persistence path for successful signup callback completion.
  • Added a Bubble Tea continue prompt for the signup verification step.
  • Removed the old PromptLine helper.
  • Added acceptance coverage for:
    • non-interactive signup URL output
    • already-authenticated guard
    • Ctrl+C cancellation at the verification prompt
    • fallback to regular login when no signup callback is available
    • direct completion when the signup callback is available

Testing

```sh
go test ./internal/httpcl ./internal/oauth ./internal/cmd/cmdauth
go test ./acceptance -run 'TestAuthSignup' -count=1
go test ./acceptance -count=1
```

🤖 Generated with Claude Code

Klaney and others added 9 commits May 12, 2026 12:15
Reuses the existing PKCE OAuth flow but appends signup=true to the
authorize URL so unauthenticated users land on the signup page instead
of the login page. PKCE handshake, loopback callback, and token exchange
are otherwise identical to 'auth login'.
- Drop the --insecure-storage example from `auth signup --help`; add a
  --no-browser flag (mirroring `auth login`) and surface it in examples.
- Remove two acceptance tests duplicating coverage already provided by
  TestUsage's golden help fixtures (auth.txt, auth/signup.txt).
- Convert the AlreadyAuthenticated stderr assertions to a golden file
  comparison so the full error layout is locked down.
- Replace `assert.Assert(t, strings.Contains(...))` with
  `assert.Check(t, cmp.Contains(...))` in the remaining substring checks.
Scrubs the random authorize URL to <authorize-url> and compares the
surrounding stderr (URL banner, disabled-spinner line, structured timeout
error + suggestions) against a golden file. Retains the targeted
substring asserts for signup=true and the loopback redirect_uri shape so
the URL semantics stay covered.
The module rename in next (circleci-cli-v2 → circleci-cli) merged
cleanly elsewhere but couldn't rewrite the import paths in the two
files unique to this branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous prompt anchored on "verifying your email" and "continue
with login", which was ambiguous: the user has just done signup, and
what we resume on Enter is the CLI authentication step. The new
wording reflects the actual signal (signed in on the web) and the
actual outcome (CLI authentication).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous signature (t *testing.T, ctx context.Context, authURL string)
tripped revive's context-as-argument rule (ctx must be first) which also
conflicts with Go's t-first test convention. Deriving the context from
t.Context() inside the helper sidesteps the ordering question entirely
and drops the now-unused ctx local in the calling test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Fab10-CircleCi Fab10-CircleCi force-pushed the fabioramirez/signup-auth-login-fallback branch from b1d2f11 to b235a33 Compare May 14, 2026 19:56
@CircleCI-Public CircleCI-Public deleted a comment from Chewypewy May 14, 2026
@Fab10-CircleCi Fab10-CircleCi changed the title Fix auth signup fallback login handoff and Ctrl+C cancellation Add auth signup fallback flow May 14, 2026
@Fab10-CircleCi Fab10-CircleCi changed the title Add auth signup fallback flow Add auth signup usign login fallback flow May 14, 2026
Comment thread acceptance/auth_signup_test.go Outdated
}))
}

func runSignupAndInterrupt(t *testing.T, env *testenv.TestEnv) (int, string) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this function is duplicated from the binary package.

I'd also like to restrict construction of the console to that package.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I’ll remove the signup-specific runSignupAndInterrupt helper and the dedicated acceptance test rather than duplicating console construction outside internal/testing/binary.
Now that the prompt is Bubble Tea-based, I don’t think this PR needs a add interrupt signup test.

Comment thread internal/cmd/cmdauth/login.go Outdated
defer func() { _ = flow.Close() }()

iostream.ErrPrintf(ctx, "Open this URL in your browser to continue:\n\n %s\n\n", flow.AuthorizeURL)
if openBrowser {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the non interactive path?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the naming/comment are now misleading. The normal auth login non-interactive path still passes openBrowser=false. Signup fallback passes openBrowser=true because it needs to launch the known-good browser login flow without running the full login TUI. I’ll rename or document this helper so it is clearly “browser OAuth without the full login TUI,” not strictly “non-interactive.”
- runLoginNoBrowser(...)
- runLoginBrowserFallback(...)
Or move the fallback into a Bubble Tea signup model so runLoginBrowser(..., openBrowser) is less awkward. 🤔

host := cfg.EffectiveHost()
deviceID := config.EnsureDeviceID(ctx, configPath)

flow, err := oauth.StartSignup(ctx, host, deviceID, runtime.GOOS)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this feels like it should be a single bubbletea program

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. The current implementation uses Bubble Tea only , which fixed Ctrl+C but leaves signup orchestration out. I can refactor interactive signup into SignupFlowModel., browser open, Enter-to-continue state, signup callback check, and fallback-to-login decision.

Fab10-CircleCi and others added 3 commits May 14, 2026 15:18
Address review feedback from @pete-woods on PR #1310:

- Replace the procedural runSignup flow with a SignupFlowModel in
  internal/ui/signup_flow.go that owns OAuth start, URL rendering,
  browser open, Enter-to-continue gate, signup-callback probe, login
  fallback, and token exchange end-to-end.
- Drop the openBrowser parameter from runLoginBrowser. With signup
  owning its own flow, the helper returns to its original purpose:
  the non-interactive / --no-browser login path that prints the URL
  and waits for the callback.
- Remove iostream.PromptContinue and internal/ui/continue.go —
  Ctrl+C/Esc handling now lives in the shared bubbletea UI layer via
  tea.WithContext, not in a signup-specific primitive.
- Delete the signup-only runSignupAndInterrupt helper and the
  TestAuthSignup_CtrlCAtVerificationPrompt acceptance test that
  duplicated PTY/console setup from internal/testing/binary.
- Tighten URL extraction regex to allow trailing ANSI control bytes
  that bubbletea may emit.

Surviving acceptance coverage: non-interactive output, already-
authenticated guard, fallback to login when no signup callback
arrives, direct completion when signup callback is available.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add doc comments to the exported SignupFailure* constants in
  internal/ui/signup_flow.go.
- Add an explicit SignupFailureNone case to the exhaustive switch
  on the failure code in runSignupInteractive.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The host field is initialized once in the struct literal because signup
has no host-picker stage — unlike LoginFlowModel which assigns
m.result.Host dynamically as the user navigates host selection. A
two-line comment prevents a reader from grepping for m.result.Host=
and concluding the field was missed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fab10-CircleCi added a commit that referenced this pull request May 15, 2026
Brings the `circleci auth signup` browser-handshake flow into the init
branch so the four init phases (scan / test / config / signup) ship in
one PR. Originally tracked in PR #1310.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Fab10-CircleCi Fab10-CircleCi changed the title Add auth signup usign login fallback flow Add auth signup using login fallback flow May 15, 2026
@Fab10-CircleCi Fab10-CircleCi merged commit 8381e5b into next May 15, 2026
1 check passed
@Fab10-CircleCi Fab10-CircleCi deleted the fabioramirez/signup-auth-login-fallback branch May 15, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants