Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] Read the bearer token over websocket endpoints instead of query parameter #37915

Closed
wants to merge 17 commits into from

Conversation

lxea
Copy link
Contributor

@lxea lxea commented Feb 8, 2024

Backport #37520 to branch/v15

changelog: Read bearer token via websocket on websocket endpoints

lxea and others added 16 commits February 8, 2024 02:24
use the request context, not session

Dont pass websocket by context

lint

resolve some comments

Add TestWSAuthenticateRequest

Close ws in handler

deprecation notices, doc

resolve comments

resolve comments

give a longer read/write deadline

dont set write deadline, ws endpoints never did before and it breaks things

convert frontend to use ws access token

Resolove comments, move to using an explicit state

fix ci

reset read deadline

prettier
…cket` (#37699)

* Converts `AuthenticatedWebSocket` into drop-in replacement for `WebSocket`
that automatically goes through Teleport's custom authentication process
before facilitating any caller-defined communication.

This also reverts previous-`WebSocket` users to their original state
(sans the code for passing the bearer token in the query string),
swapping in `AuthenticatedWebSocket` in place of `WebSocket`.
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lxea Have you tested the backport?

@lxea
Copy link
Contributor Author

lxea commented Feb 8, 2024

@lxea Have you tested the backport?

Tested it with node connection, this applied cleanly so should be okay, working on the v14/13 versions

Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playback of recorded desktop sessions appears broken on master.

image

Copy link
Contributor

@ibeckermayer ibeckermayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we forgot to update the desktop playback endpoint in the parent PR, we should refrain from cutting a release with this commit until #37981 is merged (and backported).

cc @zmb3 this fixes playback.

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.
@ibeckermayer
Copy link
Contributor

Closed in favor of #38032

@r0mant r0mant deleted the bot/backport-37520-branch/v15 branch February 9, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants