From acad4fe60f1e039e1bfb815ee2f3fc95359d2e61 Mon Sep 17 00:00:00 2001 From: Alexandre Bouchard Date: Fri, 3 Jul 2026 14:31:30 -0400 Subject: [PATCH 1/2] Simplify CLI guest login upgrade --- README.md | 2 +- pkg/config/config.go | 6 - pkg/config/profile.go | 2 - pkg/config/profile_credentials.go | 10 +- pkg/config/profile_credentials_test.go | 7 +- pkg/gateway/mcp/tool_login.go | 5 +- pkg/hookdeck/auth.go | 35 +- pkg/hookdeck/guest.go | 9 +- pkg/hookdeck/request_log_redact_test.go | 63 ++- pkg/login/claimed_cli_key_test.go | 191 +++++---- pkg/login/client_login.go | 85 ++++- pkg/login/client_login_test.go | 68 ++-- pkg/login/guest_link.go | 67 ++-- pkg/login/guest_link_test.go | 326 ++++++++-------- pkg/login/interactive_login.go | 2 +- pkg/login/login_intent.go | 29 -- pkg/login/login_intent_test.go | 34 -- .../acceptance/guest_login_acceptance_test.go | 361 ++++++++++-------- 18 files changed, 641 insertions(+), 661 deletions(-) delete mode 100644 pkg/login/login_intent.go delete mode 100644 pkg/login/login_intent_test.go diff --git a/README.md b/README.md index 8de4661e..f59ffea4 100644 --- a/README.md +++ b/README.md @@ -1626,7 +1626,7 @@ The CLI calls `POST /cli-auth/ci` with that Project API key; the server returns ### Guest credentials -If you run `hookdeck listen` without an existing profile, the CLI can create a **guest** sandbox (`POST /cli/guest`). The config may include `guest_url`, `guest_user_id`, and an `api_key` for that sandbox. Guest login and conversion flows are separate from `hookdeck login --cli-key` and from Project API keys. +If you run `hookdeck listen` without an existing profile, the CLI can create a **guest** sandbox (`POST /cli/guest`). The config may include `guest_url` and an `api_key` for that sandbox. `hookdeck login` reuses that existing guest key and waits for the server to upgrade the guest account into a permanent account. ### `project list` / `project use` diff --git a/pkg/config/config.go b/pkg/config/config.go index 1b0c3bd4..21f2adf4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -251,9 +251,6 @@ func (c *Config) setProfileFieldsInViper(v *viper.Viper) { if c.Profile.GuestURL != "" { v.Set(c.Profile.getConfigField("guest_url"), c.Profile.GuestURL) } - if c.Profile.GuestUserID != "" { - v.Set(c.Profile.getConfigField("guest_user_id"), c.Profile.GuestUserID) - } } // GetConfigFile returns the path of the currently loaded config file @@ -360,8 +357,6 @@ func (c *Config) constructConfig() { c.Profile.GuestURL = stringCoalesce(c.Profile.GuestURL, c.viper.GetString(c.Profile.getConfigField("guest_url")), c.viper.GetString("guest_url"), "") - c.Profile.GuestUserID = stringCoalesce(c.Profile.GuestUserID, c.viper.GetString(c.Profile.getConfigField("guest_user_id")), c.viper.GetString("guest_user_id"), "") - // Telemetry opt-out: check config file for telemetry_disabled = true if c.viper.IsSet("telemetry_disabled") { c.TelemetryDisabled = c.viper.GetBool("telemetry_disabled") @@ -408,7 +403,6 @@ func zeroProfileCredentialFields(p *Profile) { p.ProjectMode = "" p.ProjectType = "" p.GuestURL = "" - p.GuestUserID = "" } // SaveActiveProfileAfterLogin persists cfg.Profile credential fields and the active profile diff --git a/pkg/config/profile.go b/pkg/config/profile.go index 384e7b4e..2608f734 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -11,7 +11,6 @@ type Profile struct { ProjectMode string ProjectType string // display type: Gateway, Outpost, Console GuestURL string // URL to create permanent account for guest users - GuestUserID string // Hookdeck user id for guest accounts (for signup lineage) Config *Config } @@ -31,7 +30,6 @@ func (p *Profile) SaveProfile() error { } p.Config.viper.Set(p.getConfigField("project_type"), projectType) p.Config.viper.Set(p.getConfigField("guest_url"), p.GuestURL) - p.Config.viper.Set(p.getConfigField("guest_user_id"), p.GuestUserID) return p.Config.writeConfig() } diff --git a/pkg/config/profile_credentials.go b/pkg/config/profile_credentials.go index 26346110..fc12d300 100644 --- a/pkg/config/profile_credentials.go +++ b/pkg/config/profile_credentials.go @@ -14,14 +14,12 @@ func (p *Profile) ApplyValidateAPIKeyResponse(resp *hookdeck.ValidateAPIKeyRespo p.ProjectType = ModeToProjectType(resp.ProjectMode) if clearGuestURL { p.GuestURL = "" - p.GuestUserID = "" } } // ApplyPollAPIKeyResponse applies credentials from a completed CLI auth poll (browser or interactive login). // guestURL is the guest upgrade URL when applicable; use "" for a normal account login. -// guestUserID is the Hookdeck user id when the session is a guest account. -func (p *Profile) ApplyPollAPIKeyResponse(resp *hookdeck.PollAPIKeyResponse, guestURL string, guestUserID string) { +func (p *Profile) ApplyPollAPIKeyResponse(resp *hookdeck.PollAPIKeyResponse, guestURL string) { if resp == nil { return } @@ -30,11 +28,6 @@ func (p *Profile) ApplyPollAPIKeyResponse(resp *hookdeck.PollAPIKeyResponse, gue p.ProjectMode = resp.ProjectMode p.ProjectType = ModeToProjectType(resp.ProjectMode) p.GuestURL = guestURL - if guestUserID != "" { - p.GuestUserID = guestUserID - } else if guestURL == "" { - p.GuestUserID = "" - } } // ApplyCIClient applies credentials from hookdeck login --ci. @@ -44,5 +37,4 @@ func (p *Profile) ApplyCIClient(ci hookdeck.CIClient) { p.ProjectMode = ci.ProjectMode p.ProjectType = ModeToProjectType(ci.ProjectMode) p.GuestURL = "" - p.GuestUserID = "" } diff --git a/pkg/config/profile_credentials_test.go b/pkg/config/profile_credentials_test.go index e5c65d04..bdb48d2b 100644 --- a/pkg/config/profile_credentials_test.go +++ b/pkg/config/profile_credentials_test.go @@ -42,7 +42,7 @@ func TestProfile_ApplyValidateAPIKeyResponse(t *testing.T) { func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { t.Run("nil response is no-op", func(t *testing.T) { p := &Profile{APIKey: "k", ProjectId: "p"} - p.ApplyPollAPIKeyResponse(nil, "", "") + p.ApplyPollAPIKeyResponse(nil, "") require.Equal(t, "k", p.APIKey) require.Equal(t, "p", p.ProjectId) }) @@ -53,12 +53,11 @@ func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { APIKey: "key_from_poll", ProjectID: "team_p", ProjectMode: "inbound", - }, "https://guest", "guest_user_1") + }, "https://guest") require.Equal(t, "key_from_poll", p.APIKey) require.Equal(t, "team_p", p.ProjectId) require.Equal(t, ProjectTypeGateway, p.ProjectType) require.Equal(t, "https://guest", p.GuestURL) - require.Equal(t, "guest_user_1", p.GuestUserID) }) t.Run("clears guest URL when empty string passed", func(t *testing.T) { @@ -67,7 +66,7 @@ func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { APIKey: "k123456789012", ProjectID: "t", ProjectMode: "inbound", - }, "", "") + }, "") require.Empty(t, p.GuestURL) }) } diff --git a/pkg/gateway/mcp/tool_login.go b/pkg/gateway/mcp/tool_login.go index 0b4d2a4c..9b8de880 100644 --- a/pkg/gateway/mcp/tool_login.go +++ b/pkg/gateway/mcp/tool_login.go @@ -14,7 +14,6 @@ import ( mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" - "github.com/hookdeck/hookdeck-cli/pkg/login" "github.com/hookdeck/hookdeck-cli/pkg/project" "github.com/hookdeck/hookdeck-cli/pkg/validators" ) @@ -113,7 +112,7 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { } authClient := &hookdeck.Client{BaseURL: parsedBaseURL, TelemetryDisabled: cfg.TelemetryDisabled} - session, err := authClient.StartLogin(login.BuildStartLoginInput(cfg, "")) + session, err := authClient.StartLogin(cfg.DeviceName) if err != nil { return ErrorResult(fmt.Sprintf("Failed to start login: %s", err)), nil } @@ -163,7 +162,7 @@ func handleLogin(srv *Server) mcpsdk.ToolHandler { } // Persist credentials so future MCP sessions start authenticated. - cfg.Profile.ApplyPollAPIKeyResponse(response, "", "") + cfg.Profile.ApplyPollAPIKeyResponse(response, "") cfg.SaveActiveProfileAfterLogin() diff --git a/pkg/hookdeck/auth.go b/pkg/hookdeck/auth.go index 4c0d5cb1..9d3560a4 100644 --- a/pkg/hookdeck/auth.go +++ b/pkg/hookdeck/auth.go @@ -21,6 +21,7 @@ type ValidateAPIKeyResponse struct { UserID string `json:"user_id"` UserName string `json:"user_name"` UserEmail string `json:"user_email"` + UserIsGuest bool `json:"user_is_guest"` OrganizationName string `json:"organization_name"` OrganizationID string `json:"organization_id"` ProjectID string `json:"team_id"` @@ -49,13 +50,6 @@ type UpdateClientInput struct { DeviceName string `json:"device_name"` } -// StartLoginInput configures POST /cli-auth for browser login. -type StartLoginInput struct { - DeviceName string `json:"device_name"` - GuestUserID string `json:"guest_user_id,omitempty"` - GuestAPIKey string `json:"guest_api_key,omitempty"` -} - // LoginSession represents an in-progress login flow type LoginSession struct { BrowserURL string @@ -64,15 +58,20 @@ type LoginSession struct { // GuestSession represents an in-progress guest login flow type GuestSession struct { - BrowserURL string - GuestURL string - GuestUserID string - pollURL string + BrowserURL string + GuestURL string + pollURL string } // StartLogin initiates the login flow and returns a session to wait for completion -func (c *Client) StartLogin(input StartLoginInput) (*LoginSession, error) { - jsonData, err := json.Marshal(input) +func (c *Client) StartLogin(deviceName string) (*LoginSession, error) { + data := struct { + DeviceName string `json:"device_name"` + }{ + DeviceName: deviceName, + } + + jsonData, err := json.Marshal(data) if err != nil { return nil, err } @@ -106,17 +105,17 @@ func (c *Client) StartLogin(input StartLoginInput) (*LoginSession, error) { // StartGuestLogin initiates a guest login flow and returns a session to wait for completion func (c *Client) StartGuestLogin(deviceName string) (*GuestSession, error) { guest, err := c.CreateGuestUser(CreateGuestUserInput{ - DeviceName: deviceName, + DeviceName: deviceName, + LinkContext: "signup", }) if err != nil { return nil, err } return &GuestSession{ - BrowserURL: guest.BrowserURL, - GuestURL: guest.Url, - GuestUserID: guest.Id, - pollURL: guest.PollURL, + BrowserURL: guest.BrowserURL, + GuestURL: guest.Url, + pollURL: guest.PollURL, }, nil } diff --git a/pkg/hookdeck/guest.go b/pkg/hookdeck/guest.go index 589c79a2..1df5debf 100644 --- a/pkg/hookdeck/guest.go +++ b/pkg/hookdeck/guest.go @@ -23,7 +23,8 @@ type GuestSigninLinkResponse struct { } type CreateGuestUserInput struct { - DeviceName string `json:"device_name"` + DeviceName string `json:"device_name"` + LinkContext string `json:"link_context,omitempty"` } func (c *Client) CreateGuestUser(input CreateGuestUserInput) (GuestUser, error) { @@ -44,7 +45,11 @@ func (c *Client) CreateGuestUser(input CreateGuestUserInput) (GuestUser, error) } func (c *Client) RefreshGuestSigninLink() (GuestSigninLinkResponse, error) { - res, err := c.Post(context.Background(), APIPathPrefix+"/cli/guest/signin-link", nil, nil) + input_bytes, err := json.Marshal(CreateGuestUserInput{LinkContext: "signup"}) + if err != nil { + return GuestSigninLinkResponse{}, err + } + res, err := c.Post(context.Background(), APIPathPrefix+"/cli/guest", input_bytes, nil) if err != nil { return GuestSigninLinkResponse{}, err } diff --git a/pkg/hookdeck/request_log_redact_test.go b/pkg/hookdeck/request_log_redact_test.go index e0152406..006f8dd9 100644 --- a/pkg/hookdeck/request_log_redact_test.go +++ b/pkg/hookdeck/request_log_redact_test.go @@ -1,32 +1,31 @@ -package hookdeck - -import ( - "net/http" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestRedactHeadersForLog_redactsAuthorization(t *testing.T) { - headers := http.Header{} - headers.Set("Authorization", "Basic c2tfdGVzdDo=") - headers.Set("Content-Type", "application/json") - - redacted := redactHeadersForLog(headers) - require.Equal(t, "[redacted]", redacted.Get("Authorization")) - require.Equal(t, "application/json", redacted.Get("Content-Type")) - require.Equal(t, "Basic c2tfdGVzdDo=", headers.Get("Authorization")) -} - -func TestRedactRequestBodyForLog_redactsGuestAPIKey(t *testing.T) { - body := `{"device_name":"laptop","guest_user_id":"usr_1","guest_api_key":"hk_secret"}` - redacted := redactRequestBodyForLog(body) - require.Contains(t, redacted, `"guest_api_key":"[redacted]"`) - require.Contains(t, redacted, `"guest_user_id":"usr_1"`) - require.NotContains(t, redacted, "hk_secret") -} - -func TestRedactRequestBodyForLog_leavesNonJSONUnchanged(t *testing.T) { - body := "guest_api_key=hk_secret" - require.Equal(t, body, redactRequestBodyForLog(body)) -} +package hookdeck + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRedactHeadersForLog_redactsAuthorization(t *testing.T) { + headers := http.Header{} + headers.Set("Authorization", "Basic c2tfdGVzdDo=") + headers.Set("Content-Type", "application/json") + + redacted := redactHeadersForLog(headers) + require.Equal(t, "[redacted]", redacted.Get("Authorization")) + require.Equal(t, "application/json", redacted.Get("Content-Type")) + require.Equal(t, "Basic c2tfdGVzdDo=", headers.Get("Authorization")) +} + +func TestRedactRequestBodyForLog_redactsGuestAPIKey(t *testing.T) { + body := `{"device_name":"laptop","guest_api_key":"hk_secret"}` + redacted := redactRequestBodyForLog(body) + require.Contains(t, redacted, `"guest_api_key":"[redacted]"`) + require.NotContains(t, redacted, "hk_secret") +} + +func TestRedactRequestBodyForLog_leavesNonJSONUnchanged(t *testing.T) { + body := "guest_api_key=hk_secret" + require.Equal(t, body, redactRequestBodyForLog(body)) +} diff --git a/pkg/login/claimed_cli_key_test.go b/pkg/login/claimed_cli_key_test.go index dc4f0d5b..dda7b466 100644 --- a/pkg/login/claimed_cli_key_test.go +++ b/pkg/login/claimed_cli_key_test.go @@ -1,97 +1,94 @@ -package login - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/require" - - configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" -) - -func TestConfigureFromClaimedCliKey_guestProfileReplacesCredentials(t *testing.T) { - configpkg.ResetAPIClientForTesting() - t.Cleanup(configpkg.ResetAPIClientForTesting) - - onboardingKey := "hk_test_onboard_abcdefghij" - var serverURL string - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, http.MethodGet, r.Method) - require.True(t, strings.HasSuffix(r.URL.Path, "/cli-auth/validate")) - - body, err := json.Marshal(map[string]string{ - "user_id": "usr_platform", - "user_name": "Platform User", - "user_email": "platform@example.com", - "organization_name": "Acme", - "organization_id": "org_1", - "team_id": "tm_gateway", - "team_name_no_org": "Production", - "team_mode": "inbound", - "client_id": "cl_onboard", - }) - require.NoError(t, err) - _, _ = w.Write(body) - })) - serverURL = ts.URL - t.Cleanup(ts.Close) - - configPath := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" - -[default] -api_key = "hk_test_guestkey_abcdefghij" -guest_url = "https://console.test/signin/guest?token=abc" -guest_user_id = "usr_guest" -`), 0o600)) - - cfg, err := configpkg.LoadConfigFromFile(configPath) - require.NoError(t, err) - cfg.APIBaseURL = serverURL - cfg.LogLevel = "error" - cfg.TelemetryDisabled = true - - err = ConfigureFromClaimedCliKey(cfg, onboardingKey) - require.NoError(t, err) - require.Equal(t, onboardingKey, cfg.Profile.APIKey) - require.Equal(t, "tm_gateway", cfg.Profile.ProjectId) - require.Empty(t, cfg.Profile.GuestURL) - require.Empty(t, cfg.Profile.GuestUserID) - - rewritten, err := os.ReadFile(configPath) - require.NoError(t, err) - require.Contains(t, string(rewritten), onboardingKey) - require.Contains(t, string(rewritten), "tm_gateway") - require.NotContains(t, string(rewritten), "usr_guest") -} - -func TestConfigureFromClaimedCliKey_validateFailureDoesNotStartDeviceLogin(t *testing.T) { - configpkg.ResetAPIClientForTesting() - t.Cleanup(configpkg.ResetAPIClientForTesting) - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, http.MethodGet, r.Method) - require.True(t, strings.HasSuffix(r.URL.Path, "/cli-auth/validate")) - w.WriteHeader(http.StatusUnauthorized) - })) - t.Cleanup(ts.Close) - - cfg := &configpkg.Config{ - APIBaseURL: ts.URL, - LogLevel: "error", - TelemetryDisabled: true, - } - cfg.Profile = configpkg.Profile{ - Name: "default", - GuestURL: "https://console.test/guest", - GuestUserID: "usr_guest", - } - - err := ConfigureFromClaimedCliKey(cfg, "hk_test_invalid_abcdefghij") - require.Error(t, err) -} +package login + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" +) + +func TestConfigureFromClaimedCliKey_guestProfileReplacesCredentials(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + onboardingKey := "hk_test_onboard_abcdefghij" + var serverURL string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + require.True(t, strings.HasSuffix(r.URL.Path, "/cli-auth/validate")) + + body, err := json.Marshal(map[string]string{ + "user_id": "usr_platform", + "user_name": "Platform User", + "user_email": "platform@example.com", + "organization_name": "Acme", + "organization_id": "org_1", + "team_id": "tm_gateway", + "team_name_no_org": "Production", + "team_mode": "inbound", + "client_id": "cl_onboard", + }) + require.NoError(t, err) + _, _ = w.Write(body) + })) + serverURL = ts.URL + t.Cleanup(ts.Close) + + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" + +[default] +api_key = "hk_test_guestkey_abcdefghij" +guest_url = "https://console.test/signin/guest?token=abc" +`), 0o600)) + + cfg, err := configpkg.LoadConfigFromFile(configPath) + require.NoError(t, err) + cfg.APIBaseURL = serverURL + cfg.LogLevel = "error" + cfg.TelemetryDisabled = true + + err = ConfigureFromClaimedCliKey(cfg, onboardingKey) + require.NoError(t, err) + require.Equal(t, onboardingKey, cfg.Profile.APIKey) + require.Equal(t, "tm_gateway", cfg.Profile.ProjectId) + require.Empty(t, cfg.Profile.GuestURL) + + rewritten, err := os.ReadFile(configPath) + require.NoError(t, err) + require.Contains(t, string(rewritten), onboardingKey) + require.Contains(t, string(rewritten), "tm_gateway") + require.NotContains(t, string(rewritten), "usr_guest") +} + +func TestConfigureFromClaimedCliKey_validateFailureDoesNotStartDeviceLogin(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + require.True(t, strings.HasSuffix(r.URL.Path, "/cli-auth/validate")) + w.WriteHeader(http.StatusUnauthorized) + })) + t.Cleanup(ts.Close) + + cfg := &configpkg.Config{ + APIBaseURL: ts.URL, + LogLevel: "error", + TelemetryDisabled: true, + } + cfg.Profile = configpkg.Profile{ + Name: "default", + GuestURL: "https://console.test/guest", + } + + err := ConfigureFromClaimedCliKey(cfg, "hk_test_invalid_abcdefghij") + require.Error(t, err) +} diff --git a/pkg/login/client_login.go b/pkg/login/client_login.go index eeac0b9c..996185b2 100644 --- a/pkg/login/client_login.go +++ b/pkg/login/client_login.go @@ -5,6 +5,7 @@ import ( "io" "net/url" "os" + "time" log "github.com/sirupsen/logrus" @@ -20,12 +21,13 @@ import ( var openBrowser = open.Browser var canOpenBrowser = open.CanOpenBrowser +const guestUpgradePollInterval = 2 * time.Second +const guestUpgradeMaxAttempts = 2 * 60 + // Login function is used to obtain credentials via hookdeck dashboard. func Login(config *configpkg.Config, input io.Reader) error { var s *spinner.Spinner - saved_guest_api_key := "" - if config.Profile.APIKey != "" { log.WithFields(log.Fields{ "prefix": "login.Login", @@ -41,11 +43,8 @@ func Login(config *configpkg.Config, input io.Reader) error { // Rejected key: continue into browser login below (must clear key first // or we would re-enter this branch only). fmt.Fprintln(os.Stdout, "Your saved API key is no longer valid. Starting browser sign-in...") - if config.Profile.GuestURL != "" { - saved_guest_api_key = config.Profile.APIKey - } config.Profile.APIKey = "" - } else if config.Profile.GuestURL == "" { + } else if config.Profile.GuestURL == "" || !response.UserIsGuest { message := SuccessMessage(response.UserName, response.UserEmail, response.OrganizationName, response.ProjectName, response.ProjectMode == "console") ansi.StopSpinner(s, message, os.Stdout) @@ -60,8 +59,8 @@ func Login(config *configpkg.Config, input io.Reader) error { return nil } else { - // Guest Console profile: valid key still needs browser signup to claim sandbox. ansi.StopSpinner(s, "", os.Stdout) + return waitForGuestUpgrade(config, input) } } @@ -75,7 +74,7 @@ func Login(config *configpkg.Config, input io.Reader) error { TelemetryDisabled: config.TelemetryDisabled, } - session, err := client.StartLogin(buildStartLoginInput(config, saved_guest_api_key)) + session, err := client.StartLogin(config.DeviceName) if err != nil { return err } @@ -114,7 +113,7 @@ func waitForLoginSession(config *configpkg.Config, input io.Reader, session *hoo return err } - config.Profile.ApplyPollAPIKeyResponse(response, "", "") + config.Profile.ApplyPollAPIKeyResponse(response, "") if err = config.Profile.SaveProfile(); err != nil { return err @@ -158,7 +157,7 @@ func GuestLogin(config *configpkg.Config) (string, error) { return "", err } - config.Profile.ApplyPollAPIKeyResponse(response, session.GuestURL, session.GuestUserID) + config.Profile.ApplyPollAPIKeyResponse(response, session.GuestURL) if err = config.Profile.SaveProfile(); err != nil { return "", err @@ -225,19 +224,63 @@ func isSSH() bool { return false } -func guestCredentialsUserID(config *configpkg.Config) string { - if config == nil || config.Profile.GuestURL == "" { - return "" +func waitForGuestUpgrade(config *configpkg.Config, input io.Reader) error { + guestURL := RefreshGuestSigninLink(config) + if guestURL == "" { + return fmt.Errorf("unable to create guest sign-up link") } - return config.Profile.GuestUserID -} -func guestCredentialsAPIKey(config *configpkg.Config, saved_guest_api_key string) string { - if config == nil || config.Profile.GuestURL == "" { - return "" + var s *spinner.Spinner + if isSSH() || !canOpenBrowser() { + fmt.Printf("To create a permanent Hookdeck account, please go to: %s\n", guestURL) + s = ansi.StartNewSpinner("Waiting for account creation...", os.Stdout) + } else { + fmt.Printf("Press Enter to open the browser (^C to quit)") + fmt.Fscanln(input) + + s = ansi.StartNewSpinner("Waiting for account creation...", os.Stdout) + + err := openBrowser(guestURL) + if err != nil { + msg := fmt.Sprintf("Failed to open browser, please go to %s manually.", guestURL) + ansi.StopSpinner(s, msg, os.Stdout) + s = ansi.StartNewSpinner("Waiting for account creation...", os.Stdout) + } } - if config.Profile.APIKey != "" { - return config.Profile.APIKey + + response, err := waitForGuestUpgradeCompletion(config) + if err != nil { + return err } - return saved_guest_api_key + + config.Profile.ApplyValidateAPIKeyResponse(response, true) + + if err = config.Profile.SaveProfile(); err != nil { + return err + } + if err = config.Profile.UseProfile(); err != nil { + return err + } + + config.RefreshCachedAPIClient() + + message := SuccessMessage(response.UserName, response.UserEmail, response.OrganizationName, response.ProjectName, response.ProjectMode == "console") + ansi.StopSpinner(s, message, os.Stdout) + + return nil +} + +func waitForGuestUpgradeCompletion(config *configpkg.Config) (*hookdeck.ValidateAPIKeyResponse, error) { + for attempt := 0; attempt < guestUpgradeMaxAttempts; attempt++ { + response, err := config.GetAPIClient().ValidateAPIKey() + if err != nil { + return nil, err + } + if !response.UserIsGuest { + return response, nil + } + time.Sleep(guestUpgradePollInterval) + } + + return nil, fmt.Errorf("exceeded max attempts waiting for guest account creation") } diff --git a/pkg/login/client_login_test.go b/pkg/login/client_login_test.go index e187c710..3913b225 100644 --- a/pkg/login/client_login_test.go +++ b/pkg/login/client_login_test.go @@ -120,10 +120,10 @@ api_key = "hk_test_oldkey_abcdefghij" require.Equal(t, "hk_test_newkey_abcdefghij", cfg.Profile.APIKey) } -// TestLogin_guestProfileWithValidKeyStartsClaimFlow verifies that a guest Console -// profile with a still-valid API key skips the early-return path and POSTs guest -// credentials to /cli-auth for browser signup claim. -func TestLogin_guestProfileWithValidKeyStartsClaimFlow(t *testing.T) { +// TestLogin_guestProfileWithValidKeyStartsGuestUpgrade verifies that a guest Console +// profile with a still-valid API key opens a refreshed guest signup link and waits for +// the same key to validate as a permanent user. +func TestLogin_guestProfileWithValidKeyStartsGuestUpgrade(t *testing.T) { configpkg.ResetAPIClientForTesting() t.Cleanup(configpkg.ResetAPIClientForTesting) @@ -136,56 +136,47 @@ func TestLogin_guestProfileWithValidKeyStartsClaimFlow(t *testing.T) { openBrowser = oldOpen }) - var cli_auth_body map[string]string - pollHits := 0 - var serverURL string + validateHits := 0 + guestRefreshHits := 0 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/cli-auth/validate"): - body, err := json.Marshal(map[string]string{ + validateHits++ + resp := map[string]interface{}{ "user_id": "usr_guest", "user_name": "Guest", "user_email": "guest@example.com", + "user_is_guest": validateHits == 1, "organization_name": "Org", "organization_id": "org_1", "team_id": "tm_console", "team_name_no_org": "Sandbox", "team_mode": "console", "client_id": "cl_guest", - }) - require.NoError(t, err) - _, _ = w.Write(body) - case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli-auth"): - require.NoError(t, json.NewDecoder(r.Body).Decode(&cli_auth_body)) - pollURL := serverURL + hookdeck.APIPathPrefix + "/cli-auth/poll?key=pollkey" - body, err := json.Marshal(map[string]string{ - "browser_url": "https://example.test/signup?redirect=%2Fcli-auth%2Fkey", - "poll_url": pollURL, - }) - require.NoError(t, err) - _, _ = w.Write(body) - case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/cli-auth/poll"): - pollHits++ - resp := map[string]interface{}{ - "claimed": true, - "key": "hk_test_claimed_abcdefghij", - "team_id": "tm_console", - "team_mode": "console", - "team_name": "Sandbox", - "user_name": "Guest", - "user_email": "claimed@example.com", - "organization_name": "Org", - "organization_id": "org_1", - "client_id": "cl_claimed", } enc, err := json.Marshal(resp) require.NoError(t, err) _, _ = w.Write(enc) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest"): + guestRefreshHits++ + user, pass, ok := r.BasicAuth() + require.True(t, ok) + require.Equal(t, "hk_test_guestkey_abcdefghij", user) + require.Empty(t, pass) + var body map[string]string + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + require.Equal(t, "signup", body["link_context"]) + enc, err := json.Marshal(map[string]string{ + "id": "usr_guest", + "key": "hk_test_guestkey_abcdefghij", + "link": "https://example.test/signin/guest?token=fresh&redirect=signup", + }) + require.NoError(t, err) + _, _ = w.Write(enc) default: t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) } })) - serverURL = ts.URL t.Cleanup(ts.Close) configPath := filepath.Join(t.TempDir(), "config.toml") @@ -194,7 +185,6 @@ func TestLogin_guestProfileWithValidKeyStartsClaimFlow(t *testing.T) { [default] api_key = "hk_test_guestkey_abcdefghij" guest_url = "https://console.test/signin/guest?token=abc" -guest_user_id = "usr_guest" `), 0o600)) cfg, err := configpkg.LoadConfigFromFile(configPath) @@ -206,8 +196,8 @@ guest_user_id = "usr_guest" err = Login(cfg, strings.NewReader("\n")) require.NoError(t, err) - require.Equal(t, "usr_guest", cli_auth_body["guest_user_id"]) - require.Equal(t, "hk_test_guestkey_abcdefghij", cli_auth_body["guest_api_key"]) - require.Equal(t, 1, pollHits) - require.Equal(t, "hk_test_claimed_abcdefghij", cfg.Profile.APIKey) + require.Equal(t, 2, validateHits) + require.Equal(t, 1, guestRefreshHits) + require.Equal(t, "hk_test_guestkey_abcdefghij", cfg.Profile.APIKey) + require.Empty(t, cfg.Profile.GuestURL) } diff --git a/pkg/login/guest_link.go b/pkg/login/guest_link.go index 8a1997f6..749da340 100644 --- a/pkg/login/guest_link.go +++ b/pkg/login/guest_link.go @@ -1,36 +1,31 @@ -package login - -import ( - configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" - log "github.com/sirupsen/logrus" -) - -// RefreshGuestSigninLink mints a fresh guest sign-in URL for the active guest profile. -// On failure, returns the existing guest URL and logs a warning. -func RefreshGuestSigninLink(config *configpkg.Config) string { - if config == nil || config.Profile.GuestURL == "" || config.Profile.APIKey == "" { - return "" - } - - response, err := config.GetAPIClient().RefreshGuestSigninLink() - if err != nil { - log.WithError(err).Warn("Failed to refresh guest sign-in link; using saved URL") - return config.Profile.GuestURL - } - if response.Url == "" { - log.Warn("Guest sign-in link refresh returned empty link; using saved URL") - return config.Profile.GuestURL - } - - config.Profile.GuestURL = response.Url - // Id is the guest user id (core POST /cli/guest/signin-link returns req.context.user.id). - if response.Id != "" { - config.Profile.GuestUserID = response.Id - } - - if err := config.Profile.SaveProfile(); err != nil { - log.WithError(err).Warn("Refreshed guest sign-in link but failed to save profile") - } - - return config.Profile.GuestURL -} +package login + +import ( + configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" + log "github.com/sirupsen/logrus" +) + +// RefreshGuestSigninLink mints a fresh guest sign-in URL for the active guest profile. +// On failure, returns the existing guest URL and logs a warning. +func RefreshGuestSigninLink(config *configpkg.Config) string { + if config == nil || config.Profile.GuestURL == "" || config.Profile.APIKey == "" { + return "" + } + + response, err := config.GetAPIClient().RefreshGuestSigninLink() + if err != nil { + log.WithError(err).Warn("Failed to refresh guest sign-in link; using saved URL") + return config.Profile.GuestURL + } + if response.Url == "" { + log.Warn("Guest sign-in link refresh returned empty link; using saved URL") + return config.Profile.GuestURL + } + + config.Profile.GuestURL = response.Url + if err := config.Profile.SaveProfile(); err != nil { + log.WithError(err).Warn("Refreshed guest sign-in link but failed to save profile") + } + + return config.Profile.GuestURL +} diff --git a/pkg/login/guest_link_test.go b/pkg/login/guest_link_test.go index be1f420f..a45359c6 100644 --- a/pkg/login/guest_link_test.go +++ b/pkg/login/guest_link_test.go @@ -1,166 +1,160 @@ -package login - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "strings" - "testing" - - configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" - "github.com/stretchr/testify/require" -) - -func TestRefreshGuestSigninLink_updatesProfileOnSuccess(t *testing.T) { - configpkg.ResetAPIClientForTesting() - t.Cleanup(configpkg.ResetAPIClientForTesting) - - signin_link_hits := 0 - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest/signin-link") { - signin_link_hits++ - user, pass, ok := r.BasicAuth() - require.True(t, ok) - require.Equal(t, "hk_test_guest_refresh_key12", user) - require.Empty(t, pass) - body, err := json.Marshal(map[string]string{ - "id": "usr_guest_refresh", - "link": "https://api.example.test/signin/guest?token=fresh_token", - "expires_at": "2099-01-01T00:00:00.000Z", - }) - require.NoError(t, err) - _, _ = w.Write(body) - return - } - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) - })) - t.Cleanup(ts.Close) - - configPath := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" - -[default] -api_key = "hk_test_guest_refresh_key12" -guest_url = "https://api.example.test/signin/guest?token=stale_token" -guest_user_id = "usr_guest_old" -`), 0o600)) - - cfg, err := configpkg.LoadConfigFromFile(configPath) - require.NoError(t, err) - cfg.APIBaseURL = ts.URL - cfg.LogLevel = "error" - cfg.TelemetryDisabled = true - - got := RefreshGuestSigninLink(cfg) - require.Equal(t, 1, signin_link_hits) - require.Equal(t, "https://api.example.test/signin/guest?token=fresh_token", got) - require.Equal(t, got, cfg.Profile.GuestURL) - require.Equal(t, "usr_guest_refresh", cfg.Profile.GuestUserID) - - reloaded, err := os.ReadFile(configPath) - require.NoError(t, err) - require.Contains(t, string(reloaded), "fresh_token") - require.Contains(t, string(reloaded), "usr_guest_refresh") -} - -func TestRefreshGuestSigninLink_fallsBackToSavedURLOnAPIError(t *testing.T) { - configpkg.ResetAPIClientForTesting() - t.Cleanup(configpkg.ResetAPIClientForTesting) - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest/signin-link") { - w.WriteHeader(http.StatusInternalServerError) - _, _ = w.Write([]byte(`{"message":"server boom"}`)) - return - } - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) - })) - t.Cleanup(ts.Close) - - saved_url := "https://api.example.test/signin/guest?token=stale_token" - configPath := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" - -[default] -api_key = "hk_test_guest_refresh_key12" -guest_url = "`+saved_url+`" -guest_user_id = "usr_guest_old" -`), 0o600)) - - cfg, err := configpkg.LoadConfigFromFile(configPath) - require.NoError(t, err) - cfg.APIBaseURL = ts.URL - cfg.LogLevel = "error" - cfg.TelemetryDisabled = true - - got := RefreshGuestSigninLink(cfg) - require.Equal(t, saved_url, got) - require.Equal(t, saved_url, cfg.Profile.GuestURL) - require.Equal(t, "usr_guest_old", cfg.Profile.GuestUserID) -} - -func TestRefreshGuestSigninLink_preservesSavedURLOnEmptyLink(t *testing.T) { - configpkg.ResetAPIClientForTesting() - t.Cleanup(configpkg.ResetAPIClientForTesting) - - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest/signin-link") { - body, err := json.Marshal(map[string]string{ - "id": "usr_guest_refresh", - "link": "", - "expires_at": "2099-01-01T00:00:00.000Z", - }) - require.NoError(t, err) - _, _ = w.Write(body) - return - } - t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) - })) - t.Cleanup(ts.Close) - - saved_url := "https://api.example.test/signin/guest?token=stale_token" - configPath := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" - -[default] -api_key = "hk_test_guest_refresh_key12" -guest_url = "`+saved_url+`" -guest_user_id = "usr_guest_old" -`), 0o600)) - - cfg, err := configpkg.LoadConfigFromFile(configPath) - require.NoError(t, err) - cfg.APIBaseURL = ts.URL - cfg.LogLevel = "error" - cfg.TelemetryDisabled = true - - got := RefreshGuestSigninLink(cfg) - require.Equal(t, saved_url, got) - require.Equal(t, saved_url, cfg.Profile.GuestURL) - require.Equal(t, "usr_guest_old", cfg.Profile.GuestUserID) - - reloaded, err := os.ReadFile(configPath) - require.NoError(t, err) - require.Contains(t, string(reloaded), "stale_token") - require.Contains(t, string(reloaded), "usr_guest_old") -} - -func TestRefreshGuestSigninLink_returnsEmptyWithoutGuestProfile(t *testing.T) { - cfg := &configpkg.Config{ - LogLevel: "error", - TelemetryDisabled: true, - } - cfg.Profile = configpkg.Profile{ - Name: "default", - APIKey: "hk_test_guest_refresh_key12", - Config: cfg, - } - - require.Empty(t, RefreshGuestSigninLink(cfg)) - - cfg.Profile.GuestURL = "https://example.test/signin/guest?token=x" - cfg.Profile.APIKey = "" - require.Empty(t, RefreshGuestSigninLink(cfg)) -} +package login + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" + "github.com/stretchr/testify/require" +) + +func TestRefreshGuestSigninLink_updatesProfileOnSuccess(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + signin_link_hits := 0 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest") { + signin_link_hits++ + user, pass, ok := r.BasicAuth() + require.True(t, ok) + require.Equal(t, "hk_test_guest_refresh_key12", user) + require.Empty(t, pass) + var input map[string]string + require.NoError(t, json.NewDecoder(r.Body).Decode(&input)) + require.Equal(t, "signup", input["link_context"]) + body, err := json.Marshal(map[string]string{ + "id": "usr_guest_refresh", + "key": "hk_test_guest_refresh_key12", + "link": "https://api.example.test/signin/guest?token=fresh_token", + }) + require.NoError(t, err) + _, _ = w.Write(body) + return + } + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(ts.Close) + + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" + +[default] +api_key = "hk_test_guest_refresh_key12" +guest_url = "https://api.example.test/signin/guest?token=stale_token" +`), 0o600)) + + cfg, err := configpkg.LoadConfigFromFile(configPath) + require.NoError(t, err) + cfg.APIBaseURL = ts.URL + cfg.LogLevel = "error" + cfg.TelemetryDisabled = true + + got := RefreshGuestSigninLink(cfg) + require.Equal(t, 1, signin_link_hits) + require.Equal(t, "https://api.example.test/signin/guest?token=fresh_token", got) + require.Equal(t, got, cfg.Profile.GuestURL) + + reloaded, err := os.ReadFile(configPath) + require.NoError(t, err) + require.Contains(t, string(reloaded), "fresh_token") +} + +func TestRefreshGuestSigninLink_fallsBackToSavedURLOnAPIError(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest") { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"message":"server boom"}`)) + return + } + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(ts.Close) + + saved_url := "https://api.example.test/signin/guest?token=stale_token" + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" + +[default] +api_key = "hk_test_guest_refresh_key12" +guest_url = "`+saved_url+`" +`), 0o600)) + + cfg, err := configpkg.LoadConfigFromFile(configPath) + require.NoError(t, err) + cfg.APIBaseURL = ts.URL + cfg.LogLevel = "error" + cfg.TelemetryDisabled = true + + got := RefreshGuestSigninLink(cfg) + require.Equal(t, saved_url, got) + require.Equal(t, saved_url, cfg.Profile.GuestURL) +} + +func TestRefreshGuestSigninLink_preservesSavedURLOnEmptyLink(t *testing.T) { + configpkg.ResetAPIClientForTesting() + t.Cleanup(configpkg.ResetAPIClientForTesting) + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest") { + body, err := json.Marshal(map[string]string{ + "id": "usr_guest_refresh", + "link": "", + }) + require.NoError(t, err) + _, _ = w.Write(body) + return + } + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(ts.Close) + + saved_url := "https://api.example.test/signin/guest?token=stale_token" + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(`profile = "default" + +[default] +api_key = "hk_test_guest_refresh_key12" +guest_url = "`+saved_url+`" +`), 0o600)) + + cfg, err := configpkg.LoadConfigFromFile(configPath) + require.NoError(t, err) + cfg.APIBaseURL = ts.URL + cfg.LogLevel = "error" + cfg.TelemetryDisabled = true + + got := RefreshGuestSigninLink(cfg) + require.Equal(t, saved_url, got) + require.Equal(t, saved_url, cfg.Profile.GuestURL) + + reloaded, err := os.ReadFile(configPath) + require.NoError(t, err) + require.Contains(t, string(reloaded), "stale_token") +} + +func TestRefreshGuestSigninLink_returnsEmptyWithoutGuestProfile(t *testing.T) { + cfg := &configpkg.Config{ + LogLevel: "error", + TelemetryDisabled: true, + } + cfg.Profile = configpkg.Profile{ + Name: "default", + APIKey: "hk_test_guest_refresh_key12", + Config: cfg, + } + + require.Empty(t, RefreshGuestSigninLink(cfg)) + + cfg.Profile.GuestURL = "https://example.test/signin/guest?token=x" + cfg.Profile.APIKey = "" + require.Empty(t, RefreshGuestSigninLink(cfg)) +} diff --git a/pkg/login/interactive_login.go b/pkg/login/interactive_login.go index 80da0272..aeaf0b2d 100644 --- a/pkg/login/interactive_login.go +++ b/pkg/login/interactive_login.go @@ -55,7 +55,7 @@ func InteractiveLogin(config *configpkg.Config) error { return err } - config.Profile.ApplyPollAPIKeyResponse(response, "", "") + config.Profile.ApplyPollAPIKeyResponse(response, "") if err = config.Profile.SaveProfile(); err != nil { ansi.StopSpinner(s, "", os.Stdout) diff --git a/pkg/login/login_intent.go b/pkg/login/login_intent.go deleted file mode 100644 index 10cf1b86..00000000 --- a/pkg/login/login_intent.go +++ /dev/null @@ -1,29 +0,0 @@ -package login - -import ( - configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" - "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" -) - -func BuildStartLoginInput( - config *configpkg.Config, - saved_guest_api_key string, -) hookdeck.StartLoginInput { - return buildStartLoginInput(config, saved_guest_api_key) -} - -func buildStartLoginInput( - config *configpkg.Config, - saved_guest_api_key string, -) hookdeck.StartLoginInput { - input := hookdeck.StartLoginInput{ - DeviceName: config.DeviceName, - } - - if config != nil && config.Profile.GuestURL != "" { - input.GuestUserID = guestCredentialsUserID(config) - input.GuestAPIKey = guestCredentialsAPIKey(config, saved_guest_api_key) - } - - return input -} diff --git a/pkg/login/login_intent_test.go b/pkg/login/login_intent_test.go deleted file mode 100644 index ee7f39ca..00000000 --- a/pkg/login/login_intent_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package login - -import ( - "testing" - - configpkg "github.com/hookdeck/hookdeck-cli/pkg/config" - "github.com/stretchr/testify/require" -) - -func TestBuildStartLoginInput_guestProfileIncludesCredentials(t *testing.T) { - cfg := &configpkg.Config{ - DeviceName: "dev", - Profile: configpkg.Profile{ - GuestURL: "https://console.test/signin/guest?token=abc", - GuestUserID: "usr_guest", - APIKey: "hk_guest_key", - }, - } - - input := buildStartLoginInput(cfg, "") - require.Equal(t, "usr_guest", input.GuestUserID) - require.Equal(t, "hk_guest_key", input.GuestAPIKey) -} - -func TestBuildStartLoginInput_noGuestProfileOmitsCredentials(t *testing.T) { - cfg := &configpkg.Config{ - DeviceName: "dev", - Profile: configpkg.Profile{}, - } - - input := buildStartLoginInput(cfg, "") - require.Empty(t, input.GuestUserID) - require.Empty(t, input.GuestAPIKey) -} diff --git a/test/acceptance/guest_login_acceptance_test.go b/test/acceptance/guest_login_acceptance_test.go index a5b599f5..199c238f 100644 --- a/test/acceptance/guest_login_acceptance_test.go +++ b/test/acceptance/guest_login_acceptance_test.go @@ -1,161 +1,200 @@ -//go:build guest - -package acceptance - -import ( - "bytes" - "context" - "encoding/json" - "io" - "net/http" - "net/http/httptest" - "os" - "os/exec" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -func runGuestLoginCLI(t *testing.T, projectRoot, configPath, serverURL string, extraArgs ...string) { - t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) - defer cancel() - - mainGo := filepath.Join(projectRoot, "main.go") - args := append([]string{"run", mainGo, - "--api-base", serverURL, - "--hookdeck-config", configPath, - "--log-level", "error", - "login", - }, extraArgs...) - - cmd := exec.CommandContext(ctx, "go", args...) - cmd.Dir = projectRoot - env := appendEnvOverride(os.Environ(), "HOOKDECK_CONFIG_FILE", configPath) - env = appendEnvOverride(env, "SSH_CONNECTION", "acceptance-guest-login-mock") - env = appendEnvOverride(env, "HOOKDECK_CLI_TELEMETRY_DISABLED", "1") - cmd.Stdin = strings.NewReader("\n") - cmd.Env = env - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - err := cmd.Run() - require.NoError(t, err, "stdout=%q stderr=%q", stdout.String(), stderr.String()) -} - -func newGuestLoginMock(t *testing.T, assertBody func(map[string]interface{}), browserURL string) (*httptest.Server, string) { - t.Helper() - var serverURL string - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/cli-auth/validate"): - w.WriteHeader(http.StatusUnauthorized) - _, _ = w.Write([]byte("Unauthorized")) - case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest"): - body, encErr := json.Marshal(map[string]string{ - "id": "usr_guest_accept", - "key": "hk_guest_accept_key", - "link": "https://example.test/signin/guest?token=guest", - }) - require.NoError(t, encErr) - _, _ = w.Write(body) - case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli-auth"): - raw, readErr := io.ReadAll(r.Body) - require.NoError(t, readErr) - var payload map[string]interface{} - require.NoError(t, json.Unmarshal(raw, &payload)) - assertBody(payload) - pollURL := serverURL + "/2025-07-01/cli-auth/poll?key=pollkey" - respBody, encErr := json.Marshal(map[string]string{ - "browser_url": browserURL, - "poll_url": pollURL, - }) - require.NoError(t, encErr) - _, _ = w.Write(respBody) - case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/cli-auth/poll"): - resp := map[string]interface{}{ - "claimed": true, - "key": "hk_test_guest_claimed", - "team_id": "tm_guest", - "team_mode": "console", - "team_name": "Guest Sandbox", - "user_name": "Guest", - "user_email": "guest@example.com", - "organization_name": "GuestOrg", - "organization_id": "org_guest", - "client_id": "cl_guest", - } - enc, encErr := json.Marshal(resp) - require.NoError(t, encErr) - _, _ = w.Write(enc) - default: - t.Fatalf("unexpected %s %s", r.Method, r.URL.Path) - } - })) - serverURL = ts.URL - t.Cleanup(ts.Close) - return ts, serverURL -} - -func guestProfileConfig() string { - return `profile = "default" - -[default] -api_key = "hk_test_stale_guest01" -guest_user_id = "usr_guest_accept" -guest_url = "https://example.test/signin/guest?token=guest" -` -} - -func loggedOutProfileConfig() string { - return `profile = "default" - -[default] -` -} - -func TestGuestLoginDefaultClaimGuestAcceptance(t *testing.T) { - if testing.Short() { - t.Skip("Skipping acceptance test in short mode") - } - - projectRoot, err := filepath.Abs("../..") - require.NoError(t, err) - - configPath := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configPath, []byte(guestProfileConfig()), 0o600)) - - ts, serverURL := newGuestLoginMock(t, func(body map[string]interface{}) { - require.Equal(t, "usr_guest_accept", body["guest_user_id"]) - require.Equal(t, "hk_test_stale_guest01", body["guest_api_key"]) - require.NotContains(t, body, "auth_intent") - }, "https://example.test/signup?redirect=%2Fcli-auth%2Fkey") - - runGuestLoginCLI(t, projectRoot, configPath, serverURL) - _ = ts -} - -func TestGuestLoginAfterLogoutOmitsGuestCredentialsAcceptance(t *testing.T) { - if testing.Short() { - t.Skip("Skipping acceptance test in short mode") - } - - projectRoot, err := filepath.Abs("../..") - require.NoError(t, err) - - configPath := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configPath, []byte(loggedOutProfileConfig()), 0o600)) - - ts, serverURL := newGuestLoginMock(t, func(body map[string]interface{}) { - require.NotContains(t, body, "auth_intent") - require.NotContains(t, body, "guest_user_id") - require.NotContains(t, body, "guest_api_key") - }, "https://example.test/signin?redirect=%2Fcli-auth%2Fkey") - - runGuestLoginCLI(t, projectRoot, configPath, serverURL) - _ = ts -} +//go:build guest + +package acceptance + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func runGuestLoginCLI(t *testing.T, projectRoot, configPath, serverURL string, extraArgs ...string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) + defer cancel() + + mainGo := filepath.Join(projectRoot, "main.go") + args := append([]string{"run", mainGo, + "--api-base", serverURL, + "--hookdeck-config", configPath, + "--log-level", "error", + "login", + }, extraArgs...) + + cmd := exec.CommandContext(ctx, "go", args...) + cmd.Dir = projectRoot + env := appendEnvOverride(os.Environ(), "HOOKDECK_CONFIG_FILE", configPath) + env = appendEnvOverride(env, "SSH_CONNECTION", "acceptance-guest-login-mock") + env = appendEnvOverride(env, "HOOKDECK_CLI_TELEMETRY_DISABLED", "1") + cmd.Stdin = strings.NewReader("\n") + cmd.Env = env + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + require.NoError(t, err, "stdout=%q stderr=%q", stdout.String(), stderr.String()) +} + +func newGuestLoginMock(t *testing.T, assertBody func(map[string]interface{}), browserURL string) (*httptest.Server, string) { + t.Helper() + var serverURL string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/cli-auth/validate"): + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte("Unauthorized")) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest"): + body, encErr := json.Marshal(map[string]string{ + "id": "usr_guest_accept", + "key": "hk_guest_accept_key", + "link": "https://example.test/signin/guest?token=guest", + }) + require.NoError(t, encErr) + _, _ = w.Write(body) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli-auth"): + raw, readErr := io.ReadAll(r.Body) + require.NoError(t, readErr) + var payload map[string]interface{} + require.NoError(t, json.Unmarshal(raw, &payload)) + assertBody(payload) + pollURL := serverURL + "/2025-07-01/cli-auth/poll?key=pollkey" + respBody, encErr := json.Marshal(map[string]string{ + "browser_url": browserURL, + "poll_url": pollURL, + }) + require.NoError(t, encErr) + _, _ = w.Write(respBody) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/cli-auth/poll"): + resp := map[string]interface{}{ + "claimed": true, + "key": "hk_test_guest_claimed", + "team_id": "tm_guest", + "team_mode": "console", + "team_name": "Guest Sandbox", + "user_name": "Guest", + "user_email": "guest@example.com", + "organization_name": "GuestOrg", + "organization_id": "org_guest", + "client_id": "cl_guest", + } + enc, encErr := json.Marshal(resp) + require.NoError(t, encErr) + _, _ = w.Write(enc) + default: + t.Fatalf("unexpected %s %s", r.Method, r.URL.Path) + } + })) + serverURL = ts.URL + t.Cleanup(ts.Close) + return ts, serverURL +} + +func guestProfileConfig() string { + return `profile = "default" + +[default] +api_key = "hk_test_stale_guest01" +guest_url = "https://example.test/signin/guest?token=guest" +` +} + +func loggedOutProfileConfig() string { + return `profile = "default" + +[default] +` +} + +func TestGuestLoginDefaultClaimGuestAcceptance(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + projectRoot, err := filepath.Abs("../..") + require.NoError(t, err) + + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(guestProfileConfig()), 0o600)) + + validateHits := 0 + guestRefreshHits := 0 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/cli-auth/validate"): + validateHits++ + body, encErr := json.Marshal(map[string]interface{}{ + "user_id": "usr_guest_accept", + "user_name": "Guest", + "user_email": "guest@example.com", + "user_is_guest": validateHits == 1, + "organization_name": "GuestOrg", + "organization_id": "org_guest", + "team_id": "tm_guest", + "team_name_no_org": "Guest Sandbox", + "team_mode": "console", + "client_id": "cl_guest", + }) + require.NoError(t, encErr) + _, _ = w.Write(body) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/cli/guest"): + guestRefreshHits++ + user, pass, ok := r.BasicAuth() + require.True(t, ok) + require.Equal(t, "hk_test_stale_guest01", user) + require.Empty(t, pass) + + var payload map[string]interface{} + require.NoError(t, json.NewDecoder(r.Body).Decode(&payload)) + require.Equal(t, "signup", payload["link_context"]) + + body, encErr := json.Marshal(map[string]string{ + "id": "usr_guest_accept", + "key": "hk_test_stale_guest01", + "link": "https://example.test/signin/guest?token=fresh&redirect=signup", + }) + require.NoError(t, encErr) + _, _ = w.Write(body) + default: + t.Fatalf("unexpected %s %s", r.Method, r.URL.Path) + } + })) + serverURL := ts.URL + t.Cleanup(ts.Close) + + runGuestLoginCLI(t, projectRoot, configPath, serverURL) + require.Equal(t, 2, validateHits) + require.Equal(t, 1, guestRefreshHits) +} + +func TestGuestLoginAfterLogoutOmitsGuestCredentialsAcceptance(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + projectRoot, err := filepath.Abs("../..") + require.NoError(t, err) + + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(loggedOutProfileConfig()), 0o600)) + + ts, serverURL := newGuestLoginMock(t, func(body map[string]interface{}) { + require.NotContains(t, body, "auth_intent") + require.NotContains(t, body, "guest_user_id") + require.NotContains(t, body, "guest_api_key") + }, "https://example.test/signin?redirect=%2Fcli-auth%2Fkey") + + runGuestLoginCLI(t, projectRoot, configPath, serverURL) + _ = ts +} From ee8950e5ae9f30e9d472bebe86856e5724658cea Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Tue, 7 Jul 2026 14:56:35 +0100 Subject: [PATCH 2/2] fix(cli-guest): omit device_name from guest link refresh payload Refresh calls POST /cli/guest with link_context only; empty device_name was rejected by server Joi validation (422). Add omitempty and assert the key is absent in unit and guest acceptance tests. Co-Authored-By: Claude Co-authored-by: Cursor --- pkg/hookdeck/guest.go | 2 +- pkg/login/guest_link_test.go | 1 + test/acceptance/guest_login_acceptance_test.go | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/hookdeck/guest.go b/pkg/hookdeck/guest.go index 1df5debf..04dc133f 100644 --- a/pkg/hookdeck/guest.go +++ b/pkg/hookdeck/guest.go @@ -23,7 +23,7 @@ type GuestSigninLinkResponse struct { } type CreateGuestUserInput struct { - DeviceName string `json:"device_name"` + DeviceName string `json:"device_name,omitempty"` LinkContext string `json:"link_context,omitempty"` } diff --git a/pkg/login/guest_link_test.go b/pkg/login/guest_link_test.go index a45359c6..db5d8bc3 100644 --- a/pkg/login/guest_link_test.go +++ b/pkg/login/guest_link_test.go @@ -28,6 +28,7 @@ func TestRefreshGuestSigninLink_updatesProfileOnSuccess(t *testing.T) { var input map[string]string require.NoError(t, json.NewDecoder(r.Body).Decode(&input)) require.Equal(t, "signup", input["link_context"]) + require.NotContains(t, input, "device_name") body, err := json.Marshal(map[string]string{ "id": "usr_guest_refresh", "key": "hk_test_guest_refresh_key12", diff --git a/test/acceptance/guest_login_acceptance_test.go b/test/acceptance/guest_login_acceptance_test.go index 199c238f..893f7c08 100644 --- a/test/acceptance/guest_login_acceptance_test.go +++ b/test/acceptance/guest_login_acceptance_test.go @@ -158,6 +158,7 @@ func TestGuestLoginDefaultClaimGuestAcceptance(t *testing.T) { var payload map[string]interface{} require.NoError(t, json.NewDecoder(r.Body).Decode(&payload)) require.Equal(t, "signup", payload["link_context"]) + require.NotContains(t, payload, "device_name") body, encErr := json.Marshal(map[string]string{ "id": "usr_guest_accept",