From 510064934f03eb595c897415661cfab3ba7d5516 Mon Sep 17 00:00:00 2001 From: Zac Bergquist Date: Tue, 24 Dec 2024 15:58:28 -0700 Subject: [PATCH] Fix web session playback when a duration is not provided (#50459) Prior to Teleport 15, the web UI would download the entire session recording into browser memory before playing it back (crashing the browser tab for long sessions). Starting with Teleport 15, session playback is streamed to the browser and played back as it is received instead of waiting for the entire session to be available. A side effect of this change is that the browser needs to know the length of the session in order to render the progress bar during playback. Since the browser starts playing the session before it has received all of it, we started providing the length of the session via a URL query parameter. Some users have grown accustomed to being able to access session recordings at their original URLs (without the duration query parameter). If you attempt to play recordings from these URLs after upgrading to v15, you'll get an error that the duration is missing. To fix this, the web UI needs to request the duration of the session before it can begin playing it (unless the duration is provided via the URL). There are two ways we could get this information: 1. By querying Teleport's audit log 2. By reading the recording twice: once to get to the end event and compute the duration, and a second time to actually play it back. Since we only have a session ID, an audit log query would be inefficient - we have no idea when the session occurred, so we'd have to search from the beginning of time. (This could be resolved by using a UUIDv7 for session IDs, but Teleport uses UUIDv4 today). For this reason, we elect option 2. This commit creates a new web API endpoint that will fetch a session recording file and scan through it in the same way that streaming is done, but instaed of streaming the data through a websocket it simply reads through to the end to compute the session length. The benefit of this approach is that it will generally be faster than option 1 (unless the session is very long), and it effectively pre-downloads the recording file on the Note: option 2 is not without its drawbacks - since the web UI is making two requests that both read the session recording, the audit log will show two separate session_recording.access events. This isn't ideal but it is good enough to get playback working again for customers who don't access playbacks by clicking the "Play" button in the UI. --- lib/client/api.go | 13 ---- lib/client/api_test.go | 75 ------------------- lib/events/auditlog.go | 1 + lib/web/apiserver.go | 1 + lib/web/tty_playback.go | 41 ++++++++++ web/packages/teleport/src/Player/Player.tsx | 68 ++++++++++++++--- web/packages/teleport/src/config.ts | 35 +++++---- .../teleport/src/services/mfa/types.ts | 3 +- .../src/services/recordings/recordings.ts | 7 ++ 9 files changed, 128 insertions(+), 116 deletions(-) diff --git a/lib/client/api.go b/lib/client/api.go index 88693bc768bb4..6cc0caae15286 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -5032,19 +5032,6 @@ func findActiveApps(keyRing *KeyRing) ([]tlsca.RouteToApp, error) { return apps, nil } -// getDesktopEventWebURL returns the web UI URL users can access to -// watch a desktop session recording in the browser -func getDesktopEventWebURL(proxyHost string, cluster string, sid *session.ID, events []events.EventFields) string { - if len(events) < 1 { - return "" - } - start := events[0].GetTimestamp() - end := events[len(events)-1].GetTimestamp() - duration := end.Sub(start) - - return fmt.Sprintf("https://%s/web/cluster/%s/session/%s?recordingType=desktop&durationMs=%d", proxyHost, cluster, sid, duration/time.Millisecond) -} - // SearchSessionEvents allows searching for session events with a full pagination support. func (tc *TeleportClient) SearchSessionEvents(ctx context.Context, fromUTC, toUTC time.Time, pageSize int, order types.EventOrder, max int) ([]apievents.AuditEvent, error) { ctx, span := tc.Tracer.Start( diff --git a/lib/client/api_test.go b/lib/client/api_test.go index 9da016c7af401..136d338b01e39 100644 --- a/lib/client/api_test.go +++ b/lib/client/api_test.go @@ -27,7 +27,6 @@ import ( "math" "os" "testing" - "time" "github.com/coreos/go-semver/semver" "github.com/google/go-cmp/cmp" @@ -45,10 +44,8 @@ import ( "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/cryptosuites" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/observability/tracing" - "github.com/gravitational/teleport/lib/session" "github.com/gravitational/teleport/lib/utils" ) @@ -929,78 +926,6 @@ func TestFormatConnectToProxyErr(t *testing.T) { } } -func TestGetDesktopEventWebURL(t *testing.T) { - initDate := time.Date(2021, 1, 1, 12, 0, 0, 0, time.UTC) - - tt := []struct { - name string - proxyHost string - cluster string - sid session.ID - events []events.EventFields - expected string - }{ - { - name: "nil events", - events: nil, - expected: "", - }, - { - name: "empty events", - events: make([]events.EventFields, 0), - expected: "", - }, - { - name: "two events, 1000 ms duration", - proxyHost: "host", - cluster: "cluster", - sid: "session_id", - events: []events.EventFields{ - { - "time": initDate, - }, - { - "time": initDate.Add(1000 * time.Millisecond), - }, - }, - expected: "https://host/web/cluster/cluster/session/session_id?recordingType=desktop&durationMs=1000", - }, - { - name: "multiple events", - proxyHost: "host", - cluster: "cluster", - sid: "session_id", - events: []events.EventFields{ - { - "time": initDate, - }, - { - "time": initDate.Add(10 * time.Millisecond), - }, - { - "time": initDate.Add(20 * time.Millisecond), - }, - { - "time": initDate.Add(30 * time.Millisecond), - }, - { - "time": initDate.Add(40 * time.Millisecond), - }, - { - "time": initDate.Add(50 * time.Millisecond), - }, - }, - expected: "https://host/web/cluster/cluster/session/session_id?recordingType=desktop&durationMs=50", - }, - } - - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.expected, getDesktopEventWebURL(tc.proxyHost, tc.cluster, &tc.sid, tc.events)) - }) - } -} - type mockRoleGetter func(ctx context.Context) ([]types.Role, error) func (m mockRoleGetter) GetRoles(ctx context.Context) ([]types.Role, error) { diff --git a/lib/events/auditlog.go b/lib/events/auditlog.go index 51180746cbe7f..3570171f40996 100644 --- a/lib/events/auditlog.go +++ b/lib/events/auditlog.go @@ -555,6 +555,7 @@ func (l *AuditLog) StreamSessionEvents(ctx context.Context, sessionID session.ID } protoReader := NewProtoReader(rawSession) + defer protoReader.Close() for { if ctx.Err() != nil { diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index e293a1dfa6535..9a080398ba71d 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -850,6 +850,7 @@ func (h *Handler) bindDefaultEndpoints() { h.GET("/webapi/sites/:site/events/search", h.WithClusterAuth(h.clusterSearchEvents)) // search site events h.GET("/webapi/sites/:site/events/search/sessions", h.WithClusterAuth(h.clusterSearchSessionEvents)) // search site session events h.GET("/webapi/sites/:site/ttyplayback/:sid", h.WithClusterAuth(h.ttyPlaybackHandle)) + h.GET("/webapi/sites/:site/sessionlength/:sid", h.WithClusterAuth(h.sessionLengthHandle)) // scp file transfer h.GET("/webapi/sites/:site/nodes/:server/:login/scp", h.WithClusterAuth(h.transferFile)) diff --git a/lib/web/tty_playback.go b/lib/web/tty_playback.go index f601f4237666c..76c456a4a7010 100644 --- a/lib/web/tty_playback.go +++ b/lib/web/tty_playback.go @@ -53,6 +53,47 @@ const ( actionPause = byte(1) ) +func (h *Handler) sessionLengthHandle( + w http.ResponseWriter, + r *http.Request, + p httprouter.Params, + sctx *SessionContext, + site reversetunnelclient.RemoteSite, +) (interface{}, error) { + sID := p.ByName("sid") + if sID == "" { + return nil, trace.BadParameter("missing session ID in request URL") + } + + ctx, cancel := context.WithCancel(r.Context()) + defer cancel() + + clt, err := sctx.GetUserClient(ctx, site) + if err != nil { + return nil, trace.Wrap(err) + } + + evts, errs := clt.StreamSessionEvents(ctx, session.ID(sID), 0) + for { + select { + case err := <-errs: + return nil, trace.Wrap(err) + case evt, ok := <-evts: + if !ok { + return nil, trace.NotFound("could not find end event for session %v", sID) + } + switch evt := evt.(type) { + case *events.SessionEnd: + return map[string]any{"durationMs": evt.EndTime.Sub(evt.StartTime).Milliseconds()}, nil + case *events.WindowsDesktopSessionEnd: + return map[string]any{"durationMs": evt.EndTime.Sub(evt.StartTime).Milliseconds()}, nil + case *events.DatabaseSessionEnd: + return map[string]any{"durationMs": evt.EndTime.Sub(evt.StartTime).Milliseconds()}, nil + } + } + } +} + func (h *Handler) ttyPlaybackHandle( w http.ResponseWriter, r *http.Request, diff --git a/web/packages/teleport/src/Player/Player.tsx b/web/packages/teleport/src/Player/Player.tsx index 190f8afb1b558..7d26a785d1830 100644 --- a/web/packages/teleport/src/Player/Player.tsx +++ b/web/packages/teleport/src/Player/Player.tsx @@ -16,20 +16,22 @@ * along with this program. If not, see . */ +import { useCallback, useEffect } from 'react'; import styled from 'styled-components'; -import { Flex, Box } from 'design'; - +import { Flex, Box, Indicator } from 'design'; import { Danger } from 'design/Alert'; -import { useParams, useLocation } from 'teleport/components/Router'; +import { makeSuccessAttempt, useAsync } from 'shared/hooks/useAsync'; +import { useParams, useLocation } from 'teleport/components/Router'; import session from 'teleport/services/websession'; import { UrlPlayerParams } from 'teleport/config'; import { getUrlParameter } from 'teleport/services/history'; - import { RecordingType } from 'teleport/services/recordings'; +import useTeleport from 'teleport/useTeleport'; + import ActionBar from './ActionBar'; import { DesktopPlayer } from './DesktopPlayer'; import SshPlayer from './SshPlayer'; @@ -38,19 +40,44 @@ import Tabs, { TabItem } from './PlayerTabs'; const validRecordingTypes = ['ssh', 'k8s', 'desktop', 'database']; export function Player() { + const ctx = useTeleport(); const { sid, clusterId } = useParams(); const { search } = useLocation(); + useEffect(() => { + document.title = `Play ${sid} • ${clusterId}`; + }, [sid, clusterId]); + const recordingType = getUrlParameter( 'recordingType', search ) as RecordingType; - const durationMs = Number(getUrlParameter('durationMs', search)); + + // In order to render the progress bar, we need to know the length of the session. + // All in-product links to the session player should include the session duration in the URL. + // Some users manually build the URL based on the session ID and don't specify the session duration. + // For those cases, we make a separate API call to get the duration. + const [fetchDurationAttempt, fetchDuration] = useAsync( + useCallback( + () => ctx.recordingsService.fetchRecordingDuration(clusterId, sid), + [ctx.recordingsService, clusterId, sid] + ) + ); const validRecordingType = validRecordingTypes.includes(recordingType); - const validDurationMs = Number.isInteger(durationMs) && durationMs > 0; + const durationMs = Number(getUrlParameter('durationMs', search)); + const shouldFetchSessionDuration = + validRecordingType && (!Number.isInteger(durationMs) || durationMs <= 0); + + useEffect(() => { + if (shouldFetchSessionDuration) { + fetchDuration(); + } + }, [fetchDuration, shouldFetchSessionDuration]); - document.title = `Play ${sid} • ${clusterId}`; + const combinedAttempt = shouldFetchSessionDuration + ? fetchDurationAttempt + : makeSuccessAttempt({ durationMs }); function onLogout() { session.logout(); @@ -69,13 +96,25 @@ export function Player() { ); } - if (!validDurationMs) { + if ( + combinedAttempt.status === '' || + combinedAttempt.status === 'processing' + ) { + return ( + + + + + + ); + } + if (combinedAttempt.status === 'error') { return ( - Invalid query parameter durationMs:{' '} - {getUrlParameter('durationMs', search)}, should be an integer. + Unable to determine the length of this session. The session + recording may be incomplete or corrupted. @@ -101,15 +140,20 @@ export function Player() { ) : ( - + )} ); } + const StyledPlayer = styled.div` display: flex; height: 100%; diff --git a/web/packages/teleport/src/config.ts b/web/packages/teleport/src/config.ts index 6980b827784cd..c8883b598eebf 100644 --- a/web/packages/teleport/src/config.ts +++ b/web/packages/teleport/src/config.ts @@ -17,21 +17,12 @@ */ import { generatePath } from 'react-router'; -import { mergeDeep } from 'shared/utils/highbar'; -import { IncludedResourceMode } from 'shared/components/UnifiedResources'; -import generateResourcePath from './generateResourcePath'; +import { IncludedResourceMode } from 'shared/components/UnifiedResources'; -import { defaultEntitlements } from './entitlement'; +import { mergeDeep } from 'shared/utils/highbar'; import { - AwsOidcPolicyPreset, - IntegrationKind, - PluginKind, - Regions, -} from './services/integrations'; - -import type { Auth2faType, AuthProvider, AuthType, @@ -39,12 +30,23 @@ import type { PrimaryAuthType, } from 'shared/services'; +import { + AwsOidcPolicyPreset, + IntegrationKind, + PluginKind, + Regions, +} from 'teleport/services/integrations'; +import { KubeResourceKind } from 'teleport/services/kube/types'; + +import { defaultEntitlements } from './entitlement'; + +import generateResourcePath from './generateResourcePath'; + import type { SortType } from 'teleport/services/agents'; import type { RecordingType } from 'teleport/services/recordings'; -import type { WebauthnAssertionResponse } from './services/mfa'; +import type { WebauthnAssertionResponse } from 'teleport/services/mfa'; import type { ParticipantMode } from 'teleport/services/session'; -import type { YamlSupportedResourceKind } from './services/yaml/types'; -import type { KubeResourceKind } from './services/kube/types'; +import type { YamlSupportedResourceKind } from 'teleport/services/yaml/types'; const cfg = { /** @deprecated Use cfg.edition instead. */ @@ -268,6 +270,7 @@ const cfg = { ttyPlaybackWsAddr: 'wss://:fqdn/v1/webapi/sites/:clusterId/ttyplayback/:sid?access_token=:token', // TODO(zmb3): get token out of URL activeAndPendingSessionsPath: '/v1/webapi/sites/:clusterId/sessions', + sessionDurationPath: '/v1/webapi/sites/:clusterId/sessionlength/:sid', kubernetesPath: '/v1/webapi/sites/:clusterId/kubernetes?searchAsRoles=:searchAsRoles?&limit=:limit?&startKey=:startKey?&query=:query?&search=:search?&sort=:sort?', @@ -778,6 +781,10 @@ const cfg = { return generatePath(cfg.api.activeAndPendingSessionsPath, { clusterId }); }, + getSessionDurationUrl(clusterId: string, sid: string) { + return generatePath(cfg.api.sessionDurationPath, { clusterId, sid }); + }, + getUnifiedResourcesUrl(clusterId: string, params: UrlResourcesParams) { return generateResourcePath(cfg.api.unifiedResourcesPath, { clusterId, diff --git a/web/packages/teleport/src/services/mfa/types.ts b/web/packages/teleport/src/services/mfa/types.ts index f1292c50c99cd..382d7831f82fe 100644 --- a/web/packages/teleport/src/services/mfa/types.ts +++ b/web/packages/teleport/src/services/mfa/types.ts @@ -18,8 +18,7 @@ import { AuthProviderType } from 'shared/services'; -import { Base64urlString } from '../auth/types'; -import { CreateNewHardwareDeviceRequest } from '../auth/types'; +import { Base64urlString, CreateNewHardwareDeviceRequest } from '../auth/types'; export type DeviceType = 'totp' | 'webauthn' | 'sso'; diff --git a/web/packages/teleport/src/services/recordings/recordings.ts b/web/packages/teleport/src/services/recordings/recordings.ts index e27ca67beea03..ba71160aa1795 100644 --- a/web/packages/teleport/src/services/recordings/recordings.ts +++ b/web/packages/teleport/src/services/recordings/recordings.ts @@ -45,4 +45,11 @@ export default class RecordingsService { return { recordings: events.map(makeRecording), startKey: json.startKey }; }); } + + fetchRecordingDuration( + clusterId: string, + sessionId: string + ): Promise<{ durationMs: number }> { + return api.get(cfg.getSessionDurationUrl(clusterId, sessionId)); + } }