diff --git a/internal/auth/login.go b/internal/auth/login.go index 68aacf2c..41be3193 100644 --- a/internal/auth/login.go +++ b/internal/auth/login.go @@ -5,6 +5,8 @@ package auth import ( "context" "fmt" + "net/url" + "strings" "github.com/localstack/lstk/internal/api" "github.com/localstack/lstk/internal/output" @@ -35,14 +37,16 @@ func (l *loginProvider) Login(ctx context.Context) (string, error) { return "", fmt.Errorf("failed to create auth request: %w", err) } - authURL := fmt.Sprintf("%s/auth/request/%s", l.webAppURL, authReq.ID) + authURL := buildAuthURL(l.webAppURL, authReq.ID, authReq.Code) output.EmitAuth(l.sink, output.AuthEvent{ Preamble: "Welcome to lstk, a command-line interface for LocalStack", Code: authReq.Code, URL: authURL, }) - _ = browser.OpenURL(authURL) + if err := browser.OpenURL(authURL); err != nil { + output.EmitWarning(l.sink, fmt.Sprintf("Failed to open browser automatically. Open this URL manually to continue: %s", authURL)) + } output.EmitSpinnerStart(l.sink, "Waiting for authorization...") @@ -66,6 +70,16 @@ func (l *loginProvider) Login(ctx context.Context) (string, error) { } } +func buildAuthURL(webAppURL, authRequestID, code string) string { + authURL := fmt.Sprintf("%s/auth/request/%s", strings.TrimRight(webAppURL, "/"), authRequestID) + if code == "" { + return authURL + } + + values := url.Values{} + values.Set("code", code) + return authURL + "?" + values.Encode() +} func (l *loginProvider) completeAuth(ctx context.Context, authReq *api.AuthRequest) (string, error) { output.EmitInfo(l.sink, "Checking if auth request is confirmed...") diff --git a/internal/auth/login_test.go b/internal/auth/login_test.go new file mode 100644 index 00000000..9fe9dc81 --- /dev/null +++ b/internal/auth/login_test.go @@ -0,0 +1,62 @@ +package auth + +import ( + "strings" + "testing" +) + +func TestBuildAuthURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + webAppURL string + requestID string + code string + wantAuthURL string + }{ + { + name: "adds code query param", + webAppURL: "http://app.localstack.cloud", + requestID: "d78cc482-1db6-4d4d-9f9c-3512963fdf93", + code: "1234", + wantAuthURL: "http://app.localstack.cloud/auth/request/d78cc482-1db6-4d4d-9f9c-3512963fdf93?code=1234", + }, + { + name: "escapes query param", + webAppURL: "https://example.com", + requestID: "req-id", + code: "A B+C", + wantAuthURL: "https://example.com/auth/request/req-id?code=A+B%2BC", + }, + { + name: "omits empty code", + webAppURL: "https://example.com", + requestID: "req-id", + code: "", + wantAuthURL: "https://example.com/auth/request/req-id", + }, + { + name: "trims trailing slash from web app URL", + webAppURL: "https://example.com/", + requestID: "req-id", + code: "1234", + wantAuthURL: "https://example.com/auth/request/req-id?code=1234", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := buildAuthURL(tt.webAppURL, tt.requestID, tt.code) + if got != tt.wantAuthURL { + t.Fatalf("expected auth URL %q, got %q", tt.wantAuthURL, got) + } + if strings.Contains(got, "//auth/request") { + t.Fatalf("expected auth URL without double slash before auth/request, got %q", got) + } + }) + } +} diff --git a/internal/output/events_test.go b/internal/output/events_test.go index 25785dcf..54d641fb 100644 --- a/internal/output/events_test.go +++ b/internal/output/events_test.go @@ -16,7 +16,6 @@ func TestEmitAuth(t *testing.T) { sink := &captureSink{} EmitAuth(sink, AuthEvent{ Preamble: "Welcome", - Code: "ABC123", URL: "https://example.com", }) @@ -27,9 +26,6 @@ func TestEmitAuth(t *testing.T) { if !ok { t.Fatalf("expected AuthEvent, got %T", sink.events[0]) } - if event.Code != "ABC123" { - t.Fatalf("expected code %q, got %q", "ABC123", event.Code) - } if event.URL != "https://example.com" { t.Fatalf("expected URL %q, got %q", "https://example.com", event.URL) } @@ -41,7 +37,45 @@ func TestEmitAuth(t *testing.T) { if !ok { t.Fatal("expected formatter output") } - if line != "Welcome\nYour one-time code: ABC123\nOpening browser to login...\nhttps://example.com" { + expected := "Welcome\nOpening browser to login...\nBrowser didn't open? Visit https://example.com" + if line != expected { + t.Fatalf("unexpected formatted line: %q", line) + } +} + +func TestEmitAuthWithCode(t *testing.T) { + t.Parallel() + + sink := &captureSink{} + EmitAuth(sink, AuthEvent{ + Preamble: "Welcome", + URL: "https://example.com", + Code: "1234", + }) + + if len(sink.events) != 1 { + t.Fatalf("expected 1 event, got %d", len(sink.events)) + } + event, ok := sink.events[0].(AuthEvent) + if !ok { + t.Fatalf("expected AuthEvent, got %T", sink.events[0]) + } + if event.Preamble != "Welcome" { + t.Fatalf("expected preamble %q, got %q", "Welcome", event.Preamble) + } + if event.URL != "https://example.com" { + t.Fatalf("expected URL %q, got %q", "https://example.com", event.URL) + } + if event.Code != "1234" { + t.Fatalf("expected code %q, got %q", "1234", event.Code) + } + + line, ok := FormatEventLine(event) + if !ok { + t.Fatal("expected formatter output") + } + expected := "Welcome\nOpening browser to login...\nBrowser didn't open? Visit https://example.com\n\nOne-time code: 1234" + if line != expected { t.Fatalf("unexpected formatted line: %q", line) } } diff --git a/internal/output/plain_format.go b/internal/output/plain_format.go index eaec3942..0593be03 100644 --- a/internal/output/plain_format.go +++ b/internal/output/plain_format.go @@ -96,15 +96,16 @@ func formatAuthEvent(e AuthEvent) string { sb.WriteString(e.Preamble) sb.WriteString("\n") } - if e.Code != "" { - sb.WriteString("Your one-time code: ") - sb.WriteString(e.Code) - sb.WriteString("\n") - } if e.URL != "" { - sb.WriteString("Opening browser to login...\n") + sb.WriteString("Opening browser to login...") + sb.WriteString("\n") + sb.WriteString("Browser didn't open? Visit ") sb.WriteString(e.URL) } + if e.Code != "" { + sb.WriteString("\n\nOne-time code: ") + sb.WriteString(e.Code) + } return strings.TrimRight(sb.String(), "\n") } diff --git a/internal/output/plain_format_test.go b/internal/output/plain_format_test.go index e0bd0ec6..11ea01c2 100644 --- a/internal/output/plain_format_test.go +++ b/internal/output/plain_format_test.go @@ -37,14 +37,14 @@ func TestFormatEventLine(t *testing.T) { }, { name: "instructions event full", - event: AuthEvent{Preamble: "Welcome", Code: "ABC123", URL: "https://example.com"}, - want: "Welcome\nYour one-time code: ABC123\nOpening browser to login...\nhttps://example.com", + event: AuthEvent{Preamble: "Welcome", Code: "ABCD-1234", URL: "https://example.com"}, + want: "Welcome\nOpening browser to login...\nBrowser didn't open? Visit https://example.com\n\nOne-time code: ABCD-1234", wantOK: true, }, { - name: "instructions event code only", - event: AuthEvent{Code: "XYZ"}, - want: "Your one-time code: XYZ", + name: "instructions event url only", + event: AuthEvent{URL: "https://example.com"}, + want: "Opening browser to login...\nBrowser didn't open? Visit https://example.com", wantOK: true, }, { diff --git a/internal/output/plain_sink_test.go b/internal/output/plain_sink_test.go index b55bb872..863d1fd3 100644 --- a/internal/output/plain_sink_test.go +++ b/internal/output/plain_sink_test.go @@ -184,7 +184,7 @@ func TestPlainSink_UsesFormatterParity(t *testing.T) { MessageEvent{Severity: SeverityWarning, Text: "careful"}, MessageEvent{Severity: SeveritySuccess, Text: "done"}, MessageEvent{Severity: SeverityNote, Text: "fyi"}, - AuthEvent{Code: "ABC123", URL: "https://example.com"}, + AuthEvent{URL: "https://example.com"}, SpinnerEvent{Active: true, Text: "Loading"}, ErrorEvent{Title: "Failed", Summary: "Something broke"}, ContainerStatusEvent{Phase: "starting", Container: "localstack"}, diff --git a/internal/ui/app.go b/internal/ui/app.go index 63f97f4e..38917353 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -152,13 +152,14 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.Preamble != "" { a.lines = appendLine(a.lines, styledLine{text: "> " + msg.Preamble, secondary: true}) } - if msg.Code != "" { - a.lines = appendLine(a.lines, styledLine{text: "Your one-time code:"}) - a.lines = appendLine(a.lines, styledLine{text: msg.Code, highlight: true}) - } if msg.URL != "" { a.lines = appendLine(a.lines, styledLine{text: "Opening browser to login..."}) - a.lines = appendLine(a.lines, styledLine{text: msg.URL, secondary: true}) + a.lines = appendLine(a.lines, styledLine{text: "Browser didn't open? Visit " + msg.URL, secondary: true}) + } + if msg.Code != "" { + a.lines = appendLine(a.lines, styledLine{text: ""}) + a.lines = appendLine(a.lines, styledLine{text: styles.Secondary.Render("One-time code: ") + styles.NimboMid.Render(msg.Code)}) + a.lines = appendLine(a.lines, styledLine{text: ""}) } return a, nil case output.LogLineEvent: @@ -265,7 +266,6 @@ func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) s return fmt.Sprintf("%s %s", firstLine, selected) } - // resolveOption finds the best matching option for a key event, in priority order: // 1. "any" — matches any keypress // 2. "enter" — matches the Enter key explicitly diff --git a/test/integration/login_test.go b/test/integration/login_test.go index 89931280..dbd5ea9a 100644 --- a/test/integration/login_test.go +++ b/test/integration/login_test.go @@ -120,10 +120,9 @@ func TestDeviceFlowSuccess(t *testing.T) { out := output.String() require.NoError(t, err, "login should succeed: %s", out) - assert.Contains(t, out, "Your one-time code") - assert.Contains(t, out, "TEST123") assert.Contains(t, out, "Opening browser to login...") - assert.Contains(t, out, "/auth/request/test-auth-req-id") + assert.Contains(t, out, "Browser didn't open? Visit") + assert.Contains(t, out, "/auth/request/test-auth-req-id?code=TEST123") assert.Contains(t, out, "Waiting for authorization") assert.Contains(t, out, "Checking if auth request is confirmed") assert.Contains(t, out, "Auth request confirmed") @@ -177,10 +176,9 @@ func TestDeviceFlowFailure_RequestNotConfirmed(t *testing.T) { out := output.String() require.Error(t, err, "expected login to fail when request not confirmed") - assert.Contains(t, out, "Your one-time code") - assert.Contains(t, out, "TEST123") assert.Contains(t, out, "Opening browser to login...") - assert.Contains(t, out, "/auth/request/test-auth-req-id") + assert.Contains(t, out, "Browser didn't open? Visit") + assert.Contains(t, out, "/auth/request/test-auth-req-id?code=TEST123") assert.Contains(t, out, "Waiting for authorization") assert.Contains(t, out, "Checking if auth request is confirmed") assert.Contains(t, out, "auth request not confirmed")