diff --git a/Makefile b/Makefile index 4c52a7a9..49291284 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build test test-unit test-smoke coverage fmt fmt-check vet lint tidy tidy-check \ +.PHONY: build test test-unit test-smoke preview-callback coverage fmt fmt-check vet lint tidy tidy-check \ race-test vuln secrets replace-check check-toolchain check security \ release-check release bench bench-save bench-compare \ check-surface check-surface-compat tools clean install help @@ -23,6 +23,7 @@ help: @echo " make test-unit Run unit tests" @echo " make test Alias for test-unit" @echo " make test-smoke Run smoke tests against a live server" + @echo " make preview-callback Preview the OAuth callback screens in a browser" @echo " make coverage Run cross-package coverage and enforce the 70.8% floor" @echo " make clean Remove build artifacts" @echo " make tidy Tidy dependencies" @@ -81,6 +82,12 @@ coverage: check-toolchain @./scripts/coverage-summary.sh $(COVERAGE_PROFILE) $(COVERAGE_FUNCTIONS) $(COVERAGE_PACKAGES) @./scripts/check-coverage.sh $(COVERAGE_PROFILE) $(COVERAGE_FLOOR) +# Serve the OAuth callback screens for visual review at http://127.0.0.1:9999. +# Pages re-render from disk on every request: edit internal/auth/callback*.html +# and refresh the browser. Ctrl-C to stop. +preview-callback: check-toolchain + PREVIEW=1 go test -run TestPreviewCallbackPages ./internal/auth/ -count=1 -v -timeout=0 + # Run smoke tests against a live HEY server. # Requires: a running server (default http://app.hey.localhost:3003) and Chrome. # Override defaults: make test-smoke HEY_SMOKE_BASE_URL=... HEY_SMOKE_EMAIL=... HEY_SMOKE_PASSWORD=... diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 9f9124de..7875935e 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -307,29 +307,45 @@ func (m *Manager) waitForCallback(ctx context.Context, expectedState, authURL, c }) } - server.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mux := http.NewServeMux() + mux.HandleFunc("/callback", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + state := r.URL.Query().Get("state") code := r.URL.Query().Get("code") errParam := r.URL.Query().Get("error") - if errParam != "" { - errCh <- fmt.Errorf("OAuth error: %s", errParam) - fmt.Fprint(w, "

Authentication failed

You can close this window.

") + // Sends are non-blocking: only the first request's result matters, + // and waitForCallback stops reading once it returns. + fail := func(err error, page string) { + select { + case errCh <- err: + default: + } + fmt.Fprint(w, page) shutdownServer() - return } - if state != expectedState { - errCh <- fmt.Errorf("state mismatch: CSRF protection failed") - fmt.Fprint(w, "

Authentication failed

State mismatch.

") + switch { + case state != expectedState: + fail(fmt.Errorf("state mismatch: CSRF protection failed"), callbackInvalid) + case errParam == "access_denied": + fail(fmt.Errorf("OAuth error: %s", errParam), callbackDenied) + case errParam != "": + fail(fmt.Errorf("OAuth error: %s", errParam), callbackError) + case code == "": + fail(fmt.Errorf("OAuth callback missing authorization code"), callbackError) + default: + select { + case codeCh <- code: + default: + } + fmt.Fprint(w, callbackSuccess) shutdownServer() - return } - - codeCh <- code - fmt.Fprint(w, "

Authentication successful!

You can close this window.

