diff --git a/pkg/auth/authenticate.go b/pkg/auth/authenticate.go index af0df076..3ca2f711 100644 --- a/pkg/auth/authenticate.go +++ b/pkg/auth/authenticate.go @@ -23,7 +23,9 @@ func EnsureAuthenticated( cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s %s\n", cs.WarningIcon(), err) - return RunOAuth(io, client, false, true) + // No flow tracker: this re-authentication belongs to the calling flow, + // not to an `auth login` funnel. + return RunOAuth(io, client, false, true, nil) } // ReauthenticateIfExpired checks if err is a session-expired error from the API. @@ -41,5 +43,7 @@ func ReauthenticateIfExpired( ClearToken() fmt.Fprintf(io.Out, "%s Session expired.\n", cs.WarningIcon()) - return RunOAuth(io, client, false, true) + // No flow tracker: this re-authentication belongs to the calling flow, + // not to an `auth login` funnel. + return RunOAuth(io, client, false, true, nil) } diff --git a/pkg/auth/oauth_flow.go b/pkg/auth/oauth_flow.go index a4334f43..31d1ac29 100644 --- a/pkg/auth/oauth_flow.go +++ b/pkg/auth/oauth_flow.go @@ -8,6 +8,7 @@ import ( "github.com/algolia/cli/api/dashboard" "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" ) // DefaultOAuthClientID is a public OAuth client ID (PKCE flow, not a secret). @@ -35,7 +36,15 @@ func OAuthClientID() string { // launched, e.g. SSH / containers). // // If signup is true the browser opens to the sign-up page. -func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBrowser bool) (string, error) { +// +// The optional tracker (nil-safe) records which step the flow is in, so the +// telemetry of the calling flow can tell where the user stopped. +func RunOAuth( + io *iostreams.IOStreams, + client *dashboard.Client, + signup, openBrowser bool, + tracker *telemetry.FlowTracker, +) (string, error) { cs := io.ColorScheme() redirectURI, resultCh, err := StartCallbackServer() @@ -69,6 +78,7 @@ func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBro } fmt.Fprintf(io.Out, "Waiting for authentication...\n") + tracker.SetStep(telemetry.StepBrowserWait) cbResult := <-resultCh if cbResult.Error != "" { @@ -78,6 +88,7 @@ func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBro return "", fmt.Errorf("no authorization code received") } + tracker.SetStep(telemetry.StepCodeExchange) io.StartProgressIndicatorWithLabel("Exchanging code for tokens") tokenResp, err := client.AuthorizationCodeGrant(cbResult.Code, codeVerifier, redirectURI) io.StopProgressIndicator() diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7fdb1461..e3c844ac 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -92,17 +92,54 @@ func runLoginCmd(ctx context.Context, opts *LoginOptions) error { // RunOAuthFlow runs the full browser-based OAuth + profile setup flow. // If signup is true, the browser opens to the sign-up page instead of sign-in. func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { + flow := telemetry.FlowLogin + if signup { + flow = telemetry.FlowSignup + } + tracker := telemetry.NewFlowTracker() + telemetry.TrackEvent(ctx, telemetry.AuthStarted(flow, opts.NoBrowser)) + + err := runOAuthFlowSteps(ctx, opts, signup, tracker) + trackOAuthFlowOutcome(ctx, flow, tracker, err) + return err +} + +// trackOAuthFlowOutcome reports how the auth flow ended: completed, aborted by +// the user, or failed. +func trackOAuthFlowOutcome( + ctx context.Context, + flow telemetry.Flow, + tracker *telemetry.FlowTracker, + err error, +) { + switch { + case err == nil: + telemetry.TrackEvent(ctx, telemetry.AuthCompleted(flow, tracker)) + case cmdutil.IsUserCancellation(err): + telemetry.TrackEvent(ctx, telemetry.AuthAborted(flow, tracker)) + default: + telemetry.TrackEvent(ctx, telemetry.AuthFailed(flow, tracker, err)) + } +} + +func runOAuthFlowSteps( + ctx context.Context, + opts *LoginOptions, + signup bool, + tracker *telemetry.FlowTracker, +) error { cs := opts.IO.ColorScheme() client := opts.NewDashboardClient(auth.OAuthClientID()) openBrowser := !opts.NoBrowser - accessToken, err := auth.RunOAuth(opts.IO, client, signup, openBrowser) + accessToken, err := auth.RunOAuth(opts.IO, client, signup, openBrowser, tracker) if err != nil { return err } identifyAuthenticatedUser(ctx) + tracker.SetStep(telemetry.StepAppsFetch) opts.IO.StartProgressIndicatorWithLabel("Fetching applications") apps, err := client.ListApplications(accessToken) opts.IO.StopProgressIndicator() @@ -115,11 +152,13 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { if len(apps) == 0 { fmt.Fprintf(opts.IO.Out, "\n%s No applications found. Let's create one.\n", cs.WarningIcon()) + tracker.SetStep(telemetry.StepAppCreate) appDetails, err = apputil.CreateAndFetchApplication(opts.IO, client, accessToken, "", opts.AppName) if err != nil { return err } } else { + tracker.SetStep(telemetry.StepAppSelect) interactive := opts.IO.CanPrompt() app, err := selectApplication(opts, apps, interactive) if err != nil { @@ -139,15 +178,21 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { profileName = appDetails.Name } + tracker.SetStep(telemetry.StepProfileConfigure) return apputil.ConfigureProfile(opts.IO, opts.Config, appDetails, profileName, opts.Default) } // identifyAuthenticatedUser emits a telemetry Identify for the user that just // authenticated. It is a no-op when no identified token is present. func identifyAuthenticatedUser(ctx context.Context) { - if applyStoredIdentity(ctx) { - telemetry.IdentifyOnce(ctx) + if !applyStoredIdentity(ctx) { + return + } + client := telemetry.GetTelemetryClient(ctx) + if client == nil { + return } + _ = client.Identify(ctx) } // applyStoredIdentity copies the persisted user identity from the stored token diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index ed818558..a1a45d70 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -2,6 +2,7 @@ package login import ( "context" + "errors" "testing" "github.com/stretchr/testify/assert" @@ -13,6 +14,7 @@ import ( "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/iostreams" "github.com/algolia/cli/pkg/telemetry" + "github.com/algolia/cli/pkg/telemetry/telemetrytest" "github.com/algolia/cli/test" ) @@ -130,6 +132,52 @@ func TestApplyStoredIdentity_TokenWithoutIdentityReturnsFalse(t *testing.T) { assert.Empty(t, metadata.UserID) } +func TestTrackOAuthFlowOutcome(t *testing.T) { + tests := []struct { + name string + err error + wantEvent string + wantStep bool + }{ + { + name: "success", + err: nil, + wantEvent: telemetry.EventAuthCompleted, + }, + { + name: "user cancellation", + err: cmdutil.ErrCancel, + wantEvent: telemetry.EventAuthAborted, + wantStep: true, + }, + { + name: "failure", + err: errors.New("boom"), + wantEvent: telemetry.EventAuthFailed, + wantStep: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &telemetrytest.RecordingClient{} + ctx := telemetry.WithTelemetryClient(context.Background(), client) + tracker := telemetry.NewFlowTracker() + tracker.SetStep(telemetry.StepAppsFetch) + + trackOAuthFlowOutcome(ctx, telemetry.FlowLogin, tracker, tt.err) + + require.Len(t, client.Events, 1) + event := client.Events[0] + assert.Equal(t, tt.wantEvent, event.Name) + assert.Equal(t, telemetry.FlowLogin, event.Properties["flow"]) + if tt.wantStep { + assert.Equal(t, telemetry.StepAppsFetch, event.Properties["step"]) + } + }) + } +} + func TestSelectApplication_MultipleApps_NonInteractive_NoAppName(t *testing.T) { io, _, _, _ := iostreams.Test() opts := &LoginOptions{IO: io} diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index 6205dc3e..1b0c7b7b 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -1,6 +1,7 @@ package logout import ( + "context" "fmt" "github.com/MakeNowJust/heredoc" @@ -10,6 +11,7 @@ import ( "github.com/algolia/cli/pkg/auth" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" "github.com/algolia/cli/pkg/validators" ) @@ -38,14 +40,14 @@ func NewLogoutCmd(f *cmdutil.Factory) *cobra.Command { `), Args: validators.NoArgs(), RunE: func(cmd *cobra.Command, args []string) error { - return runLogoutCmd(opts) + return runLogoutCmd(cmd.Context(), opts) }, } return cmd } -func runLogoutCmd(opts *LogoutOptions) error { +func runLogoutCmd(ctx context.Context, opts *LogoutOptions) error { cs := opts.IO.ColorScheme() stored := auth.LoadToken() @@ -68,6 +70,10 @@ func runLogoutCmd(opts *LogoutOptions) error { } } + // Tracked before clearing the local state, while the user identifier is + // still attached to the telemetry metadata. + telemetry.TrackEvent(ctx, telemetry.AuthLogout()) + auth.ClearToken() fmt.Fprintf(opts.IO.Out, "%s Signed out successfully.\n", cs.SuccessIcon()) diff --git a/pkg/cmd/auth/logout/logout_test.go b/pkg/cmd/auth/logout/logout_test.go new file mode 100644 index 00000000..dc75bd27 --- /dev/null +++ b/pkg/cmd/auth/logout/logout_test.go @@ -0,0 +1,73 @@ +package logout + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/auth" + "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" + "github.com/algolia/cli/pkg/telemetry/telemetrytest" +) + +func newLogoutOpts(srv *httptest.Server) *LogoutOptions { + io, _, _, _ := iostreams.Test() + return &LogoutOptions{ + IO: io, + NewDashboardClient: func(string) *dashboard.Client { + c := dashboard.NewClientWithHTTPClient("test", srv.Client()) + c.DashboardURL = srv.URL + return c + }, + } +} + +func TestLogout_EmitsAuthLogoutBeforeClearingToken(t *testing.T) { + keyring.MockInit() + t.Cleanup(auth.ClearToken) + require.NoError(t, auth.SaveToken(&dashboard.OAuthTokenResponse{ + AccessToken: "access", + RefreshToken: "refresh", + ExpiresIn: 3600, + })) + + mux := http.NewServeMux() + mux.HandleFunc("/2/oauth/revoke", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + client := &telemetrytest.RecordingClient{} + ctx := telemetry.WithTelemetryClient(context.Background(), client) + + require.NoError(t, runLogoutCmd(ctx, newLogoutOpts(srv))) + + require.Len(t, client.Events, 1) + event := client.Events[0] + assert.Equal(t, telemetry.EventAuthLogout, event.Name) + assert.Equal(t, telemetry.FlowLogout, event.Properties["flow"]) + // The token is cleared after the event was tracked. + assert.Nil(t, auth.LoadToken()) + // No Identify at logout time: Segment cannot un-identify. + assert.Zero(t, client.Identifies) +} + +func TestLogout_AlreadySignedOutEmitsNothing(t *testing.T) { + keyring.MockInit() + auth.ClearToken() + + client := &telemetrytest.RecordingClient{} + ctx := telemetry.WithTelemetryClient(context.Background(), client) + + require.NoError(t, runLogoutCmd(ctx, newLogoutOpts(nil))) + + assert.Empty(t, client.Events) +} diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index 6d18201f..82fccbd6 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -1,6 +1,7 @@ package telemetry import ( + "context" "errors" "fmt" "time" @@ -17,6 +18,7 @@ const ( EventAuthCompleted = "CLI Auth Completed" EventAuthFailed = "CLI Auth Failed" EventAuthAborted = "CLI Auth Aborted" + EventAuthLogout = "CLI Auth Logout" EventApplicationCreateStarted = "CLI Application Create Started" EventApplicationCreateAcceptedTerms = "CLI Application Create Accepted Terms" @@ -39,6 +41,7 @@ type Flow string const ( FlowLogin Flow = "login" FlowSignup Flow = "signup" + FlowLogout Flow = "logout" ) // Step locates where the user is inside an interactive flow, so aborts and @@ -108,6 +111,68 @@ func (f *FlowTracker) DurationMS() int64 { return time.Since(f.start).Milliseconds() } +// Event is a fully assembled telemetry event, ready to be tracked. +type Event struct { + Name string + Properties map[string]any +} + +// TrackEvent sends the event through the context's telemetry client, silently +// doing nothing when no client is present. +func TrackEvent(ctx context.Context, event Event) { + client := GetTelemetryClient(ctx) + if client == nil { + return + } + _ = client.Track(ctx, event.Name, event.Properties) +} + +// AuthStarted is emitted when the browser-based OAuth flow begins. +func AuthStarted(flow Flow, noBrowser bool) Event { + return Event{EventAuthStarted, map[string]any{ + "flow": flow, + "no_browser": noBrowser, + }} +} + +// AuthCompleted is emitted when the profile is fully configured at the end of +// the auth flow. +func AuthCompleted(flow Flow, tracker *FlowTracker) Event { + return Event{EventAuthCompleted, map[string]any{ + "flow": flow, + "duration_ms": tracker.DurationMS(), + }} +} + +// AuthAborted is emitted when the user cancelled the auth flow, with the step +// they stopped at. +func AuthAborted(flow Flow, tracker *FlowTracker) Event { + return Event{EventAuthAborted, map[string]any{ + "flow": flow, + "step": tracker.Step(), + }} +} + +// AuthLogout is emitted when the user signs out. It must be tracked before +// the local state is cleared, while the user identifier is still attached to +// the telemetry metadata; no Identify follows, since Segment's identity graph +// has no concept of un-identifying. +func AuthLogout() Event { + return Event{EventAuthLogout, map[string]any{ + "flow": FlowLogout, + }} +} + +// AuthFailed is emitted when the auth flow failed, with the step it failed at. +func AuthFailed(flow Flow, tracker *FlowTracker, err error) Event { + return Event{EventAuthFailed, map[string]any{ + "flow": flow, + "step": tracker.Step(), + "duration_ms": tracker.DurationMS(), + "error_class": ErrorClass(err), + }} +} + // ErrorClass returns the type of the first informative error of the chain, // skipping the anonymous wrappers created by fmt.Errorf. It never returns an // error message, which could contain user data. diff --git a/pkg/telemetry/events_test.go b/pkg/telemetry/events_test.go index 61c5a9ee..7127d30e 100644 --- a/pkg/telemetry/events_test.go +++ b/pkg/telemetry/events_test.go @@ -1,6 +1,8 @@ package telemetry import ( + "context" + "errors" "fmt" "testing" @@ -30,6 +32,45 @@ func TestErrorClass_NilError(t *testing.T) { assert.Equal(t, "", ErrorClass(nil)) } +func TestTrackEvent_NoClientInContextIsSafe(t *testing.T) { + // Must not panic when the context carries no telemetry client. + TrackEvent(context.Background(), AuthStarted(FlowLogin, false)) +} + +func TestAuthStarted(t *testing.T) { + event := AuthStarted(FlowSignup, true) + assert.Equal(t, EventAuthStarted, event.Name) + assert.Equal(t, FlowSignup, event.Properties["flow"]) + assert.Equal(t, true, event.Properties["no_browser"]) +} + +func TestAuthCompleted(t *testing.T) { + event := AuthCompleted(FlowLogin, NewFlowTracker()) + assert.Equal(t, EventAuthCompleted, event.Name) + assert.Equal(t, FlowLogin, event.Properties["flow"]) + assert.Contains(t, event.Properties, "duration_ms") +} + +func TestAuthAborted(t *testing.T) { + tracker := NewFlowTracker() + tracker.SetStep(StepAppSelect) + + event := AuthAborted(FlowLogin, tracker) + assert.Equal(t, EventAuthAborted, event.Name) + assert.Equal(t, StepAppSelect, event.Properties["step"]) +} + +func TestAuthFailed(t *testing.T) { + tracker := NewFlowTracker() + tracker.SetStep(StepAppsFetch) + + event := AuthFailed(FlowLogin, tracker, errors.New("boom")) + assert.Equal(t, EventAuthFailed, event.Name) + assert.Equal(t, StepAppsFetch, event.Properties["step"]) + assert.Equal(t, "*errors.errorString", event.Properties["error_class"]) + assert.Contains(t, event.Properties, "duration_ms") +} + func TestFlowTracker_NilTrackerIsSafe(t *testing.T) { var tracker *FlowTracker tracker.SetStep(StepTerms) diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index 9330d055..d0a4c918 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -6,7 +6,6 @@ import ( "fmt" "log" "net" - "os" "runtime" "sync/atomic" @@ -76,26 +75,6 @@ func NewAnalyticsTelemetryClient(debug bool) (TelemetryClient, error) { return &AnalyticsTelemetryClient{client: client}, nil } -// IdentifyOnce sends a single Identify event through a short-lived client and -// flushes it before returning. It is meant for one-shot identification (for -// example, right after authentication fills the token) where the command's -// request-scoped client may already have been closed. It honors the same -// ALGOLIA_CLI_TELEMETRY and DEBUG environment variables as the root command and -// fails silently so telemetry never blocks the user. -func IdentifyOnce(ctx context.Context) { - if os.Getenv("ALGOLIA_CLI_TELEMETRY") == "0" { - return - } - - client, err := NewAnalyticsTelemetryClient(os.Getenv("DEBUG") != "") - if err != nil { - return - } - defer client.Close() - - _ = client.Identify(ctx) -} - // anonymousID is a unique identifier for an anonymous user of the CLI (basically the hash of the mac address) func anonymousID() string { addrs, err := net.Interfaces() diff --git a/pkg/telemetry/telemetrytest/recording.go b/pkg/telemetry/telemetrytest/recording.go new file mode 100644 index 00000000..bea7ae86 --- /dev/null +++ b/pkg/telemetry/telemetrytest/recording.go @@ -0,0 +1,45 @@ +// Package telemetrytest provides test doubles for the telemetry package. +package telemetrytest + +import ( + "context" + "sync" + + "github.com/algolia/cli/pkg/telemetry" +) + +var _ telemetry.TelemetryClient = (*RecordingClient)(nil) + +// RecordedEvent is one event captured by a RecordingClient. +type RecordedEvent struct { + Name string + Properties map[string]any +} + +// RecordingClient is a telemetry.TelemetryClient that records the tracked +// events so tests can assert on their names, properties and order. +type RecordingClient struct { + mu sync.Mutex + Events []RecordedEvent + Identifies int +} + +func (r *RecordingClient) Identify(ctx context.Context) error { + r.mu.Lock() + defer r.mu.Unlock() + r.Identifies++ + return nil +} + +func (r *RecordingClient) Track( + ctx context.Context, + event string, + properties map[string]any, +) error { + r.mu.Lock() + defer r.mu.Unlock() + r.Events = append(r.Events, RecordedEvent{event, properties}) + return nil +} + +func (r *RecordingClient) Close() {}