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

[v14] backport #37520 and #37981 #37918

Closed
wants to merge 2 commits into from

Conversation

lxea
Copy link
Contributor

@lxea lxea commented Feb 8, 2024

Backports #37520 and #37981

changelog: Removes access tokens from URL parameters, preventing them from being leaked to DNS servers, ISPs and other intermediary systems that may log them in plaintext.

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

update connectToHost

linter

read errors from websocket

missing /ws on ttyWsAddr and fix wrong onmessage

fix race in test

lint

skip TestTerminal as it takes 11 seconds to run

dont skip the test

resolve apiserver comments

Add an AuthenticatedWebSocket class

convert other clients to use AuthenticatedWebSocket

Converts `AuthenticatedWebSocket` into drop-in replacement for `WebSocket` (#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`.

Create a single authnWsUpgrader with a comment justifying why we turn off CORS

recieving to receiving

resolve comments
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).

@lxea lxea force-pushed the lxea/websocket-bearer-token-v14 branch from 7f21a4f to f7e91eb Compare February 9, 2024 17:27
@ibeckermayer ibeckermayer added the security Security Issues label Feb 9, 2024
@ibeckermayer ibeckermayer changed the title [v14] Read the bearer token over websocket endpoints instead of query parameter [v14] backport #37520 and #37981 Feb 9, 2024
@ibeckermayer ibeckermayer marked this pull request as draft February 9, 2024 19:39
@ibeckermayer
Copy link
Contributor

Converting this to a draft for now, since it requires some major surgery to get it in order.

@ibeckermayer
Copy link
Contributor

closed in favor of #38070

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

2 participants