") - shutdownServer() }) + server.Handler = mux go server.Serve(listener) //nolint:errcheck diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 66b2f4fa..6510885e 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -215,10 +215,13 @@ func TestWaitForCallback(t *testing.T) { query string wantCode string wantError string + wantBody string }{ - {name: "success", query: "?state=expected&code=authorization-code", wantCode: "authorization-code"}, - {name: "OAuth error", query: "?error=access_denied", wantError: "OAuth error: access_denied"}, - {name: "state mismatch", query: "?state=wrong&code=authorization-code", wantError: "state mismatch"}, + {name: "success", query: "?state=expected&code=authorization-code", wantCode: "authorization-code", wantBody: "Authorization successful"}, + {name: "access denied", query: "?state=expected&error=access_denied", wantError: "OAuth error: access_denied", wantBody: "You denied access"}, + {name: "OAuth error", query: "?state=expected&error=server_error", wantError: "OAuth error: server_error", wantBody: "Authorization failed"}, + {name: "missing code", query: "?state=expected", wantError: "missing authorization code", wantBody: "Authorization failed"}, + {name: "state mismatch", query: "?state=wrong&code=authorization-code", wantError: "state mismatch", wantBody: "authorization link is invalid"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -249,7 +252,14 @@ func TestWaitForCallback(t *testing.T) { if err != nil { t.Fatalf("GET callback: %v", err) } + body, err := io.ReadAll(response.Body) _ = response.Body.Close() + if err != nil { + t.Fatalf("reading callback response: %v", err) + } + if !strings.Contains(string(body), tt.wantBody) { + t.Errorf("response body does not contain %q", tt.wantBody) + } var got result select { case got = <-resultCh: diff --git a/internal/auth/callback.go b/internal/auth/callback.go new file mode 100644 index 00000000..6e0903a5 --- /dev/null +++ b/internal/auth/callback.go @@ -0,0 +1,47 @@ +package auth + +import ( + "bytes" + "embed" + "fmt" + "io/fs" + "text/template" +) + +//go:embed *.html +var callbackFS embed.FS + +var ( + callbackSuccess = mustRenderCallback("callback_success.html") + callbackError = mustRenderCallback("callback_error.html") + callbackDenied = mustRenderCallback("callback_denied.html") + callbackInvalid = mustRenderCallback("callback_invalid.html") +) + +func mustRenderCallback(filename string) string { + page, err := renderCallback(callbackFS, filename) + if err != nil { + panic(err) + } + return page +} + +// renderCallback renders one callback screen: all templates are parsed +// together so {{template "hey_logo.html"}} references resolve, the named +// content page is rendered, then wrapped in the outer shell (callback.html). +// Taking fs.FS lets the preview server render from disk. +func renderCallback(fsys fs.FS, filename string) (string, error) { + tmpl, err := template.ParseFS(fsys, "*.html") + if err != nil { + return "", fmt.Errorf("parsing callback templates: %w", err) + } + var content bytes.Buffer + if err := tmpl.ExecuteTemplate(&content, filename, nil); err != nil { + return "", fmt.Errorf("rendering callback content %s: %w", filename, err) + } + var page bytes.Buffer + if err := tmpl.ExecuteTemplate(&page, "callback.html", content.String()); err != nil { + return "", fmt.Errorf("rendering callback shell: %w", err) + } + return page.String(), nil +} diff --git a/internal/auth/callback.html b/internal/auth/callback.html new file mode 100644 index 00000000..6661b1cc --- /dev/null +++ b/internal/auth/callback.html @@ -0,0 +1,156 @@ + + + + + + Authorize HEY CLI + + + +
+ {{.}} +
+ + diff --git a/internal/auth/callback_denied.html b/internal/auth/callback_denied.html new file mode 100644 index 00000000..9114b6f6 --- /dev/null +++ b/internal/auth/callback_denied.html @@ -0,0 +1,10 @@ + +

You denied access to HEY CLI

+

If this was unintentional, return to your terminal and try hey auth login again.

diff --git a/internal/auth/callback_error.html b/internal/auth/callback_error.html new file mode 100644 index 00000000..165bd59a --- /dev/null +++ b/internal/auth/callback_error.html @@ -0,0 +1,10 @@ + +

Authorization failed

+

Something went wrong. Please return to your terminal and try hey auth login again.

diff --git a/internal/auth/callback_invalid.html b/internal/auth/callback_invalid.html new file mode 100644 index 00000000..09e37fc3 --- /dev/null +++ b/internal/auth/callback_invalid.html @@ -0,0 +1,10 @@ + +

Your authorization link is invalid

+

It may have expired. Please return to your terminal and try hey auth login again.

