Skip to content

Commit 56e2427

Browse files
sreyasreya
committed
add wsconncache tests
1 parent 3238edf commit 56e2427

File tree

5 files changed

+157
-80
lines changed

5 files changed

+157
-80
lines changed

‎coderd/tailnet.go

+12-18
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,17 @@ func NewServerTailnet(
7171
tn.transport.DialContext = tn.dialContext
7272
tn.transport.MaxIdleConnsPerHost = 10
7373
tn.transport.MaxIdleConns = 0
74+
// We intentionally don't verify the certificate chain here.
75+
// The connection to the workspace is already established and most
76+
// apps are already going to be accessed over plain HTTP, this config
77+
// simply allows apps being run over HTTPS to be accessed without error --
78+
// many of which may be using self-signed certs.
79+
tn.transport.TLSClientConfig = &tls.Config{
80+
MinVersion: tls.VersionTLS12,
81+
//nolint:gosec
82+
InsecureSkipVerify: true,
83+
}
84+
7485
agentConn, err := getMultiAgent(ctx)
7586
if err != nil {
7687
return nil, xerrors.Errorf("get initial multi agent: %w", err)
@@ -215,24 +226,7 @@ type ServerTailnet struct {
215226
transport *http.Transport
216227
}
217228

218-
// insureTLSConfig returns a tls config that does not verify
219-
// the server's certificate chain.
220-
func insecureTLSConfig() *tls.Config {
221-
return &tls.Config{
222-
MinVersion: tls.VersionTLS12,
223-
InsecureSkipVerify: true,
224-
}
225-
}
226-
227229
func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID uuid.UUID) (_ *httputil.ReverseProxy, release func(), _ error) {
228-
transport := s.transport
229-
230-
// We don't verify certificates for localhost applications.
231-
if targetURL.Scheme == "https" {
232-
transport = transport.Clone()
233-
transport.TLSClientConfig = insecureTLSConfig()
234-
}
235-
236230
proxy := httputil.NewSingleHostReverseProxy(targetURL)
237231
proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) {
238232
site.RenderStaticErrorPage(w, r, site.ErrorPageData{
@@ -244,7 +238,7 @@ func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID u
244238
})
245239
}
246240
proxy.Director = s.director(agentID, proxy.Director)
247-
proxy.Transport = transport
241+
proxy.Transport = s.transport
248242

249243
return proxy, func() {}, nil
250244
}

‎coderd/tailnet_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) {
105105
s := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
106106
w.WriteHeader(expectedResponseCode)
107107
}))
108-
defer s.Close()
108+
t.Cleanup(s.Close)
109109

110110
uri, err := url.Parse(s.URL)
111111
require.NoError(t, err)

‎coderd/workspaceapps/apptest/apptest.go

+91-2
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,51 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
349349
require.Equal(t, http.StatusOK, resp.StatusCode)
350350
})
351351

352+
t.Run("ProxiesHTTPS", func(t *testing.T) {
353+
t.Parallel()
354+
355+
appDetails := setupProxyTest(t, &DeploymentOptions{
356+
ServeHTTPS: true,
357+
})
358+
359+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
360+
defer cancel()
361+
362+
u := appDetails.PathAppURL(appDetails.Apps.Owner)
363+
resp, err := requestWithRetries(ctx, t, appDetails.AppClient(t), http.MethodGet, u.String(), nil)
364+
require.NoError(t, err)
365+
defer resp.Body.Close()
366+
body, err := io.ReadAll(resp.Body)
367+
require.NoError(t, err)
368+
require.Equal(t, proxyTestAppBody, string(body))
369+
require.Equal(t, http.StatusOK, resp.StatusCode)
370+
371+
var appTokenCookie *http.Cookie
372+
for _, c := range resp.Cookies() {
373+
if c.Name == codersdk.DevURLSignedAppTokenCookie {
374+
appTokenCookie = c
375+
break
376+
}
377+
}
378+
require.NotNil(t, appTokenCookie, "no signed app token cookie in response")
379+
require.Equal(t, appTokenCookie.Path, u.Path, "incorrect path on app token cookie")
380+
381+
// Ensure the signed app token cookie is valid.
382+
appTokenClient := appDetails.AppClient(t)
383+
appTokenClient.SetSessionToken("")
384+
appTokenClient.HTTPClient.Jar, err = cookiejar.New(nil)
385+
require.NoError(t, err)
386+
appTokenClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{appTokenCookie})
387+
388+
resp, err = requestWithRetries(ctx, t, appTokenClient, http.MethodGet, u.String(), nil)
389+
require.NoError(t, err)
390+
defer resp.Body.Close()
391+
body, err = io.ReadAll(resp.Body)
392+
require.NoError(t, err)
393+
require.Equal(t, proxyTestAppBody, string(body))
394+
require.Equal(t, http.StatusOK, resp.StatusCode)
395+
})
396+
352397
t.Run("BlocksMe", func(t *testing.T) {
353398
t.Parallel()
354399

@@ -762,6 +807,50 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
762807
require.Equal(t, http.StatusOK, resp.StatusCode)
763808
})
764809

810+
t.Run("ProxiesHTTPS", func(t *testing.T) {
811+
t.Parallel()
812+
813+
appDetails := setupProxyTest(t, &DeploymentOptions{
814+
ServeHTTPS: true,
815+
})
816+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
817+
defer cancel()
818+
819+
u := appDetails.SubdomainAppURL(appDetails.Apps.Owner)
820+
resp, err := requestWithRetries(ctx, t, appDetails.AppClient(t), http.MethodGet, u.String(), nil)
821+
require.NoError(t, err)
822+
defer resp.Body.Close()
823+
body, err := io.ReadAll(resp.Body)
824+
require.NoError(t, err)
825+
require.Equal(t, proxyTestAppBody, string(body))
826+
require.Equal(t, http.StatusOK, resp.StatusCode)
827+
828+
var appTokenCookie *http.Cookie
829+
for _, c := range resp.Cookies() {
830+
if c.Name == codersdk.DevURLSignedAppTokenCookie {
831+
appTokenCookie = c
832+
break
833+
}
834+
}
835+
require.NotNil(t, appTokenCookie, "no signed token cookie in response")
836+
require.Equal(t, appTokenCookie.Path, "/", "incorrect path on signed token cookie")
837+
838+
// Ensure the signed app token cookie is valid.
839+
appTokenClient := appDetails.AppClient(t)
840+
appTokenClient.SetSessionToken("")
841+
appTokenClient.HTTPClient.Jar, err = cookiejar.New(nil)
842+
require.NoError(t, err)
843+
appTokenClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{appTokenCookie})
844+
845+
resp, err = requestWithRetries(ctx, t, appTokenClient, http.MethodGet, u.String(), nil)
846+
require.NoError(t, err)
847+
defer resp.Body.Close()
848+
body, err = io.ReadAll(resp.Body)
849+
require.NoError(t, err)
850+
require.Equal(t, proxyTestAppBody, string(body))
851+
require.Equal(t, http.StatusOK, resp.StatusCode)
852+
})
853+
765854
t.Run("ProxiesPort", func(t *testing.T) {
766855
t.Parallel()
767856

@@ -928,8 +1017,8 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
9281017
forceURLTransport(t, client)
9291018

9301019
// Create workspace.
931-
port := appServer(t, nil)
932-
workspace, _ = createWorkspaceWithApps(t, client, user.OrganizationIDs[0], user, port)
1020+
port := appServer(t, nil, false)
1021+
workspace, _ = createWorkspaceWithApps(t, client, user.OrganizationIDs[0], user, port, false)
9331022

9341023
// Verify that the apps have the correct sharing levels set.
9351024
workspaceBuild, err := client.WorkspaceBuild(ctx, workspace.LatestBuild.ID)

‎coderd/workspaceapps/apptest/setup.go

+41-46
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net"
77
"net/http"
8+
"net/http/httptest"
89
"net/url"
910
"path"
1011
"strconv"
@@ -48,6 +49,7 @@ type DeploymentOptions struct {
4849
DisableSubdomainApps bool
4950
DangerousAllowPathAppSharing bool
5051
DangerousAllowPathAppSiteOwnerAccess bool
52+
ServeHTTPS bool
5153

5254
// The following fields are only used by setupProxyTestWithFactory.
5355
noWorkspace bool
@@ -185,9 +187,9 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De
185187
}
186188

187189
if opts.port == 0 {
188-
opts.port = appServer(t, opts.headers)
190+
opts.port = appServer(t, opts.headers, opts.ServeHTTPS)
189191
}
190-
workspace, agnt := createWorkspaceWithApps(t, deployment.SDKClient, deployment.FirstUser.OrganizationID, me, opts.port)
192+
workspace, agnt := createWorkspaceWithApps(t, deployment.SDKClient, deployment.FirstUser.OrganizationID, me, opts.port, opts.ServeHTTPS)
191193

192194
details := &Details{
193195
Deployment: deployment,
@@ -234,60 +236,53 @@ func setupProxyTestWithFactory(t *testing.T, factory DeploymentFactory, opts *De
234236
return details
235237
}
236238

237-
func appServer(t *testing.T, headers http.Header) uint16 {
238-
// Start a listener on a random port greater than the minimum app port.
239-
var (
240-
ln net.Listener
241-
tcpAddr *net.TCPAddr
239+
//nolint:revive
240+
func appServer(t *testing.T, headers http.Header, isHTTPS bool) uint16 {
241+
server := httptest.NewUnstartedServer(
242+
http.HandlerFunc(
243+
func(w http.ResponseWriter, r *http.Request) {
244+
_, err := r.Cookie(codersdk.SessionTokenCookie)
245+
assert.ErrorIs(t, err, http.ErrNoCookie)
246+
w.Header().Set("X-Forwarded-For", r.Header.Get("X-Forwarded-For"))
247+
for name, values := range headers {
248+
for _, value := range values {
249+
w.Header().Add(name, value)
250+
}
251+
}
252+
w.WriteHeader(http.StatusOK)
253+
_, _ = w.Write([]byte(proxyTestAppBody))
254+
},
255+
),
242256
)
243-
for i := 0; i < 32; i++ {
244-
var err error
245-
// #nosec
246-
ln, err = net.Listen("tcp", ":0")
247-
require.NoError(t, err)
248257

249-
var ok bool
250-
tcpAddr, ok = ln.Addr().(*net.TCPAddr)
251-
require.True(t, ok)
252-
if tcpAddr.Port < codersdk.WorkspaceAgentMinimumListeningPort {
253-
_ = ln.Close()
254-
ln = nil
255-
time.Sleep(20 * time.Millisecond)
256-
continue
257-
}
258-
}
259-
require.NotNil(t, ln, "failed to find a free port greater than the minimum app port")
260-
261-
server := http.Server{
262-
ReadHeaderTimeout: time.Minute,
263-
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
264-
_, err := r.Cookie(codersdk.SessionTokenCookie)
265-
assert.ErrorIs(t, err, http.ErrNoCookie)
266-
w.Header().Set("X-Forwarded-For", r.Header.Get("X-Forwarded-For"))
267-
for name, values := range headers {
268-
for _, value := range values {
269-
w.Header().Add(name, value)
270-
}
271-
}
272-
w.WriteHeader(http.StatusOK)
273-
_, _ = w.Write([]byte(proxyTestAppBody))
274-
}),
258+
server.Config.ReadHeaderTimeout = time.Minute
259+
if isHTTPS {
260+
server.StartTLS()
261+
} else {
262+
server.Start()
275263
}
276264
t.Cleanup(func() {
277-
_ = server.Close()
278-
_ = ln.Close()
265+
server.Close()
279266
})
280-
go func() {
281-
_ = server.Serve(ln)
282-
}()
283267

284-
return uint16(tcpAddr.Port)
268+
_, portStr, err := net.SplitHostPort(server.Listener.Addr().String())
269+
require.NoError(t, err)
270+
port, err := strconv.ParseUint(portStr, 10, 16)
271+
require.NoError(t, err)
272+
273+
return uint16(port)
285274
}
286275

287-
func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.UUID, me codersdk.User, port uint16, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) (codersdk.Workspace, codersdk.WorkspaceAgent) {
276+
//nolint:revive
277+
func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.UUID, me codersdk.User, port uint16, serveHTTPS bool, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) (codersdk.Workspace, codersdk.WorkspaceAgent) {
288278
authToken := uuid.NewString()
289279

290-
appURL := fmt.Sprintf("http://127.0.0.1:%d?%s", port, proxyTestAppQuery)
280+
scheme := "http"
281+
if serveHTTPS {
282+
scheme = "https"
283+
}
284+
285+
appURL := fmt.Sprintf("%s://127.0.0.1:%d?%s", scheme, port, proxyTestAppQuery)
291286
version := coderdtest.CreateTemplateVersion(t, client, orgID, &echo.Responses{
292287
Parse: echo.ParseComplete,
293288
ProvisionPlan: echo.ProvisionComplete,

‎coderd/wsconncache/wsconncache.go

+12-13
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,6 @@ func (a *AgentProvider) ReverseProxy(targetURL *url.URL, dashboardURL *url.URL,
5151
}
5252

5353
transport := conn.HTTPTransport()
54-
// We don't verify certificates for localhost applications.
55-
if targetURL.Scheme == "https" {
56-
trans := transport.Clone()
57-
trans.TLSClientConfig = insecureTLSConfig()
58-
59-
}
6054

6155
proxy.Transport = transport
6256
return proxy, release, nil
@@ -162,6 +156,18 @@ func (c *Cache) Acquire(id uuid.UUID) (*Conn, func(), error) {
162156
}
163157
transport := defaultTransport.Clone()
164158
transport.DialContext = agentConn.DialContext
159+
160+
// // We intentionally don't verify the certificate chain here.
161+
// // The connection to the workspace is already established and most
162+
// // apps are already going to be accessed over plain HTTP, this config
163+
// // simply allows apps being run over HTTPS to be accessed without error --
164+
// // many of which may be using self-signed certs.
165+
transport.TLSClientConfig = &tls.Config{
166+
MinVersion: tls.VersionTLS12,
167+
//nolint:gosec
168+
InsecureSkipVerify: true,
169+
}
170+
165171
conn := &Conn{
166172
WorkspaceAgentConn: agentConn,
167173
timeoutCancel: timeoutCancelFunc,
@@ -219,10 +225,3 @@ func (c *Cache) Close() error {
219225
c.closeGroup.Wait()
220226
return nil
221227
}
222-
223-
func insecureTLSConfig() *tls.Config {
224-
return &tls.Config{
225-
MinVersion: tls.VersionTLS12,
226-
InsecureSkipVerify: true,
227-
}
228-
}

0 commit comments

Comments
 (0)