Skip to content

Commit 298adb6

Browse files
gtsiolisgtsiolis
authored
Improve auth flow and append code in auth request URL (#102)
1 parent 37f88c7 commit 298adb6

File tree

8 files changed

+140
-31
lines changed

8 files changed

+140
-31
lines changed

‎internal/auth/login.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ package auth
55
import (
66
"context"
77
"fmt"
8+
"net/url"
9+
"strings"
810

911
"github.com/localstack/lstk/internal/api"
1012
"github.com/localstack/lstk/internal/output"
@@ -35,14 +37,16 @@ func (l *loginProvider) Login(ctx context.Context) (string, error) {
3537
return "", fmt.Errorf("failed to create auth request: %w", err)
3638
}
3739

38-
authURL := fmt.Sprintf("%s/auth/request/%s", l.webAppURL, authReq.ID)
40+
authURL := buildAuthURL(l.webAppURL, authReq.ID, authReq.Code)
3941

4042
output.EmitAuth(l.sink, output.AuthEvent{
4143
Preamble: "Welcome to lstk, a command-line interface for LocalStack",
4244
Code: authReq.Code,
4345
URL: authURL,
4446
})
45-
_ = browser.OpenURL(authURL)
47+
if err := browser.OpenURL(authURL); err != nil {
48+
output.EmitWarning(l.sink, fmt.Sprintf("Failed to open browser automatically. Open this URL manually to continue: %s", authURL))
49+
}
4650

4751
output.EmitSpinnerStart(l.sink, "Waiting for authorization...")
4852

@@ -66,6 +70,16 @@ func (l *loginProvider) Login(ctx context.Context) (string, error) {
6670
}
6771
}
6872

73+
func buildAuthURL(webAppURL, authRequestID, code string) string {
74+
authURL := fmt.Sprintf("%s/auth/request/%s", strings.TrimRight(webAppURL, "/"), authRequestID)
75+
if code == "" {
76+
return authURL
77+
}
78+
79+
values := url.Values{}
80+
values.Set("code", code)
81+
return authURL + "?" + values.Encode()
82+
}
6983

7084
func (l *loginProvider) completeAuth(ctx context.Context, authReq *api.AuthRequest) (string, error) {
7185
output.EmitInfo(l.sink, "Checking if auth request is confirmed...")

‎internal/auth/login_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package auth
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
func TestBuildAuthURL(t *testing.T) {
9+
t.Parallel()
10+
11+
tests := []struct {
12+
name string
13+
webAppURL string
14+
requestID string
15+
code string
16+
wantAuthURL string
17+
}{
18+
{
19+
name: "adds code query param",
20+
webAppURL: "http://app.localstack.cloud",
21+
requestID: "d78cc482-1db6-4d4d-9f9c-3512963fdf93",
22+
code: "1234",
23+
wantAuthURL: "http://app.localstack.cloud/auth/request/d78cc482-1db6-4d4d-9f9c-3512963fdf93?code=1234",
24+
},
25+
{
26+
name: "escapes query param",
27+
webAppURL: "https://example.com",
28+
requestID: "req-id",
29+
code: "A B+C",
30+
wantAuthURL: "https://example.com/auth/request/req-id?code=A+B%2BC",
31+
},
32+
{
33+
name: "omits empty code",
34+
webAppURL: "https://example.com",
35+
requestID: "req-id",
36+
code: "",
37+
wantAuthURL: "https://example.com/auth/request/req-id",
38+
},
39+
{
40+
name: "trims trailing slash from web app URL",
41+
webAppURL: "https://example.com/",
42+
requestID: "req-id",
43+
code: "1234",
44+
wantAuthURL: "https://example.com/auth/request/req-id?code=1234",
45+
},
46+
}
47+
48+
for _, tt := range tests {
49+
tt := tt
50+
t.Run(tt.name, func(t *testing.T) {
51+
t.Parallel()
52+
53+
got := buildAuthURL(tt.webAppURL, tt.requestID, tt.code)
54+
if got != tt.wantAuthURL {
55+
t.Fatalf("expected auth URL %q, got %q", tt.wantAuthURL, got)
56+
}
57+
if strings.Contains(got, "//auth/request") {
58+
t.Fatalf("expected auth URL without double slash before auth/request, got %q", got)
59+
}
60+
})
61+
}
62+
}

‎internal/output/events_test.go‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ func TestEmitAuth(t *testing.T) {
1616
sink := &captureSink{}
1717
EmitAuth(sink, AuthEvent{
1818
Preamble: "Welcome",
19-
Code: "ABC123",
2019
URL: "https://example.com",
2120
})
2221

@@ -27,9 +26,6 @@ func TestEmitAuth(t *testing.T) {
2726
if !ok {
2827
t.Fatalf("expected AuthEvent, got %T", sink.events[0])
2928
}
30-
if event.Code != "ABC123" {
31-
t.Fatalf("expected code %q, got %q", "ABC123", event.Code)
32-
}
3329
if event.URL != "https://example.com" {
3430
t.Fatalf("expected URL %q, got %q", "https://example.com", event.URL)
3531
}
@@ -41,7 +37,45 @@ func TestEmitAuth(t *testing.T) {
4137
if !ok {
4238
t.Fatal("expected formatter output")
4339
}
44-
if line != "Welcome\nYour one-time code: ABC123\nOpening browser to login...\nhttps://example.com" {
40+
expected := "Welcome\nOpening browser to login...\nBrowser didn't open? Visit https://example.com"
41+
if line != expected {
42+
t.Fatalf("unexpected formatted line: %q", line)
43+
}
44+
}
45+
46+
func TestEmitAuthWithCode(t *testing.T) {
47+
t.Parallel()
48+
49+
sink := &captureSink{}
50+
EmitAuth(sink, AuthEvent{
51+
Preamble: "Welcome",
52+
URL: "https://example.com",
53+
Code: "1234",
54+
})
55+
56+
if len(sink.events) != 1 {
57+
t.Fatalf("expected 1 event, got %d", len(sink.events))
58+
}
59+
event, ok := sink.events[0].(AuthEvent)
60+
if !ok {
61+
t.Fatalf("expected AuthEvent, got %T", sink.events[0])
62+
}
63+
if event.Preamble != "Welcome" {
64+
t.Fatalf("expected preamble %q, got %q", "Welcome", event.Preamble)
65+
}
66+
if event.URL != "https://example.com" {
67+
t.Fatalf("expected URL %q, got %q", "https://example.com", event.URL)
68+
}
69+
if event.Code != "1234" {
70+
t.Fatalf("expected code %q, got %q", "1234", event.Code)
71+
}
72+
73+
line, ok := FormatEventLine(event)
74+
if !ok {
75+
t.Fatal("expected formatter output")
76+
}
77+
expected := "Welcome\nOpening browser to login...\nBrowser didn't open? Visit https://example.com\n\nOne-time code: 1234"
78+
if line != expected {
4579
t.Fatalf("unexpected formatted line: %q", line)
4680
}
4781
}

‎internal/output/plain_format.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,16 @@ func formatAuthEvent(e AuthEvent) string {
9696
sb.WriteString(e.Preamble)
9797
sb.WriteString("\n")
9898
}
99-
if e.Code != "" {
100-
sb.WriteString("Your one-time code: ")
101-
sb.WriteString(e.Code)
102-
sb.WriteString("\n")
103-
}
10499
if e.URL != "" {
105-
sb.WriteString("Opening browser to login...\n")
100+
sb.WriteString("Opening browser to login...")
101+
sb.WriteString("\n")
102+
sb.WriteString("Browser didn't open? Visit ")
106103
sb.WriteString(e.URL)
107104
}
105+
if e.Code != "" {
106+
sb.WriteString("\n\nOne-time code: ")
107+
sb.WriteString(e.Code)
108+
}
108109
return strings.TrimRight(sb.String(), "\n")
109110
}
110111

‎internal/output/plain_format_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,14 @@ func TestFormatEventLine(t *testing.T) {
3737
},
3838
{
3939
name: "instructions event full",
40-
event: AuthEvent{Preamble: "Welcome", Code: "ABC123", URL: "https://example.com"},
41-
want: "Welcome\nYour one-time code: ABC123\nOpening browser to login...\nhttps://example.com",
40+
event: AuthEvent{Preamble: "Welcome", Code: "ABCD-1234", URL: "https://example.com"},
41+
want: "Welcome\nOpening browser to login...\nBrowser didn't open? Visit https://example.com\n\nOne-time code: ABCD-1234",
4242
wantOK: true,
4343
},
4444
{
45-
name: "instructions event code only",
46-
event: AuthEvent{Code: "XYZ"},
47-
want: "Your one-time code: XYZ",
45+
name: "instructions event url only",
46+
event: AuthEvent{URL: "https://example.com"},
47+
want: "Opening browser to login...\nBrowser didn't open? Visit https://example.com",
4848
wantOK: true,
4949
},
5050
{

‎internal/output/plain_sink_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func TestPlainSink_UsesFormatterParity(t *testing.T) {
184184
MessageEvent{Severity: SeverityWarning, Text: "careful"},
185185
MessageEvent{Severity: SeveritySuccess, Text: "done"},
186186
MessageEvent{Severity: SeverityNote, Text: "fyi"},
187-
AuthEvent{Code: "ABC123", URL: "https://example.com"},
187+
AuthEvent{URL: "https://example.com"},
188188
SpinnerEvent{Active: true, Text: "Loading"},
189189
ErrorEvent{Title: "Failed", Summary: "Something broke"},
190190
ContainerStatusEvent{Phase: "starting", Container: "localstack"},

‎internal/ui/app.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,14 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
152152
if msg.Preamble != "" {
153153
a.lines = appendLine(a.lines, styledLine{text: "> " + msg.Preamble, secondary: true})
154154
}
155-
if msg.Code != "" {
156-
a.lines = appendLine(a.lines, styledLine{text: "Your one-time code:"})
157-
a.lines = appendLine(a.lines, styledLine{text: msg.Code, highlight: true})
158-
}
159155
if msg.URL != "" {
160156
a.lines = appendLine(a.lines, styledLine{text: "Opening browser to login..."})
161-
a.lines = appendLine(a.lines, styledLine{text: msg.URL, secondary: true})
157+
a.lines = appendLine(a.lines, styledLine{text: "Browser didn't open? Visit " + msg.URL, secondary: true})
158+
}
159+
if msg.Code != "" {
160+
a.lines = appendLine(a.lines, styledLine{text: ""})
161+
a.lines = appendLine(a.lines, styledLine{text: styles.Secondary.Render("One-time code: ") + styles.NimboMid.Render(msg.Code)})
162+
a.lines = appendLine(a.lines, styledLine{text: ""})
162163
}
163164
return a, nil
164165
case output.LogLineEvent:
@@ -265,7 +266,6 @@ func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) s
265266
return fmt.Sprintf("%s %s", firstLine, selected)
266267
}
267268

268-
269269
// resolveOption finds the best matching option for a key event, in priority order:
270270
// 1. "any" — matches any keypress
271271
// 2. "enter" — matches the Enter key explicitly

‎test/integration/login_test.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,9 @@ func TestDeviceFlowSuccess(t *testing.T) {
120120

121121
out := output.String()
122122
require.NoError(t, err, "login should succeed: %s", out)
123-
assert.Contains(t, out, "Your one-time code")
124-
assert.Contains(t, out, "TEST123")
125123
assert.Contains(t, out, "Opening browser to login...")
126-
assert.Contains(t, out, "/auth/request/test-auth-req-id")
124+
assert.Contains(t, out, "Browser didn't open? Visit")
125+
assert.Contains(t, out, "/auth/request/test-auth-req-id?code=TEST123")
127126
assert.Contains(t, out, "Waiting for authorization")
128127
assert.Contains(t, out, "Checking if auth request is confirmed")
129128
assert.Contains(t, out, "Auth request confirmed")
@@ -177,10 +176,9 @@ func TestDeviceFlowFailure_RequestNotConfirmed(t *testing.T) {
177176

178177
out := output.String()
179178
require.Error(t, err, "expected login to fail when request not confirmed")
180-
assert.Contains(t, out, "Your one-time code")
181-
assert.Contains(t, out, "TEST123")
182179
assert.Contains(t, out, "Opening browser to login...")
183-
assert.Contains(t, out, "/auth/request/test-auth-req-id")
180+
assert.Contains(t, out, "Browser didn't open? Visit")
181+
assert.Contains(t, out, "/auth/request/test-auth-req-id?code=TEST123")
184182
assert.Contains(t, out, "Waiting for authorization")
185183
assert.Contains(t, out, "Checking if auth request is confirmed")
186184
assert.Contains(t, out, "auth request not confirmed")

0 commit comments

Comments
 (0)