Skip to content

Commit d6edd29

Browse files
deansheatherdeansheather
committed
Smuggling for path apps on proxies
1 parent a112e29 commit d6edd29

File tree

6 files changed

+376
-240
lines changed

6 files changed

+376
-240
lines changed

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 210 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"net/http/cookiejar"
1212
"net/http/httputil"
1313
"net/url"
14+
"path"
1415
"runtime"
1516
"strconv"
1617
"strings"
@@ -24,6 +25,7 @@ import (
2425

2526
"github.com/coder/coder/coderd/coderdtest"
2627
"github.com/coder/coder/coderd/rbac"
28+
"github.com/coder/coder/coderd/workspaceapps"
2729
"github.com/coder/coder/codersdk"
2830
"github.com/coder/coder/testutil"
2931
)
@@ -122,9 +124,13 @@ func Run(t *testing.T, factory DeploymentFactory) {
122124
require.Contains(t, string(body), "Path-based applications are disabled")
123125
})
124126

125-
t.Run("LoginWithoutAuth", func(t *testing.T) {
127+
t.Run("LoginWithoutAuthOnPrimary", func(t *testing.T) {
126128
t.Parallel()
127129

130+
if !appDetails.AppHostServesAPI {
131+
t.Skip("This test only applies when testing apps on the primary.")
132+
}
133+
128134
unauthedClient := appDetails.AppClient(t)
129135
unauthedClient.SetSessionToken("")
130136

@@ -143,6 +149,43 @@ func Run(t *testing.T, factory DeploymentFactory) {
143149
require.True(t, loc.Query().Has("redirect"))
144150
})
145151

152+
t.Run("LoginWithoutAuthOnProxy", func(t *testing.T) {
153+
t.Parallel()
154+
155+
if appDetails.AppHostServesAPI {
156+
t.Skip("This test only applies when testing apps on workspace proxies.")
157+
}
158+
159+
unauthedClient := appDetails.AppClient(t)
160+
unauthedClient.SetSessionToken("")
161+
162+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
163+
defer cancel()
164+
165+
u := appDetails.PathAppURL(appDetails.OwnerApp)
166+
resp, err := requestWithRetries(ctx, t, unauthedClient, http.MethodGet, u.String(), nil)
167+
require.NoError(t, err)
168+
defer resp.Body.Close()
169+
170+
require.Equal(t, http.StatusSeeOther, resp.StatusCode)
171+
loc, err := resp.Location()
172+
require.NoError(t, err)
173+
require.Equal(t, appDetails.APIClient.URL.Host, loc.Host)
174+
require.Equal(t, "/api/v2/applications/auth-redirect", loc.Path)
175+
176+
redirectURIStr := loc.Query().Get("redirect_uri")
177+
require.NotEmpty(t, redirectURIStr)
178+
redirectURI, err := url.Parse(redirectURIStr)
179+
require.NoError(t, err)
180+
181+
require.Equal(t, u.Scheme, redirectURI.Scheme)
182+
require.Equal(t, u.Host, redirectURI.Host)
183+
// TODO(@dean): I have no idea how but the trailing slash on this
184+
// request is getting stripped.
185+
require.Equal(t, u.Path, redirectURI.Path+"/")
186+
require.Equal(t, u.RawQuery, redirectURI.RawQuery)
187+
})
188+
146189
t.Run("NoAccessShould404", func(t *testing.T) {
147190
t.Parallel()
148191

@@ -304,129 +347,181 @@ func Run(t *testing.T, factory DeploymentFactory) {
304347

305348
appDetails := setupProxyTest(t, nil)
306349

307-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
308-
defer cancel()
350+
cases := []struct {
351+
name string
352+
appURL *url.URL
353+
verifyCookie func(t *testing.T, c *http.Cookie)
354+
}{
355+
{
356+
name: "Subdomain",
357+
appURL: appDetails.SubdomainAppURL(appDetails.OwnerApp),
358+
verifyCookie: func(t *testing.T, c *http.Cookie) {
359+
// TODO(@dean): fix these asserts, they don't seem to
360+
// work. I wonder if Go strips the domain from the
361+
// cookie object if it's invalid or something.
362+
// domain := strings.SplitN(appDetails.Options.AppHost, ".", 2)
363+
// require.Equal(t, "."+domain[1], c.Domain, "incorrect domain on app token cookie")
364+
},
365+
},
366+
{
367+
name: "Path",
368+
appURL: appDetails.PathAppURL(appDetails.OwnerApp),
369+
verifyCookie: func(t *testing.T, c *http.Cookie) {
370+
// TODO(@dean): fix these asserts, they don't seem to
371+
// work. I wonder if Go strips the domain from the
372+
// cookie object if it's invalid or something.
373+
// require.Equal(t, "", c.Domain, "incorrect domain on app token cookie")
374+
},
375+
},
376+
}
309377

310-
// Get the current user and API key.
311-
user, err := appDetails.APIClient.User(ctx, codersdk.Me)
312-
require.NoError(t, err)
313-
currentAPIKey, err := appDetails.APIClient.APIKeyByID(ctx, appDetails.FirstUser.UserID.String(), strings.Split(appDetails.APIClient.SessionToken(), "-")[0])
314-
require.NoError(t, err)
378+
for _, c := range cases {
379+
c := c
315380

316-
appClient := appDetails.AppClient(t)
317-
appClient.SetSessionToken("")
381+
if c.name == "Path" && appDetails.AppHostServesAPI {
382+
// Workspace application auth does not apply to path apps
383+
// served from the primary access URL as no smuggling needs
384+
// to take place (they're already logged in with a session
385+
// token).
386+
continue
387+
}
318388

319-
// Try to load the application without authentication.
320-
u := appDetails.SubdomainAppURL(appDetails.OwnerApp)
321-
u.Path = "/test"
322-
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
323-
require.NoError(t, err)
389+
t.Run(c.name, func(t *testing.T) {
390+
t.Parallel()
324391

325-
var resp *http.Response
326-
resp, err = doWithRetries(t, appClient, req)
327-
require.NoError(t, err)
328-
resp.Body.Close()
392+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
393+
defer cancel()
329394

330-
// Check that the Location is correct.
331-
gotLocation, err := resp.Location()
332-
require.NoError(t, err)
333-
// This should always redirect to the primary access URL.
334-
require.Equal(t, appDetails.APIClient.URL.Host, gotLocation.Host)
335-
require.Equal(t, "/api/v2/applications/auth-redirect", gotLocation.Path)
336-
require.Equal(t, u.String(), gotLocation.Query().Get("redirect_uri"))
337-
338-
// Load the application auth-redirect endpoint.
339-
resp, err = requestWithRetries(ctx, t, appDetails.APIClient, http.MethodGet, "/api/v2/applications/auth-redirect", nil, codersdk.WithQueryParam(
340-
"redirect_uri", u.String(),
341-
))
342-
require.NoError(t, err)
343-
defer resp.Body.Close()
395+
// Get the current user and API key.
396+
user, err := appDetails.APIClient.User(ctx, codersdk.Me)
397+
require.NoError(t, err)
398+
currentAPIKey, err := appDetails.APIClient.APIKeyByID(ctx, appDetails.FirstUser.UserID.String(), strings.Split(appDetails.APIClient.SessionToken(), "-")[0])
399+
require.NoError(t, err)
344400

345-
require.Equal(t, http.StatusSeeOther, resp.StatusCode)
346-
gotLocation, err = resp.Location()
347-
require.NoError(t, err)
401+
appClient := appDetails.AppClient(t)
402+
appClient.SetSessionToken("")
348403

349-
// Copy the query parameters and then check equality.
350-
u.RawQuery = gotLocation.RawQuery
351-
require.Equal(t, u, gotLocation)
352-
353-
// Verify the API key is set.
354-
var encryptedAPIKey string
355-
for k, v := range gotLocation.Query() {
356-
// The query parameter may change dynamically in the future and is
357-
// not exported, so we just use a fuzzy check instead.
358-
if strings.Contains(k, "api_key") {
359-
encryptedAPIKey = v[0]
360-
}
361-
}
362-
require.NotEmpty(t, encryptedAPIKey, "no API key was set in the query parameters")
404+
// Try to load the application without authentication.
405+
u := c.appURL
406+
u.Path = path.Join(u.Path, "/test")
407+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
408+
require.NoError(t, err)
363409

364-
// Decrypt the API key by following the request.
365-
t.Log("navigating to: ", gotLocation.String())
366-
req, err = http.NewRequestWithContext(ctx, "GET", gotLocation.String(), nil)
367-
require.NoError(t, err)
368-
resp, err = doWithRetries(t, appClient, req)
369-
require.NoError(t, err)
370-
resp.Body.Close()
371-
require.Equal(t, http.StatusSeeOther, resp.StatusCode)
372-
cookies := resp.Cookies()
373-
require.Len(t, cookies, 1)
374-
apiKey := cookies[0].Value
410+
var resp *http.Response
411+
resp, err = doWithRetries(t, appClient, req)
412+
require.NoError(t, err)
375413

376-
// Fetch the API key from the API.
377-
apiKeyInfo, err := appDetails.APIClient.APIKeyByID(ctx, appDetails.FirstUser.UserID.String(), strings.Split(apiKey, "-")[0])
378-
require.NoError(t, err)
379-
require.Equal(t, user.ID, apiKeyInfo.UserID)
380-
require.Equal(t, codersdk.LoginTypePassword, apiKeyInfo.LoginType)
381-
require.WithinDuration(t, currentAPIKey.ExpiresAt, apiKeyInfo.ExpiresAt, 5*time.Second)
382-
require.EqualValues(t, currentAPIKey.LifetimeSeconds, apiKeyInfo.LifetimeSeconds)
383-
384-
// Verify the API key permissions
385-
appTokenAPIClient := codersdk.New(appDetails.APIClient.URL)
386-
appTokenAPIClient.SetSessionToken(apiKey)
387-
appTokenAPIClient.HTTPClient.CheckRedirect = appDetails.APIClient.HTTPClient.CheckRedirect
388-
appTokenAPIClient.HTTPClient.Transport = appDetails.APIClient.HTTPClient.Transport
389-
390-
var (
391-
canCreateApplicationConnect = "can-create-application_connect"
392-
canReadUserMe = "can-read-user-me"
393-
)
394-
authRes, err := appTokenAPIClient.AuthCheck(ctx, codersdk.AuthorizationRequest{
395-
Checks: map[string]codersdk.AuthorizationCheck{
396-
canCreateApplicationConnect: {
397-
Object: codersdk.AuthorizationObject{
398-
ResourceType: "application_connect",
399-
OwnerID: "me",
400-
OrganizationID: appDetails.FirstUser.OrganizationID.String(),
401-
},
402-
Action: "create",
403-
},
404-
canReadUserMe: {
405-
Object: codersdk.AuthorizationObject{
406-
ResourceType: "user",
407-
OwnerID: "me",
408-
ResourceID: appDetails.FirstUser.UserID.String(),
414+
if !assert.Equal(t, http.StatusSeeOther, resp.StatusCode) {
415+
dump, err := httputil.DumpResponse(resp, true)
416+
require.NoError(t, err)
417+
t.Log(string(dump))
418+
}
419+
resp.Body.Close()
420+
421+
// Check that the Location is correct.
422+
gotLocation, err := resp.Location()
423+
require.NoError(t, err)
424+
// This should always redirect to the primary access URL.
425+
require.Equal(t, appDetails.APIClient.URL.Host, gotLocation.Host)
426+
require.Equal(t, "/api/v2/applications/auth-redirect", gotLocation.Path)
427+
require.Equal(t, u.String(), gotLocation.Query().Get("redirect_uri"))
428+
429+
// Load the application auth-redirect endpoint.
430+
resp, err = requestWithRetries(ctx, t, appDetails.APIClient, http.MethodGet, "/api/v2/applications/auth-redirect", nil, codersdk.WithQueryParam(
431+
"redirect_uri", u.String(),
432+
))
433+
require.NoError(t, err)
434+
defer resp.Body.Close()
435+
436+
require.Equal(t, http.StatusSeeOther, resp.StatusCode)
437+
gotLocation, err = resp.Location()
438+
require.NoError(t, err)
439+
440+
// Copy the query parameters and then check equality.
441+
u.RawQuery = gotLocation.RawQuery
442+
require.Equal(t, u, gotLocation)
443+
444+
// Verify the API key is set.
445+
encryptedAPIKey := gotLocation.Query().Get(workspaceapps.SubdomainProxyAPIKeyParam)
446+
require.NotEmpty(t, encryptedAPIKey, "no API key was set in the query parameters")
447+
448+
// Decrypt the API key by following the request.
449+
t.Log("navigating to: ", gotLocation.String())
450+
req, err = http.NewRequestWithContext(ctx, "GET", gotLocation.String(), nil)
451+
require.NoError(t, err)
452+
resp, err = doWithRetries(t, appClient, req)
453+
require.NoError(t, err)
454+
resp.Body.Close()
455+
require.Equal(t, http.StatusSeeOther, resp.StatusCode)
456+
457+
cookies := resp.Cookies()
458+
var cookie *http.Cookie
459+
for _, c := range cookies {
460+
if c.Name == codersdk.DevURLSessionTokenCookie {
461+
cookie = c
462+
break
463+
}
464+
}
465+
require.NotNil(t, cookie, "no app session token cookie was set")
466+
c.verifyCookie(t, cookie)
467+
apiKey := cookie.Value
468+
469+
// Fetch the API key from the API.
470+
apiKeyInfo, err := appDetails.APIClient.APIKeyByID(ctx, appDetails.FirstUser.UserID.String(), strings.Split(apiKey, "-")[0])
471+
require.NoError(t, err)
472+
require.Equal(t, user.ID, apiKeyInfo.UserID)
473+
require.Equal(t, codersdk.LoginTypePassword, apiKeyInfo.LoginType)
474+
require.WithinDuration(t, currentAPIKey.ExpiresAt, apiKeyInfo.ExpiresAt, 5*time.Second)
475+
require.EqualValues(t, currentAPIKey.LifetimeSeconds, apiKeyInfo.LifetimeSeconds)
476+
477+
// Verify the API key permissions
478+
appTokenAPIClient := codersdk.New(appDetails.APIClient.URL)
479+
appTokenAPIClient.SetSessionToken(apiKey)
480+
appTokenAPIClient.HTTPClient.CheckRedirect = appDetails.APIClient.HTTPClient.CheckRedirect
481+
appTokenAPIClient.HTTPClient.Transport = appDetails.APIClient.HTTPClient.Transport
482+
483+
var (
484+
canCreateApplicationConnect = "can-create-application_connect"
485+
canReadUserMe = "can-read-user-me"
486+
)
487+
authRes, err := appTokenAPIClient.AuthCheck(ctx, codersdk.AuthorizationRequest{
488+
Checks: map[string]codersdk.AuthorizationCheck{
489+
canCreateApplicationConnect: {
490+
Object: codersdk.AuthorizationObject{
491+
ResourceType: "application_connect",
492+
OwnerID: "me",
493+
OrganizationID: appDetails.FirstUser.OrganizationID.String(),
494+
},
495+
Action: "create",
496+
},
497+
canReadUserMe: {
498+
Object: codersdk.AuthorizationObject{
499+
ResourceType: "user",
500+
OwnerID: "me",
501+
ResourceID: appDetails.FirstUser.UserID.String(),
502+
},
503+
Action: "read",
504+
},
409505
},
410-
Action: "read",
411-
},
412-
},
413-
})
414-
require.NoError(t, err)
415-
416-
require.True(t, authRes[canCreateApplicationConnect])
417-
require.False(t, authRes[canReadUserMe])
418-
419-
// Load the application page with the API key set.
420-
gotLocation, err = resp.Location()
421-
require.NoError(t, err)
422-
t.Log("navigating to: ", gotLocation.String())
423-
req, err = http.NewRequestWithContext(ctx, "GET", gotLocation.String(), nil)
424-
require.NoError(t, err)
425-
req.Header.Set(codersdk.SessionTokenHeader, apiKey)
426-
resp, err = doWithRetries(t, appClient, req)
427-
require.NoError(t, err)
428-
resp.Body.Close()
429-
require.Equal(t, http.StatusOK, resp.StatusCode)
506+
})
507+
require.NoError(t, err)
508+
509+
require.True(t, authRes[canCreateApplicationConnect])
510+
require.False(t, authRes[canReadUserMe])
511+
512+
// Load the application page with the API key set.
513+
gotLocation, err = resp.Location()
514+
require.NoError(t, err)
515+
t.Log("navigating to: ", gotLocation.String())
516+
req, err = http.NewRequestWithContext(ctx, "GET", gotLocation.String(), nil)
517+
require.NoError(t, err)
518+
req.Header.Set(codersdk.SessionTokenHeader, apiKey)
519+
resp, err = doWithRetries(t, appClient, req)
520+
require.NoError(t, err)
521+
resp.Body.Close()
522+
require.Equal(t, http.StatusOK, resp.StatusCode)
523+
})
524+
}
430525
})
431526
})
432527

@@ -866,7 +961,7 @@ func Run(t *testing.T, factory DeploymentFactory) {
866961
require.NoError(t, err, msg)
867962

868963
expectedPath := "/login"
869-
if !isPathApp {
964+
if !isPathApp || !appDetails.AppHostServesAPI {
870965
expectedPath = "/api/v2/applications/auth-redirect"
871966
}
872967
assert.Equal(t, expectedPath, location.Path, "should not have access, expected redirect to applicable login endpoint. "+msg)

0 commit comments

Comments
 (0)