Skip to content

Commit aa200ff

Browse files
deansheatherdeansheather
committed
chore: support signed token query param for web terminal
1 parent 9f5220e commit aa200ff

File tree

6 files changed

+119
-49
lines changed

6 files changed

+119
-49
lines changed

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 85 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/stretchr/testify/assert"
2323
"github.com/stretchr/testify/require"
2424
"golang.org/x/xerrors"
25+
"nhooyr.io/websocket"
2526

2627
"github.com/coder/coder/coderd/coderdtest"
2728
"github.com/coder/coder/coderd/rbac"
@@ -46,43 +47,9 @@ func Run(t *testing.T, factory DeploymentFactory) {
4647
t.Skip("ConPTY appears to be inconsistent on Windows.")
4748
}
4849

49-
appDetails := setupProxyTest(t, nil)
50-
51-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
52-
defer cancel()
53-
54-
// Run the test against the path app hostname since that's where the
55-
// reconnecting-pty proxy server we want to test is mounted.
56-
client := appDetails.AppClient(t)
57-
conn, err := client.WorkspaceAgentReconnectingPTY(ctx, appDetails.Agent.ID, uuid.New(), 80, 80, "/bin/bash")
58-
require.NoError(t, err)
59-
defer conn.Close()
60-
61-
// First attempt to resize the TTY.
62-
// The websocket will close if it fails!
63-
data, err := json.Marshal(codersdk.ReconnectingPTYRequest{
64-
Height: 250,
65-
Width: 250,
66-
})
67-
require.NoError(t, err)
68-
_, err = conn.Write(data)
69-
require.NoError(t, err)
70-
bufRead := bufio.NewReader(conn)
71-
72-
// Brief pause to reduce the likelihood that we send keystrokes while
73-
// the shell is simultaneously sending a prompt.
74-
time.Sleep(100 * time.Millisecond)
75-
76-
data, err = json.Marshal(codersdk.ReconnectingPTYRequest{
77-
Data: "echo test\r\n",
78-
})
79-
require.NoError(t, err)
80-
_, err = conn.Write(data)
81-
require.NoError(t, err)
82-
83-
expectLine := func(matcher func(string) bool) {
50+
expectLine := func(r *bufio.Reader, matcher func(string) bool) {
8451
for {
85-
line, err := bufRead.ReadString('\n')
52+
line, err := r.ReadString('\n')
8653
require.NoError(t, err)
8754
if matcher(line) {
8855
break
@@ -96,8 +63,88 @@ func Run(t *testing.T, factory DeploymentFactory) {
9663
return strings.Contains(line, "test") && !strings.Contains(line, "echo")
9764
}
9865

99-
expectLine(matchEchoCommand)
100-
expectLine(matchEchoOutput)
66+
t.Run("OK", func(t *testing.T) {
67+
appDetails := setupProxyTest(t, nil)
68+
69+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
70+
defer cancel()
71+
72+
// Run the test against the path app hostname since that's where the
73+
// reconnecting-pty proxy server we want to test is mounted.
74+
client := appDetails.AppClient(t)
75+
conn, err := client.WorkspaceAgentReconnectingPTY(ctx, appDetails.Agent.ID, uuid.New(), 80, 80, "/bin/bash")
76+
require.NoError(t, err)
77+
defer conn.Close()
78+
79+
// First attempt to resize the TTY.
80+
// The websocket will close if it fails!
81+
data, err := json.Marshal(codersdk.ReconnectingPTYRequest{
82+
Height: 250,
83+
Width: 250,
84+
})
85+
require.NoError(t, err)
86+
_, err = conn.Write(data)
87+
require.NoError(t, err)
88+
bufRead := bufio.NewReader(conn)
89+
90+
// Brief pause to reduce the likelihood that we send keystrokes while
91+
// the shell is simultaneously sending a prompt.
92+
time.Sleep(100 * time.Millisecond)
93+
94+
data, err = json.Marshal(codersdk.ReconnectingPTYRequest{
95+
Data: "echo test\r\n",
96+
})
97+
require.NoError(t, err)
98+
_, err = conn.Write(data)
99+
require.NoError(t, err)
100+
101+
expectLine(bufRead, matchEchoCommand)
102+
expectLine(bufRead, matchEchoOutput)
103+
})
104+
105+
t.Run("SignedTokenQueryParameter", func(t *testing.T) {
106+
t.Parallel()
107+
108+
appDetails := setupProxyTest(t, nil)
109+
if appDetails.AppHostIsPrimary {
110+
t.Skip("Tickets are not used for terminal requests on the primary.")
111+
}
112+
113+
u := *appDetails.PathAppBaseURL
114+
u.Path = fmt.Sprintf("/api/v2/workspaceagents/%s/pty", appDetails.Agent.ID.String())
115+
116+
ctx := testutil.Context(t, testutil.WaitLong)
117+
issueRes, err := appDetails.SDKClient.IssueReconnectingPTYSignedToken(ctx, codersdk.IssueReconnectingPTYSignedTokenRequest{
118+
URL: u.String(),
119+
AgentID: appDetails.Agent.ID,
120+
})
121+
require.NoError(t, err)
122+
123+
// Try to connect to the endpoint with the signed token and no other
124+
// authentication.
125+
q := u.Query()
126+
q.Set("reconnect", uuid.NewString())
127+
q.Set("height", strconv.Itoa(24))
128+
q.Set("width", strconv.Itoa(80))
129+
q.Set("command", `/bin/sh -c "echo test"`)
130+
q.Set(codersdk.SignedAppTokenQueryParameter, issueRes.SignedToken)
131+
u.RawQuery = q.Encode()
132+
133+
//nolint:bodyclose
134+
wsConn, res, err := websocket.Dial(ctx, u.String(), nil)
135+
if !assert.NoError(t, err) {
136+
dump, err := httputil.DumpResponse(res, true)
137+
if err == nil {
138+
t.Log(string(dump))
139+
}
140+
return
141+
}
142+
defer wsConn.Close(websocket.StatusNormalClosure, "")
143+
conn := websocket.NetConn(ctx, wsConn, websocket.MessageBinary)
144+
bufRead := bufio.NewReader(conn)
145+
146+
expectLine(bufRead, matchEchoOutput)
147+
})
101148
})
102149

103150
t.Run("WorkspaceAppsProxyPath", func(t *testing.T) {

‎coderd/workspaceapps/proxy_test.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
11
package workspaceapps_test
22

3-
// NOTE: for now, app proxying tests are still in their old locations, pending
4-
// being moved to their own package.
5-
//
6-
// See:
7-
// - coderd/workspaceapps_test.go
8-
// - coderd/workspaceagents_test.go (for PTY)
3+
// App tests can be found in the apptest package.

‎coderd/workspaceapps/token.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,30 @@ func (k SecurityKey) DecryptAPIKey(encryptedAPIKey string) (string, error) {
220220
return payload.APIKey, nil
221221
}
222222

223+
// FromRequest returns the signed token from the request, if it exists and is
224+
// valid. The caller must check that the token matches the request.
223225
func FromRequest(r *http.Request, key SecurityKey) (*SignedToken, bool) {
224-
// Get the existing token from the request.
225-
tokenCookie, err := r.Cookie(codersdk.DevURLSignedAppTokenCookie)
226-
if err == nil {
227-
token, err := key.VerifySignedToken(tokenCookie.Value)
226+
// Get the token string from the request. We usually use a cookie for this,
227+
// but for web terminal we also support a query parameter to support
228+
// cross-domain terminal access.
229+
tokenStr := ""
230+
tokenCookie, cookieErr := r.Cookie(codersdk.DevURLSignedAppTokenCookie)
231+
if cookieErr == nil {
232+
tokenStr = tokenCookie.Value
233+
} else {
234+
tokenStr = r.URL.Query().Get(codersdk.SignedAppTokenQueryParameter)
235+
}
236+
237+
if tokenStr != "" {
238+
token, err := key.VerifySignedToken(tokenStr)
228239
if err == nil {
229240
req := token.Request.Normalize()
241+
if cookieErr != nil && req.AccessMethod != AccessMethodTerminal {
242+
// The request must be a terminal request if we're using a
243+
// query parameter.
244+
return nil, false
245+
}
246+
230247
err := req.Validate()
231248
if err == nil {
232249
// The request has a valid signed app token, which is a valid

‎codersdk/client.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ const (
4646
// token.
4747
//nolint:gosec
4848
DevURLSignedAppTokenCookie = "coder_devurl_signed_app_token"
49+
// SignedAppTokenQueryParameter is the name of the query parameter that
50+
// stores a temporary JWT that can be used to authenticate instead of the
51+
// session token. This is only acceptable on reconnecting-pty requests, not
52+
// apps.
53+
//
54+
// It has a random suffix to avoid conflict with user query parameters on
55+
// apps.
56+
//nolint:gosec
57+
SignedAppTokenQueryParameter = "coder_signed_app_token_23db1dde"
4958

5059
// BypassRatelimitHeader is the custom header to use to bypass ratelimits.
5160
// Only owners can bypass rate limits. This is typically used for scale testing.

‎docs/api/applications enterprise.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Applications Enterprise

‎enterprise/coderd/workspaceproxy_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ func TestReconnectingPTYSignedToken(t *testing.T) {
359359
require.NoError(t, err)
360360
require.NotEmpty(t, res.SignedToken)
361361

362-
// Verify the token is valid for connecting to the terminal.
362+
// The token is validated in the apptest suite, so we don't need to
363+
// validate it here.
363364
})
364365
}

0 commit comments

Comments
 (0)