diff --git a/internal/auth/callback_preview_test.go b/internal/auth/callback_preview_test.go new file mode 100644 index 00000000..ca4766fc --- /dev/null +++ b/internal/auth/callback_preview_test.go @@ -0,0 +1,74 @@ +package auth + +import ( + "fmt" + "net/http" + "os" + "testing" +) + +// TestPreviewCallbackPages serves the callback pages for visual review: +// +// make preview-callback +// +// or directly: +// +// PREVIEW=1 go test -run TestPreviewCallbackPages ./internal/auth/ -count=1 -v -timeout=0 +// +// Then open http://127.0.0.1:9999 in your browser. Pages re-render from the +// HTML files on disk on every request, so edit a template and refresh the +// browser to see the change — no restart needed. Ctrl-C to stop. +func TestPreviewCallbackPages(t *testing.T) { + if os.Getenv("PREVIEW") == "" { + t.Skip("set PREVIEW=1 to run this preview server") + } + + pages := []struct { + path, label, filename string + }{ + {"/success", "Success", "callback_success.html"}, + {"/error", "Error", "callback_error.html"}, + {"/denied", "Denied", "callback_denied.html"}, + {"/invalid", "Invalid / expired", "callback_invalid.html"}, + } + + mux := http.NewServeMux() + mux.HandleFunc("/{$}", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + fmt.Fprint(w, ` + +

Callback page previews

`) + for _, p := range pages { + fmt.Fprintf(w, ` %s`+"\n", p.path, p.label) + } + fmt.Fprint(w, ``) + }) + for _, p := range pages { + mux.HandleFunc(p.path, func(w http.ResponseWriter, r *http.Request) { + // Render from disk, not the embedded copy, so edits show + // up on refresh while the server keeps running. + page, err := renderCallback(os.DirFS("."), p.filename) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") + fmt.Fprint(w, page) + }) + } + + t.Log("Preview server running at http://127.0.0.1:9999") + for _, p := range pages { + t.Logf(" http://127.0.0.1:9999%s", p.path) + } + t.Log("Edit the callback_*.html files and refresh the browser. Ctrl-C to stop.") + + server := &http.Server{Addr: "127.0.0.1:9999", Handler: mux} //nolint:gosec // G112: local preview server, timeouts irrelevant + if err := server.ListenAndServe(); err != http.ErrServerClosed { + t.Fatal(err) + } +} diff --git a/internal/auth/callback_success.html b/internal/auth/callback_success.html new file mode 100644 index 00000000..48b380c2 --- /dev/null +++ b/internal/auth/callback_success.html @@ -0,0 +1,10 @@ + +

Authorization successful

+

You can close this window and return to your terminal.

diff --git a/internal/auth/callback_test.go b/internal/auth/callback_test.go new file mode 100644 index 00000000..b491789e --- /dev/null +++ b/internal/auth/callback_test.go @@ -0,0 +1,40 @@ +package auth + +import ( + "strings" + "testing" +) + +func TestCallbackPagesRender(t *testing.T) { + tests := []struct { + name string + page string + }{ + {name: "success", page: callbackSuccess}, + {name: "error", page: callbackError}, + {name: "denied", page: callbackDenied}, + {name: "invalid", page: callbackInvalid}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if !strings.HasPrefix(tt.page, "") { + t.Error("page is not wrapped in the shell template") + } + if !strings.Contains(tt.page, "") { + t.Error("page does not contain a heading") + } + if strings.Contains(tt.page, "{{") { + t.Error("page contains an unrendered template directive") + } + }) + } +} + +func TestRenderCallbackUnknownFile(t *testing.T) { + if _, err := renderCallback(callbackFS, "callback_missing.html"); err == nil { + t.Fatal("expected an error for an unknown template file") + } +} diff --git a/internal/auth/hey_logo.html b/internal/auth/hey_logo.html new file mode 100644 index 00000000..7457c516 --- /dev/null +++ b/internal/auth/hey_logo.html @@ -0,0 +1 @@ + \ No newline at end of file