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] backport #37520 and #37981 #38032

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

ibeckermayer
Copy link
Contributor

@ibeckermayer ibeckermayer commented Feb 9, 2024

Backports #37520 and #37981

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

Alex McGrath and others added 2 commits February 9, 2024 10:39
…eter (#37520)

* Read the bearer token over WS endpoints

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

---------

Co-authored-by: Isaiah Becker-Mayer <[email protected]>
* 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 <[email protected]>

---------

Co-authored-by: Zac Bergquist <[email protected]>
@r0mant
Copy link
Collaborator

r0mant commented Feb 9, 2024

/excludeflake *

Copy link
Contributor

@jakule jakule left a comment

Choose a reason for hiding this comment

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

bot

@ibeckermayer ibeckermayer added this pull request to the merge queue Feb 9, 2024
Merged via the queue into branch/v15 with commit 3a9d624 Feb 9, 2024
39 of 41 checks passed
@ibeckermayer ibeckermayer deleted the isaiah/backport-37520-37981-branch/v15 branch February 9, 2024 19:40
@fheinecke fheinecke mentioned this pull request Feb 16, 2024
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.

3 participants