fix: avoid redirect loop on workspace proxies (#9389)

* fix: avoid redirect loop on workspace proxies

---------

Co-authored-by: Steven Masley <stevenmasley@coder.com>
This commit is contained in:
Dean Sheather 2023-08-28 18:34:52 -07:00 committed by GitHub
parent eb68684327
commit 5993f85ec9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 265 additions and 99 deletions

View File

@ -23,8 +23,9 @@ func StripCoderCookies(header string) string {
if name == codersdk.SessionTokenCookie || if name == codersdk.SessionTokenCookie ||
name == codersdk.OAuth2StateCookie || name == codersdk.OAuth2StateCookie ||
name == codersdk.OAuth2RedirectCookie || name == codersdk.OAuth2RedirectCookie ||
name == codersdk.DevURLSessionTokenCookie || name == codersdk.PathAppSessionTokenCookie ||
name == codersdk.DevURLSignedAppTokenCookie { name == codersdk.SubdomainAppSessionTokenCookie ||
name == codersdk.SignedAppTokenCookie {
continue continue
} }
cookies = append(cookies, part) cookies = append(cookies, part)

View File

@ -447,10 +447,10 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon
// APITokenFromRequest returns the api token from the request. // APITokenFromRequest returns the api token from the request.
// Find the session token from: // Find the session token from:
// 1: The cookie // 1: The cookie
// 1: The devurl cookie // 2. The coder_session_token query parameter
// 3: The old cookie // 3. The custom auth header
// 4. The coder_session_token query parameter //
// 5. The custom auth header // API tokens for apps are read from workspaceapps/cookies.go.
func APITokenFromRequest(r *http.Request) string { func APITokenFromRequest(r *http.Request) string {
cookie, err := r.Cookie(codersdk.SessionTokenCookie) cookie, err := r.Cookie(codersdk.SessionTokenCookie)
if err == nil && cookie.Value != "" { if err == nil && cookie.Value != "" {
@ -467,11 +467,6 @@ func APITokenFromRequest(r *http.Request) string {
return headerValue return headerValue
} }
cookie, err = r.Cookie(codersdk.DevURLSessionTokenCookie)
if err == nil && cookie.Value != "" {
return cookie.Value
}
return "" return ""
} }

View File

@ -257,7 +257,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
var appTokenCookie *http.Cookie var appTokenCookie *http.Cookie
for _, c := range resp.Cookies() { for _, c := range resp.Cookies() {
if c.Name == codersdk.DevURLSignedAppTokenCookie { if c.Name == codersdk.SignedAppTokenCookie {
appTokenCookie = c appTokenCookie = c
break break
} }
@ -302,7 +302,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
var appTokenCookie *http.Cookie var appTokenCookie *http.Cookie
for _, c := range resp.Cookies() { for _, c := range resp.Cookies() {
if c.Name == codersdk.DevURLSignedAppTokenCookie { if c.Name == codersdk.SignedAppTokenCookie {
appTokenCookie = c appTokenCookie = c
break break
} }
@ -400,30 +400,19 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
appDetails := setupProxyTest(t, nil) appDetails := setupProxyTest(t, nil)
cases := []struct { cases := []struct {
name string name string
appURL *url.URL appURL *url.URL
verifyCookie func(t *testing.T, c *http.Cookie) sessionTokenCookieName string
}{ }{
{ {
name: "Subdomain", name: "Subdomain",
appURL: appDetails.SubdomainAppURL(appDetails.Apps.Owner), appURL: appDetails.SubdomainAppURL(appDetails.Apps.Owner),
verifyCookie: func(t *testing.T, c *http.Cookie) { sessionTokenCookieName: codersdk.SubdomainAppSessionTokenCookie,
// TODO(@dean): fix these asserts, they don't seem to
// work. I wonder if Go strips the domain from the
// cookie object if it's invalid or something.
// domain := strings.SplitN(appDetails.Options.AppHost, ".", 2)
// require.Equal(t, "."+domain[1], c.Domain, "incorrect domain on app token cookie")
},
}, },
{ {
name: "Path", name: "Path",
appURL: appDetails.PathAppURL(appDetails.Apps.Owner), appURL: appDetails.PathAppURL(appDetails.Apps.Owner),
verifyCookie: func(t *testing.T, c *http.Cookie) { sessionTokenCookieName: codersdk.PathAppSessionTokenCookie,
// TODO(@dean): fix these asserts, they don't seem to
// work. I wonder if Go strips the domain from the
// cookie object if it's invalid or something.
// require.Equal(t, "", c.Domain, "incorrect domain on app token cookie")
},
}, },
} }
@ -508,14 +497,13 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
cookies := resp.Cookies() cookies := resp.Cookies()
var cookie *http.Cookie var cookie *http.Cookie
for _, c := range cookies { for _, co := range cookies {
if c.Name == codersdk.DevURLSessionTokenCookie { if co.Name == c.sessionTokenCookieName {
cookie = c cookie = co
break break
} }
} }
require.NotNil(t, cookie, "no app session token cookie was set") require.NotNil(t, cookie, "no app session token cookie was set")
c.verifyCookie(t, cookie)
apiKey := cookie.Value apiKey := cookie.Value
// Fetch the API key from the API. // Fetch the API key from the API.
@ -715,7 +703,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
var appTokenCookie *http.Cookie var appTokenCookie *http.Cookie
for _, c := range resp.Cookies() { for _, c := range resp.Cookies() {
if c.Name == codersdk.DevURLSignedAppTokenCookie { if c.Name == codersdk.SignedAppTokenCookie {
appTokenCookie = c appTokenCookie = c
break break
} }
@ -759,7 +747,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
var appTokenCookie *http.Cookie var appTokenCookie *http.Cookie
for _, c := range resp.Cookies() { for _, c := range resp.Cookies() {
if c.Name == codersdk.DevURLSignedAppTokenCookie { if c.Name == codersdk.SignedAppTokenCookie {
appTokenCookie = c appTokenCookie = c
break break
} }

View File

@ -0,0 +1,51 @@
package workspaceapps
import (
"net/http"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/codersdk"
)
// AppConnectSessionTokenCookieName returns the cookie name for the session
// token for the given access method.
func AppConnectSessionTokenCookieName(accessMethod AccessMethod) string {
if accessMethod == AccessMethodSubdomain {
return codersdk.SubdomainAppSessionTokenCookie
}
return codersdk.PathAppSessionTokenCookie
}
// AppConnectSessionTokenFromRequest returns the session token from the request
// if it exists. The access method is used to determine which cookie name to
// use.
//
// We use different cookie names for path apps and for subdomain apps to avoid
// both being set and sent to the server at the same time and the server using
// the wrong value.
//
// We use different cookie names for:
// - path apps on primary access URL: coder_session_token
// - path apps on proxies: coder_path_app_session_token
// - subdomain apps: coder_subdomain_app_session_token
//
// First we try the default function to get a token from request, which supports
// query parameters, the Coder-Session-Token header and the coder_session_token
// cookie.
//
// Then we try the specific cookie name for the access method.
func AppConnectSessionTokenFromRequest(r *http.Request, accessMethod AccessMethod) string {
// Try the default function first.
token := httpmw.APITokenFromRequest(r)
if token != "" {
return token
}
// Then try the specific cookie name for the access method.
cookie, err := r.Cookie(AppConnectSessionTokenCookieName(accessMethod))
if err == nil && cookie.Value != "" {
return cookie.Value
}
return ""
}

View File

@ -222,14 +222,14 @@ func Test_ResolveRequest(t *testing.T) {
// Try resolving a request for each app as the owner, without a // Try resolving a request for each app as the owner, without a
// token, then use the token to resolve each app. // token, then use the token to resolve each app.
for _, app := range allApps { for _, app := range allApps {
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: c.workspaceNameOrID, WorkspaceNameOrID: c.workspaceNameOrID,
AgentNameOrID: c.agentNameOrID, AgentNameOrID: c.agentNameOrID,
AppSlugOrPort: app, AppSlugOrPort: app,
} }).Normalize()
t.Log("app", app) t.Log("app", app)
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
@ -268,7 +268,7 @@ func Test_ResolveRequest(t *testing.T) {
// Check that the token was set in the response and is valid. // Check that the token was set in the response and is valid.
require.Len(t, w.Cookies(), 1) require.Len(t, w.Cookies(), 1)
cookie := w.Cookies()[0] cookie := w.Cookies()[0]
require.Equal(t, codersdk.DevURLSignedAppTokenCookie, cookie.Name) require.Equal(t, codersdk.SignedAppTokenCookie, cookie.Name)
require.Equal(t, req.BasePath, cookie.Path) require.Equal(t, req.BasePath, cookie.Path)
parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookie.Value) parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookie.Value)
@ -305,14 +305,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Parallel() t.Parallel()
for _, app := range allApps { for _, app := range allApps {
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: app, AppSlugOrPort: app,
} }).Normalize()
t.Log("app", app) t.Log("app", app)
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
@ -346,14 +346,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Parallel() t.Parallel()
for _, app := range allApps { for _, app := range allApps {
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: app, AppSlugOrPort: app,
} }).Normalize()
t.Log("app", app) t.Log("app", app)
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
@ -391,9 +391,9 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("Invalid", func(t *testing.T) { t.Run("Invalid", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: "invalid", AccessMethod: "invalid",
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
token, ok := workspaceapps.ResolveRequest(rw, r, workspaceapps.ResolveRequestOptions{ token, ok := workspaceapps.ResolveRequest(rw, r, workspaceapps.ResolveRequestOptions{
@ -465,13 +465,13 @@ func Test_ResolveRequest(t *testing.T) {
for _, c := range cases { for _, c := range cases {
t.Run(c.name, func(t *testing.T) { t.Run(c.name, func(t *testing.T) {
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceAndAgent: c.workspaceAndAgent, WorkspaceAndAgent: c.workspaceAndAgent,
AppSlugOrPort: appNamePublic, AppSlugOrPort: appNamePublic,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
@ -510,7 +510,7 @@ func Test_ResolveRequest(t *testing.T) {
t.Parallel() t.Parallel()
badToken := workspaceapps.SignedToken{ badToken := workspaceapps.SignedToken{
Request: workspaceapps.Request{ Request: (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
@ -518,7 +518,7 @@ func Test_ResolveRequest(t *testing.T) {
AgentNameOrID: agentName, AgentNameOrID: agentName,
// App name differs // App name differs
AppSlugOrPort: appNamePublic, AppSlugOrPort: appNamePublic,
}, }).Normalize(),
Expiry: time.Now().Add(time.Minute), Expiry: time.Now().Add(time.Minute),
UserID: me.ID, UserID: me.ID,
WorkspaceID: workspace.ID, WorkspaceID: workspace.ID,
@ -528,7 +528,7 @@ func Test_ResolveRequest(t *testing.T) {
badTokenStr, err := api.AppSecurityKey.SignToken(badToken) badTokenStr, err := api.AppSecurityKey.SignToken(badToken)
require.NoError(t, err) require.NoError(t, err)
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
@ -536,13 +536,13 @@ func Test_ResolveRequest(t *testing.T) {
AgentNameOrID: agentName, AgentNameOrID: agentName,
// App name differs // App name differs
AppSlugOrPort: appNameOwner, AppSlugOrPort: appNameOwner,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken())
r.AddCookie(&http.Cookie{ r.AddCookie(&http.Cookie{
Name: codersdk.DevURLSignedAppTokenCookie, Name: codersdk.SignedAppTokenCookie,
Value: badTokenStr, Value: badTokenStr,
}) })
@ -566,7 +566,7 @@ func Test_ResolveRequest(t *testing.T) {
_ = w.Body.Close() _ = w.Body.Close()
cookies := w.Cookies() cookies := w.Cookies()
require.Len(t, cookies, 1) require.Len(t, cookies, 1)
require.Equal(t, cookies[0].Name, codersdk.DevURLSignedAppTokenCookie) require.Equal(t, cookies[0].Name, codersdk.SignedAppTokenCookie)
require.NotEqual(t, cookies[0].Value, badTokenStr) require.NotEqual(t, cookies[0].Value, badTokenStr)
parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookies[0].Value) parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookies[0].Value)
require.NoError(t, err) require.NoError(t, err)
@ -576,14 +576,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("PortPathBlocked", func(t *testing.T) { t.Run("PortPathBlocked", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: "8080", AppSlugOrPort: "8080",
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
@ -604,14 +604,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("PortSubdomain", func(t *testing.T) { t.Run("PortSubdomain", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain, AccessMethod: workspaceapps.AccessMethodSubdomain,
BasePath: "/", BasePath: "/",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: "9090", AppSlugOrPort: "9090",
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/", nil) r := httptest.NewRequest("GET", "/", nil)
@ -633,11 +633,11 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("Terminal", func(t *testing.T) { t.Run("Terminal", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodTerminal, AccessMethod: workspaceapps.AccessMethodTerminal,
BasePath: "/app", BasePath: "/app",
AgentNameOrID: agentID.String(), AgentNameOrID: agentID.String(),
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
@ -664,14 +664,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("InsufficientPermissions", func(t *testing.T) { t.Run("InsufficientPermissions", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: appNameOwner, AppSlugOrPort: appNameOwner,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
@ -691,14 +691,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("UserNotFound", func(t *testing.T) { t.Run("UserNotFound", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: "thisuserdoesnotexist", UsernameOrID: "thisuserdoesnotexist",
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: appNameOwner, AppSlugOrPort: appNameOwner,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
@ -719,14 +719,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("RedirectSubdomainAuth", func(t *testing.T) { t.Run("RedirectSubdomainAuth", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain, AccessMethod: workspaceapps.AccessMethodSubdomain,
BasePath: "/", BasePath: "/",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: appNameOwner, AppSlugOrPort: appNameOwner,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/some-path", nil) r := httptest.NewRequest("GET", "/some-path", nil)
@ -771,14 +771,14 @@ func Test_ResolveRequest(t *testing.T) {
t.Run("UnhealthyAgent", func(t *testing.T) { t.Run("UnhealthyAgent", func(t *testing.T) {
t.Parallel() t.Parallel()
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentNameUnhealthy, AgentNameOrID: agentNameUnhealthy,
AppSlugOrPort: appNameAgentUnhealthy, AppSlugOrPort: appNameAgentUnhealthy,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)
@ -832,14 +832,14 @@ func Test_ResolveRequest(t *testing.T) {
return false return false
}, testutil.WaitLong, testutil.IntervalFast, "wait for app to become unhealthy") }, testutil.WaitLong, testutil.IntervalFast, "wait for app to become unhealthy")
req := workspaceapps.Request{ req := (workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath, AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app", BasePath: "/app",
UsernameOrID: me.Username, UsernameOrID: me.Username,
WorkspaceNameOrID: workspace.Name, WorkspaceNameOrID: workspace.Name,
AgentNameOrID: agentName, AgentNameOrID: agentName,
AppSlugOrPort: appNameUnhealthy, AppSlugOrPort: appNameUnhealthy,
} }).Normalize()
rw := httptest.NewRecorder() rw := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/app", nil) r := httptest.NewRequest("GET", "/app", nil)

View File

@ -7,7 +7,6 @@ import (
"time" "time"
"cdr.dev/slog" "cdr.dev/slog"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk"
) )
@ -58,7 +57,7 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
AppRequest: appReq, AppRequest: appReq,
PathAppBaseURL: opts.PathAppBaseURL.String(), PathAppBaseURL: opts.PathAppBaseURL.String(),
AppHostname: opts.AppHostname, AppHostname: opts.AppHostname,
SessionToken: httpmw.APITokenFromRequest(r), SessionToken: AppConnectSessionTokenFromRequest(r, appReq.AccessMethod),
AppPath: opts.AppPath, AppPath: opts.AppPath,
AppQuery: opts.AppQuery, AppQuery: opts.AppQuery,
} }
@ -68,11 +67,16 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
return nil, false return nil, false
} }
// Write the signed app token cookie. We always want this to apply to the // Write the signed app token cookie.
// current hostname (even for subdomain apps, without any wildcard //
// shenanigans, because the token is only valid for a single app). // For path apps, this applies to only the path app base URL on the current
// domain, e.g.
// /@user/workspace[.agent]/apps/path-app/
//
// For subdomain apps, this applies to the entire subdomain, e.g.
// app--agent--workspace--user.apps.example.com
http.SetCookie(rw, &http.Cookie{ http.SetCookie(rw, &http.Cookie{
Name: codersdk.DevURLSignedAppTokenCookie, Name: codersdk.SignedAppTokenCookie,
Value: tokenStr, Value: tokenStr,
Path: appReq.BasePath, Path: appReq.BasePath,
Expires: token.Expiry, Expires: token.Expiry,

View File

@ -220,8 +220,12 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request,
// We don't set an expiration because the key in the database already has an // We don't set an expiration because the key in the database already has an
// expiration, and expired tokens don't affect the user experience (they get // expiration, and expired tokens don't affect the user experience (they get
// auto-redirected to re-smuggle the API key). // auto-redirected to re-smuggle the API key).
//
// We use different cookie names for path apps and for subdomain apps to
// avoid both being set and sent to the server at the same time and the
// server using the wrong value.
http.SetCookie(rw, &http.Cookie{ http.SetCookie(rw, &http.Cookie{
Name: codersdk.DevURLSessionTokenCookie, Name: AppConnectSessionTokenCookieName(accessMethod),
Value: token, Value: token,
Domain: domain, Domain: domain,
Path: "/", Path: "/",

View File

@ -103,6 +103,10 @@ func (r Request) Normalize() Request {
} }
} }
if !strings.HasSuffix(req.BasePath, "/") {
req.BasePath += "/"
}
return req return req
} }

View File

@ -15,6 +15,7 @@ func Test_RequestValidate(t *testing.T) {
cases := []struct { cases := []struct {
name string name string
req workspaceapps.Request req workspaceapps.Request
noNormalize bool
errContains string errContains string
}{ }{
{ {
@ -90,6 +91,7 @@ func Test_RequestValidate(t *testing.T) {
AgentNameOrID: "baz", AgentNameOrID: "baz",
AppSlugOrPort: "qux", AppSlugOrPort: "qux",
}, },
noNormalize: true,
errContains: "base path is required", errContains: "base path is required",
}, },
{ {
@ -215,7 +217,10 @@ func Test_RequestValidate(t *testing.T) {
c := c c := c
t.Run(c.name, func(t *testing.T) { t.Run(c.name, func(t *testing.T) {
t.Parallel() t.Parallel()
req := c.req.Normalize() req := c.req
if !c.noNormalize {
req = c.req.Normalize()
}
err := req.Validate() err := req.Validate()
if c.errContains == "" { if c.errContains == "" {
require.NoError(t, err) require.NoError(t, err)

View File

@ -5,6 +5,7 @@ import (
"encoding/hex" "encoding/hex"
"encoding/json" "encoding/json"
"net/http" "net/http"
"strings"
"time" "time"
"github.com/go-jose/go-jose/v3" "github.com/go-jose/go-jose/v3"
@ -38,8 +39,17 @@ type SignedToken struct {
// MatchesRequest returns true if the token matches the request. Any token that // MatchesRequest returns true if the token matches the request. Any token that
// does not match the request should be considered invalid. // does not match the request should be considered invalid.
func (t SignedToken) MatchesRequest(req Request) bool { func (t SignedToken) MatchesRequest(req Request) bool {
tokenBasePath := t.Request.BasePath
if !strings.HasSuffix(tokenBasePath, "/") {
tokenBasePath += "/"
}
reqBasePath := req.BasePath
if !strings.HasSuffix(reqBasePath, "/") {
reqBasePath += "/"
}
return t.AccessMethod == req.AccessMethod && return t.AccessMethod == req.AccessMethod &&
t.BasePath == req.BasePath && tokenBasePath == reqBasePath &&
t.UsernameOrID == req.UsernameOrID && t.UsernameOrID == req.UsernameOrID &&
t.WorkspaceNameOrID == req.WorkspaceNameOrID && t.WorkspaceNameOrID == req.WorkspaceNameOrID &&
t.AgentNameOrID == req.AgentNameOrID && t.AgentNameOrID == req.AgentNameOrID &&
@ -227,22 +237,39 @@ func (k SecurityKey) DecryptAPIKey(encryptedAPIKey string) (string, error) {
// FromRequest returns the signed token from the request, if it exists and is // FromRequest returns the signed token from the request, if it exists and is
// valid. The caller must check that the token matches the request. // valid. The caller must check that the token matches the request.
func FromRequest(r *http.Request, key SecurityKey) (*SignedToken, bool) { func FromRequest(r *http.Request, key SecurityKey) (*SignedToken, bool) {
// Get the token string from the request. We usually use a cookie for this, // Get all signed app tokens from the request. This includes the query
// but for web terminal we also support a query parameter to support // parameter and all matching cookies sent with the request. If there are
// cross-domain terminal access. // somehow multiple signed app token cookies, we want to try all of them
tokenStr := "" // (up to 4). The first one that is valid is used.
tokenCookie, cookieErr := r.Cookie(codersdk.DevURLSignedAppTokenCookie) //
if cookieErr == nil { // Browsers will send all cookies in the request, even if there are multiple
tokenStr = tokenCookie.Value // with the same name on different paths.
} else { //
tokenStr = r.URL.Query().Get(codersdk.SignedAppTokenQueryParameter) // If using a query parameter the request MUST be a terminal request. We use
// this to support cross-domain terminal access for the web terminal.
var (
tokens = []string{}
hasQueryParam = false
)
if q := r.URL.Query().Get(codersdk.SignedAppTokenQueryParameter); q != "" {
hasQueryParam = true
tokens = append(tokens, q)
}
for _, cookie := range r.Cookies() {
if cookie.Name == codersdk.SignedAppTokenCookie {
tokens = append(tokens, cookie.Value)
}
} }
if tokenStr != "" { if len(tokens) > 4 {
tokens = tokens[:4]
}
for _, tokenStr := range tokens {
token, err := key.VerifySignedToken(tokenStr) token, err := key.VerifySignedToken(tokenStr)
if err == nil { if err == nil {
req := token.Request.Normalize() req := token.Request.Normalize()
if cookieErr != nil && req.AccessMethod != AccessMethodTerminal { if hasQueryParam && req.AccessMethod != AccessMethodTerminal {
// The request must be a terminal request if we're using a // The request must be a terminal request if we're using a
// query parameter. // query parameter.
return nil, false return nil, false

View File

@ -2,9 +2,13 @@ package workspaceapps_test
import ( import (
"fmt" "fmt"
"net/http"
"net/http/httptest"
"testing" "testing"
"time" "time"
"github.com/coder/coder/v2/codersdk"
"github.com/go-jose/go-jose/v3" "github.com/go-jose/go-jose/v3"
"github.com/google/uuid" "github.com/google/uuid"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -46,6 +50,29 @@ func Test_TokenMatchesRequest(t *testing.T) {
}, },
want: true, want: true,
}, },
{
name: "NormalizePath",
req: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath,
BasePath: "/app",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
token: workspaceapps.SignedToken{
Request: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodPath,
// With trailing slash
BasePath: "/app/",
UsernameOrID: "foo",
WorkspaceNameOrID: "bar",
AgentNameOrID: "baz",
AppSlugOrPort: "qux",
},
},
want: true,
},
{ {
name: "DifferentAccessMethod", name: "DifferentAccessMethod",
req: workspaceapps.Request{ req: workspaceapps.Request{
@ -283,6 +310,62 @@ func Test_GenerateToken(t *testing.T) {
} }
} }
func Test_FromRequest(t *testing.T) {
t.Parallel()
t.Run("MultipleTokens", func(t *testing.T) {
t.Parallel()
r := httptest.NewRequest("GET", "/", nil)
// Add an invalid token
r.AddCookie(&http.Cookie{
Name: codersdk.SignedAppTokenCookie,
Value: "invalid",
})
token := workspaceapps.SignedToken{
Request: workspaceapps.Request{
AccessMethod: workspaceapps.AccessMethodSubdomain,
BasePath: "/",
UsernameOrID: "user",
WorkspaceAndAgent: "workspace/agent",
WorkspaceNameOrID: "workspace",
AgentNameOrID: "agent",
AppSlugOrPort: "app",
},
Expiry: time.Now().Add(time.Hour),
UserID: uuid.New(),
WorkspaceID: uuid.New(),
AgentID: uuid.New(),
AppURL: "/",
}
// Add an expired cookie
expired := token
expired.Expiry = time.Now().Add(time.Hour * -1)
expiredStr, err := coderdtest.AppSecurityKey.SignToken(token)
require.NoError(t, err)
r.AddCookie(&http.Cookie{
Name: codersdk.SignedAppTokenCookie,
Value: expiredStr,
})
// Add a valid token
validStr, err := coderdtest.AppSecurityKey.SignToken(token)
require.NoError(t, err)
r.AddCookie(&http.Cookie{
Name: codersdk.SignedAppTokenCookie,
Value: validStr,
})
signed, ok := workspaceapps.FromRequest(r, coderdtest.AppSecurityKey)
require.True(t, ok, "expected a token to be found")
// Confirm it is the correct token.
require.Equal(t, signed.UserID, token.UserID)
})
}
// The ParseToken fn is tested quite thoroughly in the GenerateToken test as // The ParseToken fn is tested quite thoroughly in the GenerateToken test as
// well. // well.
func Test_ParseToken(t *testing.T) { func Test_ParseToken(t *testing.T) {

View File

@ -38,15 +38,19 @@ const (
// OAuth2RedirectCookie is the name of the cookie that stores the oauth2 redirect. // OAuth2RedirectCookie is the name of the cookie that stores the oauth2 redirect.
OAuth2RedirectCookie = "oauth_redirect" OAuth2RedirectCookie = "oauth_redirect"
// DevURLSessionTokenCookie is the name of the cookie that stores a devurl // PathAppSessionTokenCookie is the name of the cookie that stores an
// token on app domains. // application-scoped API token on workspace proxy path app domains.
//nolint:gosec //nolint:gosec
DevURLSessionTokenCookie = "coder_devurl_session_token" PathAppSessionTokenCookie = "coder_path_app_session_token"
// DevURLSignedAppTokenCookie is the name of the cookie that stores a // SubdomainAppSessionTokenCookie is the name of the cookie that stores an
// temporary JWT that can be used to authenticate instead of the session // application-scoped API token on subdomain app domains (both the primary
// token. // and proxies).
//nolint:gosec //nolint:gosec
DevURLSignedAppTokenCookie = "coder_devurl_signed_app_token" SubdomainAppSessionTokenCookie = "coder_subdomain_app_session_token"
// SignedAppTokenCookie is the name of the cookie that stores a temporary
// JWT that can be used to authenticate instead of the app session token.
//nolint:gosec
SignedAppTokenCookie = "coder_signed_app_token"
// SignedAppTokenQueryParameter is the name of the query parameter that // SignedAppTokenQueryParameter is the name of the query parameter that
// stores a temporary JWT that can be used to authenticate instead of the // stores a temporary JWT that can be used to authenticate instead of the
// session token. This is only acceptable on reconnecting-pty requests, not // session token. This is only acceptable on reconnecting-pty requests, not