Skip to content

Commit

Permalink
Updates desktopPlaybackHandle to new ws paradigm (#37981)
Browse files Browse the repository at this point in the history
* Updates `desktopPlaybackHandle` to new ws paradigm

This was mistakenly left out of #37520.
This commit also refactors `WithClusterAuthWebSocket` slightly for easier
comprehension, and updates the vite config to facilitate the new websocket
endpoints in development mode.

* Update lib/web/apiserver.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
  • Loading branch information
ibeckermayer and zmb3 committed Feb 9, 2024
1 parent ce51720 commit 031a3a3
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 31 deletions.
47 changes: 26 additions & 21 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,11 @@ func (h *Handler) bindDefaultEndpoints() {
// GET /webapi/sites/:site/desktops/:desktopName/connect?username=<username>&width=<width>&height=<height>
h.GET("/webapi/sites/:site/desktops/:desktopName/connect/ws", h.WithClusterAuthWebSocket(true, h.desktopConnectHandle))
// GET /webapi/sites/:site/desktopplayback/:sid?access_token=<bearer_token>
h.GET("/webapi/sites/:site/desktopplayback/:sid", h.WithClusterAuth(h.desktopPlaybackHandle))
// Deprecated: The desktopplayback/ws variant should be used instead.
// TODO(lxea): DELETE in v16
h.GET("/webapi/sites/:site/desktopplayback/:sid", h.WithClusterAuthWebSocket(false, h.desktopPlaybackHandle))
// GET /webapi/sites/:site/desktopplayback/:sid/ws
h.GET("/webapi/sites/:site/desktopplayback/:sid/ws", h.WithClusterAuthWebSocket(true, h.desktopPlaybackHandle))
h.GET("/webapi/sites/:site/desktops/:desktopName/active", h.WithClusterAuth(h.desktopIsActive))

// GET a Connection Diagnostics by its name
Expand Down Expand Up @@ -3811,32 +3815,20 @@ var authnWsUpgrader = websocket.Upgrader{
// TODO(lxea): remove the 'websocketAuth' bool once the deprecated websocket handlers are removed
func (h *Handler) WithClusterAuthWebSocket(websocketAuth bool, fn ClusterWebsocketHandler) httprouter.Handle {
return httplib.MakeHandler(func(w http.ResponseWriter, r *http.Request, p httprouter.Params) (any, error) {
if websocketAuth {
sctx, ws, site, err := h.authenticateWSRequestWithCluster(w, r, p)
if err != nil {
return nil, trace.Wrap(err)
}
// WS protocol requires the server send a close message
// which should be done by downstream users
defer ws.Close()
var sctx *SessionContext
var ws *websocket.Conn
var site reversetunnelclient.RemoteSite
var err error

if _, err := fn(w, r, p, sctx, site, ws); err != nil {
h.writeErrToWebSocket(ws, err)
}
return nil, nil
if websocketAuth {
sctx, ws, site, err = h.authenticateWSRequestWithCluster(w, r, p)
} else {
sctx, ws, site, err = h.authenticateWSRequestWithClusterDeprecated(w, r, p)
}

sctx, site, err := h.authenticateRequestWithCluster(w, r, p)
if err != nil {
return nil, trace.Wrap(err)
}
ws, err := authnWsUpgrader.Upgrade(w, r, nil)
if err != nil {
const errMsg = "Error upgrading to websocket"
h.log.WithError(err).Error(errMsg)
http.Error(w, errMsg, http.StatusInternalServerError)
return nil, nil
}
// WS protocol requires the server send a close message
// which should be done by downstream users
defer ws.Close()
Expand Down Expand Up @@ -3866,6 +3858,19 @@ func (h *Handler) authenticateWSRequestWithCluster(w http.ResponseWriter, r *htt
return sctx, ws, site, nil
}

// TODO(lxea): remove once the deprecated websocket handlers are removed
func (h *Handler) authenticateWSRequestWithClusterDeprecated(w http.ResponseWriter, r *http.Request, p httprouter.Params) (*SessionContext, *websocket.Conn, reversetunnelclient.RemoteSite, error) {
sctx, site, err := h.authenticateRequestWithCluster(w, r, p)
if err != nil {
return nil, nil, nil, trace.Wrap(err)
}
ws, err := authnWsUpgrader.Upgrade(w, r, nil)
if err != nil {
return nil, nil, nil, trace.Wrap(err)
}
return sctx, ws, site, nil
}

// authenticateRequestWithCluster ensures that a request is authenticated
// to this proxy, returning the *SessionContext (same as AuthenticateRequest),
// and also grabs the remoteSite (which can represent this local cluster or a
Expand Down
11 changes: 1 addition & 10 deletions lib/web/desktop_playback.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func (h *Handler) desktopPlaybackHandle(
p httprouter.Params,
sctx *SessionContext,
site reversetunnelclient.RemoteSite,
ws *websocket.Conn,
) (interface{}, error) {
sID := p.ByName("sid")
if sID == "" {
Expand All @@ -49,16 +50,6 @@ func (h *Handler) desktopPlaybackHandle(
return nil, trace.Wrap(err)
}

upgrader := websocket.Upgrader{
ReadBufferSize: 4096,
WriteBufferSize: 4096,
}
ws, err := upgrader.Upgrade(w, r, nil)
if err != nil {
return nil, trace.Wrap(err)
}
defer ws.Close()

player, err := player.New(&player.Config{
Clock: h.clock,
Log: h.log,
Expand Down

0 comments on commit 031a3a3

Please sign in to comment.