Skip to content

chore: support signed token query param for web terminal #7197

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

Merged
merged 4 commits into from
Apr 20, 2023

Conversation

deansheather
Copy link
Member

We have an issue where we have no way of authenticating the websocket requests to the reconnecting-pty endpoint on the moon because of cross-site cookie constraints.

One solution would be to host the whole terminal page on the moon, but it's included in the whole site bundle AND it makes API requests (other than just the terminal endpoint) so it'd be difficult to move across.

This solution avoids hosting static files on the moon.

  • Add an endpoint to generate a signed app token for a reconnecting-pty request.
  • Add support for reading the signed app token from a query parameter when connecting to a web terminal websocket.

The goal is to have the frontend make a request to the new endpoint if it's about to connect to a web terminal on an external workspace proxy, then include it in the URL.

Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

We should make sure this only works for terminal apps. There is no use outside of it 👍

@deansheather deansheather enabled auto-merge (squash) April 20, 2023 23:47
@deansheather deansheather merged commit 6866732 into main Apr 20, 2023
@deansheather deansheather deleted the dreamteam/moon-terminal branch April 20, 2023 23:59
@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants