Commit79c666b

authored

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.

1 parent288ec77 commit79c666bCopy full SHA for 79c666b

File tree

+12

-4

lines changed

+12

-4

lines changed

Lines changed: 1 addition & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -354,7 +354,7 @@ func (c Client) Dial(ctx context.Context, path string, opts websocket.DialOpti`
`354`	`354`	`ifopts.HTTPHeader==nil {`
`355`	`355`	`opts.HTTPHeader= http.Header{}`
`356`	`356`	`}`
`357`		`-ifopts.HTTPHeader.Get("tokenHeader")=="" {`
	`357`	`+ifopts.HTTPHeader.Get(tokenHeader)=="" {`
`358`	`358`	`opts.HTTPHeader.Set(tokenHeader,c.SessionToken())`
`359`	`359`	`}`
`360`	`360`

Lines changed: 4 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ func (client) NewConn(initCtx context.Context, serverURL url.URL, token string`
`92`	`92`	`sdk.SetSessionToken(token)`
`93`	`93`	`sdk.HTTPClient.Transport=&codersdk.HeaderTransport{`
`94`	`94`	`Transport:http.DefaultTransport,`
`95`		`-Header:headers,`
	`95`	`+Header:headers.Clone(),`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`// 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`
`129`	`129`	`headers.Set(codersdk.SessionTokenHeader,token)`
`130`	`130`	`dialer:=workspacesdk.NewWebsocketDialer(options.Logger,rpcURL,&websocket.DialOptions{`
`131`	`131`	`HTTPClient:sdk.HTTPClient,`
`132`		`-HTTPHeader:headers,`
	`132`	`+HTTPHeader:headers.Clone(),`
`133`	`133`	`CompressionMode:websocket.CompressionDisabled,`
`134`	`134`	`},workspacesdk.WithWorkspaceUpdates(&proto.WorkspaceUpdatesRequest{`
`135`	`135`	`WorkspaceOwnerId:tailnet.UUIDToByteSlice(me.ID),`
`136`	`136`	`}))`
`137`	`137`
	`138`	`+clonedHeaders:=headers.Clone()`
`138`	`139`	`ip:=tailnet.CoderServicePrefix.RandomAddr()`
`139`	`140`	`conn,err:=tailnet.NewConn(&tailnet.Options{`
`140`	`141`	`Addresses: []netip.Prefix{netip.PrefixFrom(ip,128)},`
`141`	`142`	`DERPMap:connInfo.DERPMap,`
`142`		`-DERPHeader:&headers,`
	`143`	`+DERPHeader:&clonedHeaders,`
`143`	`144`	`DERPForceWebSockets:connInfo.DERPForceWebSockets,`
`144`	`145`	`Logger:options.Logger,`
`145`	`146`	`BlockEndpoints:connInfo.DisableDirectConnections,`

Lines changed: 7 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,8 @@ func TestClient_WorkspaceUpdates(t *testing.T) {`
`90`	`90`	`server:=httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {`
`91`	`91`	`switchr.URL.Path {`
`92`	`92`	`case"/api/v2/users/me":`
	`93`	`+values:=r.Header.Values(codersdk.SessionTokenHeader)`
	`94`	`+assert.Len(t,values,1,"expected exactly one session token header value")`
`93`	`95`	`httpapi.Write(ctx,w,http.StatusOK, codersdk.User{`
`94`	`96`	`ReducedUser: codersdk.ReducedUser{`
`95`	`97`	`MinimalUser: codersdk.MinimalUser{`
`@@ -101,6 +103,8 @@ func TestClient_WorkspaceUpdates(t *testing.T) {`
`101`	`103`	`user<-struct{}{}`
`102`	`104`
`103`	`105`	`case"/api/v2/workspaceagents/connection":`
	`106`	`+values:=r.Header.Values(codersdk.SessionTokenHeader)`
	`107`	`+assert.Len(t,values,1,"expected exactly one session token header value")`
`104`	`108`	`httpapi.Write(ctx,w,http.StatusOK,tc.agentConnectionInfo)`
`105`	`109`	`connInfo<-struct{}{}`
`106`	`110`
`@@ -109,6 +113,9 @@ func TestClient_WorkspaceUpdates(t *testing.T) {`
`109`	`113`	`cVer:=r.URL.Query().Get("version")`
`110`	`114`	`assert.Equal(t,"2.3",cVer)`
`111`	`115`
	`116`	`+values:=r.Header.Values(codersdk.SessionTokenHeader)`
	`117`	`+assert.Len(t,values,1,"expected exactly one session token header value")`
	`118`	`+`
`112`	`119`	`sws,err:=websocket.Accept(w,r,nil)`
`113`	`120`	`if!assert.NoError(t,err) {`
`114`	`121`	`return`

Comments

(0)