fix(vpn): avoid setting session token header twice (#18524)

`coderd` currently does not handle a session token header value of the form `token1, token2`. However, it does handle multiple instances of the token header by simply taking the first. This is the default behaviour of `http.Header.Get`.

So, setting the token header twice causes issues when Coder is behind a proxy that merges duplicate headers, such as [Apache](https://httpd.apache.org/docs/2.4/mod/mod_headers.html#:~:text=list%20of%20values.-,When%20a%20new%20value%20is%20merged%20onto%20an%20existing%20header,format%20specifiers%20have%20been%20processed).


This PR ensures we don't set it twice by not sharing one slice between the `HTTPClient` and the `websocket.DialerOptions`. It also adds a regression test.
This commit is contained in:
Ethan
2025-06-25 11:27:35 +10:00
committed by GitHub
parent 288ec7709d
commit 79c666bf08
3 changed files with 12 additions and 4 deletions
+1 -1
View File
@@ -354,7 +354,7 @@ func (c *Client) Dial(ctx context.Context, path string, opts *websocket.DialOpti
if opts.HTTPHeader == nil {
opts.HTTPHeader = http.Header{}
}
if opts.HTTPHeader.Get("tokenHeader") == "" {
if opts.HTTPHeader.Get(tokenHeader) == "" {
opts.HTTPHeader.Set(tokenHeader, c.SessionToken())
}
+4 -3
View File
@@ -92,7 +92,7 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string
sdk.SetSessionToken(token)
sdk.HTTPClient.Transport = &codersdk.HeaderTransport{
Transport: http.DefaultTransport,
Header: headers,
Header: headers.Clone(),
}
// New context, separate from initCtx. We don't want to cancel the
@@ -129,17 +129,18 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string
headers.Set(codersdk.SessionTokenHeader, token)
dialer := workspacesdk.NewWebsocketDialer(options.Logger, rpcURL, &websocket.DialOptions{
HTTPClient: sdk.HTTPClient,
HTTPHeader: headers,
HTTPHeader: headers.Clone(),
CompressionMode: websocket.CompressionDisabled,
}, workspacesdk.WithWorkspaceUpdates(&proto.WorkspaceUpdatesRequest{
WorkspaceOwnerId: tailnet.UUIDToByteSlice(me.ID),
}))
clonedHeaders := headers.Clone()
ip := tailnet.CoderServicePrefix.RandomAddr()
conn, err := tailnet.NewConn(&tailnet.Options{
Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)},
DERPMap: connInfo.DERPMap,
DERPHeader: &headers,
DERPHeader: &clonedHeaders,
DERPForceWebSockets: connInfo.DERPForceWebSockets,
Logger: options.Logger,
BlockEndpoints: connInfo.DisableDirectConnections,
+7
View File
@@ -90,6 +90,8 @@ func TestClient_WorkspaceUpdates(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v2/users/me":
values := r.Header.Values(codersdk.SessionTokenHeader)
assert.Len(t, values, 1, "expected exactly one session token header value")
httpapi.Write(ctx, w, http.StatusOK, codersdk.User{
ReducedUser: codersdk.ReducedUser{
MinimalUser: codersdk.MinimalUser{
@@ -101,6 +103,8 @@ func TestClient_WorkspaceUpdates(t *testing.T) {
user <- struct{}{}
case "/api/v2/workspaceagents/connection":
values := r.Header.Values(codersdk.SessionTokenHeader)
assert.Len(t, values, 1, "expected exactly one session token header value")
httpapi.Write(ctx, w, http.StatusOK, tc.agentConnectionInfo)
connInfo <- struct{}{}
@@ -109,6 +113,9 @@ func TestClient_WorkspaceUpdates(t *testing.T) {
cVer := r.URL.Query().Get("version")
assert.Equal(t, "2.3", cVer)
values := r.Header.Values(codersdk.SessionTokenHeader)
assert.Len(t, values, 1, "expected exactly one session token header value")
sws, err := websocket.Accept(w, r, nil)
if !assert.NoError(t, err) {
return