From 576b00cd5261ec1b4e5dbe0db872f44fed58bf74 Mon Sep 17 00:00:00 2001 From: Maurice Kherlakian Date: Wed, 28 Oct 2026 09:08:59 -0400 Subject: [PATCH] feat(login): print the confirmation code the sign-in page asks for hookdeck login and the MCP login tool now ask POST /cli-auth for a user code (RFC 8628) and show it with the sign-in link. The browser page asks for that code before connecting the CLI. An API that issues no code is handled as before: only the link is shown. Co-Authored-By: Claude Opus 5.5 --- pkg/gateway/mcp/server_test.go | 67 ++++++++++++++++++++++++++++++++++ pkg/hookdeck/auth.go | 16 ++++++-- pkg/hookdeck/auth_test.go | 42 +++++++++++++++++++++ pkg/login/client_login.go | 13 ++++++- pkg/login/login_link_test.go | 42 +++++++++++++++++++++ pkg/mcpcore/tool_login.go | 18 +++++++-- 6 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 pkg/login/login_link_test.go diff --git a/pkg/gateway/mcp/server_test.go b/pkg/gateway/mcp/server_test.go index b9ab5739..8fbaaaf0 100644 --- a/pkg/gateway/mcp/server_test.go +++ b/pkg/gateway/mcp/server_test.go @@ -1841,6 +1841,73 @@ func TestLoginTool_InProgressShowsURL(t *testing.T) { assert.Contains(t, text, "https://hookdeck.com/auth?code=xyz") } +func TestLoginTool_ShowsUserCode(t *testing.T) { + api := mockAPI(t, map[string]http.HandlerFunc{ + hookdeck.APIPathPrefix + "/cli-auth": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "browser_url": "https://hookdeck.com/auth?code=xyz", + "poll_url": "http://" + r.Host + hookdeck.APIPathPrefix + "/cli-auth/poll?key=xyz", + "user_code": "WDJB-MJHT", + }) + }, + hookdeck.APIPathPrefix + "/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"claimed": false}) + }, + }) + + unauthClient := newTestClient(api.URL, "") + cfg := &config.Config{APIBaseURL: api.URL} + srv := NewServer(ServerOptions{Client: unauthClient, Config: cfg}) + + serverTransport, clientTransport := mcpsdk.NewInMemoryTransports() + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + go func() { _ = srv.Run(ctx, serverTransport) }() + + mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil) + session, err := mcpClient.Connect(ctx, clientTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = session.Close() }) + + // Both the call that starts the flow and the in-progress reminder carry the code. + for _, call := range []string{"start", "in progress"} { + result := callTool(t, session, "hookdeck_login", map[string]any{}) + assert.False(t, result.IsError, call) + assert.Contains(t, textContent(t, result), "the user must enter: WDJB-MJHT", call) + } +} + +func TestLoginTool_NoCodeLineWithoutUserCode(t *testing.T) { + api := mockAPI(t, map[string]http.HandlerFunc{ + hookdeck.APIPathPrefix + "/cli-auth": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{ + "browser_url": "https://hookdeck.com/auth?code=xyz", + "poll_url": "http://" + r.Host + hookdeck.APIPathPrefix + "/cli-auth/poll?key=xyz", + }) + }, + hookdeck.APIPathPrefix + "/cli-auth/poll": func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"claimed": false}) + }, + }) + + unauthClient := newTestClient(api.URL, "") + cfg := &config.Config{APIBaseURL: api.URL} + srv := NewServer(ServerOptions{Client: unauthClient, Config: cfg}) + + serverTransport, clientTransport := mcpsdk.NewInMemoryTransports() + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + go func() { _ = srv.Run(ctx, serverTransport) }() + + mcpClient := mcpsdk.NewClient(&mcpsdk.Implementation{Name: "test", Version: "0.0.1"}, nil) + session, err := mcpClient.Connect(ctx, clientTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = session.Close() }) + + result := callTool(t, session, "hookdeck_login", map[string]any{}) + assert.NotContains(t, textContent(t, result), "code, the user must enter") +} + func TestLoginTool_PollSurvivesAcrossToolCalls(t *testing.T) { // Regression: the login polling goroutine must use the session-level // context, not the per-request ctx (which is cancelled when the handler diff --git a/pkg/hookdeck/auth.go b/pkg/hookdeck/auth.go index 9ff6f02d..dc6c9dd7 100644 --- a/pkg/hookdeck/auth.go +++ b/pkg/hookdeck/auth.go @@ -61,7 +61,10 @@ type UpdateClientInput struct { // LoginSession represents an in-progress login flow type LoginSession struct { BrowserURL string - pollURL string + // UserCode is the code the user types on the browser page to confirm the + // login is theirs (RFC 8628 user_code). Empty when the API issued none. + UserCode string + pollURL string } // GuestSession represents an in-progress guest login flow @@ -73,10 +76,15 @@ type GuestSession struct { // StartLogin initiates the login flow and returns a session to wait for completion func (c *Client) StartLogin(deviceName string) (*LoginSession, error) { + // with_user_code asks for a code to print here, which the browser page asks + // for to confirm the login (RFC 8628 user_code). An API that predates it + // drops the unknown field and returns no code. data := struct { - DeviceName string `json:"device_name"` + DeviceName string `json:"device_name"` + WithUserCode bool `json:"with_user_code"` }{ - DeviceName: deviceName, + DeviceName: deviceName, + WithUserCode: true, } jsonData, err := json.Marshal(data) @@ -98,6 +106,7 @@ func (c *Client) StartLogin(deviceName string) (*LoginSession, error) { var links struct { BrowserURL string `json:"browser_url"` PollURL string `json:"poll_url"` + UserCode string `json:"user_code"` } err = json.Unmarshal(body, &links) if err != nil { @@ -106,6 +115,7 @@ func (c *Client) StartLogin(deviceName string) (*LoginSession, error) { return &LoginSession{ BrowserURL: links.BrowserURL, + UserCode: links.UserCode, pollURL: links.PollURL, }, nil } diff --git a/pkg/hookdeck/auth_test.go b/pkg/hookdeck/auth_test.go index 981f2a3c..3866a33d 100644 --- a/pkg/hookdeck/auth_test.go +++ b/pkg/hookdeck/auth_test.go @@ -51,3 +51,45 @@ func TestValidateAPIKey_omitsTeamAndProjectHeadersWhenConfigHasProjectID(t *test require.Equal(t, "t1", resp.ProjectID) require.Equal(t, "event_gateway", resp.ProjectType) } + +func TestStartLogin_requestsAndReturnsUserCode(t *testing.T) { + var body map[string]any + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, APIPathPrefix+"/cli-auth", r.URL.Path) + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{ + "browser_url": "https://dashboard.example.test/cli-auth/k", + "poll_url": "https://api.example.test/cli-auth/poll?key=k", + "user_code": "WDJB-MJHT", + }) + })) + t.Cleanup(server.Close) + + baseURL, err := url.Parse(server.URL) + require.NoError(t, err) + + session, err := (&Client{BaseURL: baseURL}).StartLogin("laptop") + require.NoError(t, err) + require.Equal(t, map[string]any{"device_name": "laptop", "with_user_code": true}, body) + require.Equal(t, "WDJB-MJHT", session.UserCode) + require.Equal(t, "https://dashboard.example.test/cli-auth/k", session.BrowserURL) +} + +func TestStartLogin_noUserCodeFromAnAPIThatIssuesNone(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{ + "browser_url": "https://dashboard.example.test/cli-auth/k", + "poll_url": "https://api.example.test/cli-auth/poll?key=k", + }) + })) + t.Cleanup(server.Close) + + baseURL, err := url.Parse(server.URL) + require.NoError(t, err) + + session, err := (&Client{BaseURL: baseURL}).StartLogin("laptop") + require.NoError(t, err) + require.Empty(t, session.UserCode) +} diff --git a/pkg/login/client_login.go b/pkg/login/client_login.go index 821d3ada..0e97e5ed 100644 --- a/pkg/login/client_login.go +++ b/pkg/login/client_login.go @@ -150,11 +150,20 @@ func Login(config *configpkg.Config, input io.Reader) error { return waitForLoginSession(config, input, session) } +// printLoginLink prints the sign-in URL and, when the API issued one, the code +// the browser page asks for. +func printLoginLink(session *hookdeck.LoginSession) { + fmt.Printf("To authenticate with Hookdeck, please go to: %s\n", session.BrowserURL) + if session.UserCode != "" { + fmt.Printf("When the page asks for a code, enter: %s\n", session.UserCode) + } +} + func waitForLoginSession(config *configpkg.Config, input io.Reader, session *hookdeck.LoginSession) error { var s *spinner.Spinner if isSSH() || !canOpenBrowser() { - fmt.Printf("To authenticate with Hookdeck, please go to: %s\n", session.BrowserURL) + printLoginLink(session) s = ansi.StartNewSpinner("Waiting for confirmation...", os.Stdout) } else { @@ -168,7 +177,7 @@ func waitForLoginSession(config *configpkg.Config, input io.Reader, session *hoo // is spawned, so a browser that dies straight after - WSL, containers, // VS Code Remote - reports success and leaves the user with a spinner // and no link. The error branch below is the detectable half only. - fmt.Printf("To authenticate with Hookdeck, please go to: %s\n", session.BrowserURL) + printLoginLink(session) if err := openBrowser(session.BrowserURL); err != nil { fmt.Println("Could not open the browser for you; use the link above.") diff --git a/pkg/login/login_link_test.go b/pkg/login/login_link_test.go new file mode 100644 index 00000000..5975699c --- /dev/null +++ b/pkg/login/login_link_test.go @@ -0,0 +1,42 @@ +package login + +import ( + "io" + "os" + "testing" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/stretchr/testify/require" +) + +func capturePrintLoginLink(t *testing.T, session *hookdeck.LoginSession) string { + t.Helper() + r, w, err := os.Pipe() + require.NoError(t, err) + oldStdout := os.Stdout + os.Stdout = w + printLoginLink(session) + os.Stdout = oldStdout + require.NoError(t, w.Close()) + out, err := io.ReadAll(r) + require.NoError(t, err) + return string(out) +} + +func TestPrintLoginLink_printsTheCodeThePageAsksFor(t *testing.T) { + out := capturePrintLoginLink(t, &hookdeck.LoginSession{ + BrowserURL: "https://dashboard.example.test/cli-auth/k", + UserCode: "WDJB-MJHT", + }) + require.Equal(t, + "To authenticate with Hookdeck, please go to: https://dashboard.example.test/cli-auth/k\n"+ + "When the page asks for a code, enter: WDJB-MJHT\n", + out) +} + +func TestPrintLoginLink_onlyTheURLWithoutACode(t *testing.T) { + out := capturePrintLoginLink(t, &hookdeck.LoginSession{ + BrowserURL: "https://dashboard.example.test/cli-auth/k", + }) + require.Equal(t, "To authenticate with Hookdeck, please go to: https://dashboard.example.test/cli-auth/k\n", out) +} diff --git a/pkg/mcpcore/tool_login.go b/pkg/mcpcore/tool_login.go index 2ff6ff69..7e5bea85 100644 --- a/pkg/mcpcore/tool_login.go +++ b/pkg/mcpcore/tool_login.go @@ -34,6 +34,7 @@ const ( // close provides the happens-before guarantee — no separate mutex needed. type loginState struct { browserURL string // URL the user must open + userCode string // code the page asks for; empty if the API issued none done chan struct{} // closed when polling finishes err error // non-nil if polling failed } @@ -180,8 +181,8 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { default: // Still polling — remind the agent about the URL. return TextResult(fmt.Sprintf( - "Login is already in progress. Waiting for the user to complete authentication.\n\nThe user needs to open this URL in their browser:\n\n%s\n\nCall %s again to check status.", - state.browserURL, loginTool, + "Login is already in progress. Waiting for the user to complete authentication.\n\nThe user needs to open this URL in their browser:\n\n%s%s\n\nCall %s again to check status.", + state.browserURL, userCodeInstruction(state.userCode), loginTool, )), nil } } @@ -205,6 +206,7 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { // Set up background polling state. state = &loginState{ browserURL: session.BrowserURL, + userCode: session.UserCode, done: make(chan struct{}), } @@ -292,10 +294,20 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { // Return the URL immediately so the agent can show it to the user. return TextResult(fmt.Sprintf( - "%sLogin initiated. The user must open the following URL in their browser to authenticate:\n\n%s\n\nOnce the user completes authentication in the browser, all Hookdeck tools will become available.\nCall %s again to check if authentication has completed.", + "%sLogin initiated. The user must open the following URL in their browser to authenticate:\n\n%s%s\n\nOnce the user completes authentication in the browser, all Hookdeck tools will become available.\nCall %s again to check if authentication has completed.", loginPrefix, session.BrowserURL, + userCodeInstruction(session.UserCode), loginTool, )), nil } } + +// userCodeInstruction tells the agent which code the user must type on the +// sign-in page, or nothing when the API issued none. +func userCodeInstruction(userCode string) string { + if userCode == "" { + return "" + } + return fmt.Sprintf("\n\nWhen the page asks for a code, the user must enter: %s", userCode) +}