Skip to content

Commit

Permalink
Fix web session playback when a duration is not provided (#50461)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zmb3
zmb3 authored Dec 24, 2024
1 parent e2ee237 commit d47b6d8
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 112 deletions.
13 changes: 0 additions & 13 deletions lib/client/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4950,19 +4950,6 @@ func findActiveApps(key *Key) ([]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(
Expand Down
75 changes: 0 additions & 75 deletions lib/client/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"math"
"os"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/gravitational/trace"
Expand All @@ -43,10 +42,8 @@ import (
"github.com/gravitational/teleport/api/utils/grpc/interceptors"
"github.com/gravitational/teleport/api/utils/keys"
"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"
)

Expand Down Expand Up @@ -906,78 +903,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) {
Expand Down
1 change: 1 addition & 0 deletions lib/events/auditlog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,7 @@ func (l *AuditLog) StreamSessionEvents(ctx context.Context, sessionID session.ID
}

protoReader := NewProtoReader(rawSession)
defer protoReader.Close()

for {
if ctx.Err() != nil {
Expand Down
1 change: 1 addition & 0 deletions lib/web/apiserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,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))

// TODO(zmb3): remove these endpoints when Assist is no longer using them
// (assist calls the proxy's web API, and the proxy uses an HTTP client to call auth's API)
Expand Down
41 changes: 41 additions & 0 deletions lib/web/tty_playback.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
70 changes: 57 additions & 13 deletions web/packages/teleport/src/Player/Player.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,23 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react';
import styled from 'styled-components';
import React, { useCallback, useEffect } from 'react';

import { Flex, Box } from 'design';
import styled from 'styled-components';

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';
Expand All @@ -39,19 +41,44 @@ import Tabs, { TabItem } from './PlayerTabs';
const validRecordingTypes = ['ssh', 'k8s', 'desktop', 'database'];

export function Player() {
const ctx = useTeleport();
const { sid, clusterId } = useParams<UrlPlayerParams>();
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();
Expand All @@ -70,13 +97,25 @@ export function Player() {
);
}

if (!validDurationMs) {
if (
combinedAttempt.status === '' ||
combinedAttempt.status === 'processing'
) {
return (
<StyledPlayer>
<Box textAlign="center" mx={10} mt={5}>
<Indicator />
</Box>
</StyledPlayer>
);
}
if (combinedAttempt.status === 'error') {
return (
<StyledPlayer>
<Box textAlign="center" mx={10} mt={5}>
<Danger mb={0}>
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.
</Danger>
</Box>
</StyledPlayer>
Expand All @@ -102,15 +141,20 @@ export function Player() {
<DesktopPlayer
sid={sid}
clusterId={clusterId}
durationMs={durationMs}
durationMs={combinedAttempt.data.durationMs}
/>
) : (
<SshPlayer sid={sid} clusterId={clusterId} durationMs={durationMs} />
<SshPlayer
sid={sid}
clusterId={clusterId}
durationMs={combinedAttempt.data.durationMs}
/>
)}
</Flex>
</StyledPlayer>
);
}

const StyledPlayer = styled.div`
display: flex;
height: 100%;
Expand Down
25 changes: 16 additions & 9 deletions web/packages/teleport/src/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,30 @@
*/

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 type {
import {
Auth2faType,
AuthProvider,
AuthType,
PreferredMfaType,
PrimaryAuthType,
} from 'shared/services';

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/auth';
import type { PluginKind, Regions } from './services/integrations';
import type { WebauthnAssertionResponse } from 'teleport/services/auth';
import type { PluginKind, Regions } from 'teleport/services/integrations';
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. */
Expand Down Expand Up @@ -250,6 +252,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',

// TODO(zmb3): remove this when Assist is no longer using it
sshPlaybackPrefix: '/v1/webapi/sites/:clusterId/sessions/:sid', // prefix because this is eventually concatenated with "/stream" or "/events"
Expand Down Expand Up @@ -747,6 +750,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,
Expand Down
3 changes: 1 addition & 2 deletions web/packages/teleport/src/services/mfa/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
*/

import { WebauthnAssertionResponse } from '../auth';
import { DeviceUsage } from '../auth/types';
import { CreateNewHardwareDeviceRequest } from '../auth/types';
import { CreateNewHardwareDeviceRequest, DeviceUsage } from '../auth/types';

export interface MfaDevice {
id: string;
Expand Down
7 changes: 7 additions & 0 deletions web/packages/teleport/src/services/recordings/recordings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

0 comments on commit d47b6d8

Please sign in to comment.