Skip to content

Commit f4039ae

Browse files
jordanstephensclaudelionellocoderabbitai[bot]
authored
fix(aws): use survey for AWS login code input (#2070)
* use survey * test(aws): extract collectAuthCode and add unit tests Extract the browser-open + prompt retry loop from CrossDeviceLogin into a testable collectAuthCode helper with injectable askFn and openBrowser params, then cover all branches with table-driven unit tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * delete now-unused OpenBrowserWithInputOnEnter * Apply suggestion from @coderabbitai[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Lio李歐 <lionello@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 7b9006b commit f4039ae

3 files changed

Lines changed: 145 additions & 57 deletions

File tree

src/pkg/clouds/aws/login.go

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"strings"
2020
"time"
2121

22+
"github.com/AlecAivazis/survey/v2"
2223
"github.com/DefangLabs/defang/src/pkg/auth"
2324
"github.com/DefangLabs/defang/src/pkg/term"
2425
"github.com/DefangLabs/defang/src/pkg/tokenstore"
@@ -30,6 +31,7 @@ import (
3031
awssts "github.com/aws/aws-sdk-go-v2/service/sts"
3132
"github.com/golang-jwt/jwt/v5"
3233
"github.com/google/uuid"
34+
"github.com/pkg/browser"
3335
)
3436

3537
const (
@@ -353,6 +355,29 @@ func (a *Aws) InteractiveLogin(ctx context.Context) (*awsTokenCache, error) {
353355
return RetrieveToken(ctx, tokenURL, clientIDSameDevice, code, pkce.Verifier, redirectURI)
354356
}
355357

358+
// collectAuthCode prompts the user for an authorization code, opening the browser
359+
// on the first empty submission. askFn is called repeatedly until a non-empty
360+
// trimmed value is returned; openBrowser is called at most once.
361+
func collectAuthCode(askFn func() (string, error), openBrowser func(string) error, authURL string) (string, error) {
362+
browserOpened := false
363+
for {
364+
raw, err := askFn()
365+
if err != nil {
366+
return "", fmt.Errorf("reading authorization code: %w", err)
367+
}
368+
code := strings.TrimSpace(raw)
369+
if code != "" {
370+
return code, nil
371+
}
372+
if !browserOpened {
373+
if err := openBrowser(authURL); err != nil {
374+
term.Warn("failed to open browser automatically, please open the URL above manually")
375+
}
376+
browserOpened = true
377+
}
378+
}
379+
}
380+
356381
// CrossDeviceLogin runs the cross-device flow for remote/SSH sessions where the
357382
// browser runs on a different machine. It prints the auth URL and prompts the user
358383
// to paste the base64-encoded verification code displayed in their browser.
@@ -368,18 +393,21 @@ func (a *Aws) CrossDeviceLogin(ctx context.Context) (*awsTokenCache, error) {
368393
term.Println("Please visit the following URL to log in to AWS: (Right click the URL or press ENTER to open browser)")
369394
term.Printf(" %s\n", authURL)
370395
term.Print("Enter the authorization code displayed in your browser: ")
371-
ctx, inputCh, done := term.OpenBrowserWithInputOnEnter(ctx, authURL)
372-
defer done()
373-
374-
var input string
375-
select {
376-
case input = <-inputCh:
377-
input = strings.TrimSpace(input)
378-
case <-ctx.Done():
379-
return nil, ctx.Err()
396+
askFn := func() (string, error) {
397+
var code string
398+
err := survey.AskOne(
399+
&survey.Input{Message: "Code"},
400+
&code,
401+
survey.WithStdio(term.DefaultTerm.Stdio()),
402+
)
403+
return code, err
404+
}
405+
code, err := collectAuthCode(askFn, browser.OpenURL, authURL)
406+
if err != nil {
407+
return nil, err
380408
}
381409

382-
authCode, gotState, err := parseVerificationCode(input)
410+
authCode, gotState, err := parseVerificationCode(code)
383411
if err != nil {
384412
return nil, err
385413
}

src/pkg/clouds/aws/login_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,113 @@ func TestRefreshTokenSuccess(t *testing.T) {
366366
}
367367
}
368368

369+
func TestCollectAuthCode(t *testing.T) {
370+
noopBrowser := func(string) error { return nil }
371+
372+
t.Run("non-empty first response returns immediately without opening browser", func(t *testing.T) {
373+
browserOpened := false
374+
code, err := collectAuthCode(
375+
func() (string, error) { return "mycode", nil },
376+
func(string) error { browserOpened = true; return nil },
377+
"https://example.com",
378+
)
379+
if err != nil {
380+
t.Fatalf("unexpected error: %v", err)
381+
}
382+
if code != "mycode" {
383+
t.Errorf("code = %q, want %q", code, "mycode")
384+
}
385+
if browserOpened {
386+
t.Error("browser should not have been opened")
387+
}
388+
})
389+
390+
t.Run("whitespace-only response is treated as empty", func(t *testing.T) {
391+
calls := 0
392+
code, err := collectAuthCode(
393+
func() (string, error) {
394+
calls++
395+
if calls == 1 {
396+
return " ", nil
397+
}
398+
return "realcode", nil
399+
},
400+
noopBrowser,
401+
"https://example.com",
402+
)
403+
if err != nil {
404+
t.Fatalf("unexpected error: %v", err)
405+
}
406+
if code != "realcode" {
407+
t.Errorf("code = %q, want %q", code, "realcode")
408+
}
409+
})
410+
411+
t.Run("empty response opens browser once then retries", func(t *testing.T) {
412+
browserCalls := 0
413+
calls := 0
414+
_, err := collectAuthCode(
415+
func() (string, error) {
416+
calls++
417+
if calls < 3 {
418+
return "", nil
419+
}
420+
return "code", nil
421+
},
422+
func(string) error { browserCalls++; return nil },
423+
"https://example.com",
424+
)
425+
if err != nil {
426+
t.Fatalf("unexpected error: %v", err)
427+
}
428+
if browserCalls != 1 {
429+
t.Errorf("browser opened %d times, want 1", browserCalls)
430+
}
431+
})
432+
433+
t.Run("browser open error does not abort the loop", func(t *testing.T) {
434+
calls := 0
435+
browserCalls := 0
436+
code, err := collectAuthCode(
437+
func() (string, error) {
438+
calls++
439+
if calls == 1 {
440+
return "", nil
441+
}
442+
return "code", nil
443+
},
444+
func(string) error {
445+
browserCalls++
446+
return errors.New("no browser")
447+
},
448+
"https://example.com",
449+
)
450+
if err != nil {
451+
t.Fatalf("unexpected error: %v", err)
452+
}
453+
if code != "code" {
454+
t.Errorf("code = %q, want %q", code, "code")
455+
}
456+
if browserCalls != 1 {
457+
t.Errorf("browser opened %d times, want 1", browserCalls)
458+
}
459+
})
460+
461+
t.Run("askFn error is returned", func(t *testing.T) {
462+
_, err := collectAuthCode(
463+
func() (string, error) { return "", errors.New("interrupted") },
464+
noopBrowser,
465+
"https://example.com",
466+
)
467+
if err == nil {
468+
t.Fatal("expected error from askFn")
469+
}
470+
if !strings.Contains(err.Error(), "interrupted") {
471+
t.Errorf("error = %q, want it to contain %q", err.Error(), "interrupted")
472+
}
473+
})
474+
}
475+
369476
func TestSameRole(t *testing.T) {
370477
tests := []struct {
371478
name string

src/pkg/term/browser.go

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package term
22

33
import (
4-
"bytes"
54
"context"
65
"io"
76

@@ -46,49 +45,3 @@ func OpenBrowserOnEnter(ctx context.Context, url string) (context.Context, func(
4645
}()
4746
return ctx, cancel
4847
}
49-
50-
func OpenBrowserWithInputOnEnter(ctx context.Context, url string) (context.Context, <-chan string, func()) {
51-
ctx, cancel := context.WithCancel(ctx)
52-
input := NewNonBlockingStdin()
53-
inputChan := make(chan string, 1) // Buffered channel to avoid blocking goroutine
54-
55-
// Handles context cancellation to ensure input is closed and goroutine exits when context is cancelled.
56-
// In linux Ctrl-C is handled by the signal handler, and input will not get a byte
57-
go func() {
58-
<-ctx.Done()
59-
input.Close()
60-
}()
61-
62-
go func() {
63-
var b [1]byte
64-
var buf bytes.Buffer
65-
inputloop:
66-
for {
67-
if _, err := input.Read(b[:]); err != nil {
68-
break
69-
}
70-
switch b[0] {
71-
case 3: // Ctrl-C
72-
cancel()
73-
break inputloop
74-
case 10, 13: // Enter or Return
75-
// If the user has already entered some input, we assume browser is already opened
76-
// and we don't want to open the browser again.
77-
if buf.Len() > 0 {
78-
break inputloop
79-
}
80-
err := browser.OpenURL(url)
81-
if err != nil {
82-
Errorf("failed to open browser: %v", err)
83-
}
84-
default:
85-
buf.WriteByte(b[0]) //nolint:gosec // G602 false positive: b is [1]byte, index 0 is always valid
86-
}
87-
}
88-
if buf.Len() > 0 {
89-
inputChan <- buf.String()
90-
}
91-
close(inputChan)
92-
}()
93-
return ctx, inputChan, cancel
94-
}

0 commit comments

Comments
 (0)