working session tickets

coder · deansheather · Mar 7, 2023 · Feb 10, 2023 · Feb 10, 2023 · Feb 11, 2023
commit e60c6454e609dd4b3238a89cfd8ab5aa13e14fde
diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -264,7 +264,7 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht
 
 			ticket, ok := api.resolveWorkspaceApp(rw, r, workspaceAppRequest{
 				AccessMethod:      workspaceAppAccessMethodSubdomain,
-				BasePath:          "",
+				BasePath:          "/",
 				UsernameOrID:      app.Username,
 				WorkspaceNameOrID: app.WorkspaceName,
 				AgentNameOrID:     app.AgentName,

diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go
@@ -8,6 +8,7 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"net/http/cookiejar"
 	"net/http/httputil"
 	"net/url"
 	"strconv"
@@ -400,13 +401,43 @@ func TestWorkspaceAppsProxyPath(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 		defer cancel()
 
-		resp, err := requestWithRetries(ctx, t, client, http.MethodGet, fmt.Sprintf("/@%s/%s/apps/%s/?%s", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery), nil)
+		basePath := fmt.Sprintf("/@%s/%s/apps/%s/", coderdtest.FirstUserParams.Username, workspace.Name, proxyTestAppNameOwner)
+		path := fmt.Sprintf("%s?%s", basePath, proxyTestAppQuery)
+		resp, err := requestWithRetries(ctx, t, client, http.MethodGet, path, nil)
 		require.NoError(t, err)
 		defer resp.Body.Close()
 		body, err := io.ReadAll(resp.Body)
 		require.NoError(t, err)
 		require.Equal(t, proxyTestAppBody, string(body))
 		require.Equal(t, http.StatusOK, resp.StatusCode)
+
+		var sessionTicketCookie *http.Cookie
+		for _, c := range resp.Cookies() {
+			if c.Name == codersdk.DevURLSessionTicketCookie {
+				sessionTicketCookie = c
+				break
+			}
+		}
+		require.NotNil(t, sessionTicketCookie, "no session ticket in response")
+		require.Equal(t, sessionTicketCookie.Path, basePath, "incorrect path on session ticket cookie")
+
+		// Ensure the session ticket cookie is valid.
+		ticketClient := codersdk.New(client.URL)
+		ticketClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect
+		ticketClient.HTTPClient.Transport = client.HTTPClient.Transport
+		u, err := ticketClient.URL.Parse(path)
+		require.NoError(t, err)
+		ticketClient.HTTPClient.Jar, err = cookiejar.New(nil)
+		require.NoError(t, err)
+		ticketClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{sessionTicketCookie})
+
+		resp, err = requestWithRetries(ctx, t, ticketClient, http.MethodGet, path, nil)
+		require.NoError(t, err)
+		defer resp.Body.Close()
+		body, err = io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		require.Equal(t, proxyTestAppBody, string(body))
+		require.Equal(t, http.StatusOK, resp.StatusCode)
 	})
 
 	t.Run("RedirectsMe", func(t *testing.T) {
@@ -879,13 +910,42 @@ func TestWorkspaceAppsProxySubdomain(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 		defer cancel()
 
-		resp, err := requestWithRetries(ctx, t, client, http.MethodGet, proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery), nil)
+		uStr := proxyURL(t, client, proxyTestAppNameOwner, "/", proxyTestAppQuery)
+		u, err := url.Parse(uStr)
+		require.NoError(t, err)
+		resp, err := requestWithRetries(ctx, t, client, http.MethodGet, uStr, nil)
 		require.NoError(t, err)
 		defer resp.Body.Close()
 		body, err := io.ReadAll(resp.Body)
 		require.NoError(t, err)
 		require.Equal(t, proxyTestAppBody, string(body))
 		require.Equal(t, http.StatusOK, resp.StatusCode)
+
+		var sessionTicketCookie *http.Cookie
+		for _, c := range resp.Cookies() {
+			if c.Name == codersdk.DevURLSessionTicketCookie {
+				sessionTicketCookie = c
+				break
+			}
+		}
+		require.NotNil(t, sessionTicketCookie, "no session ticket in response")
+		require.Equal(t, sessionTicketCookie.Path, "/", "incorrect path on session ticket cookie")
+
+		// Ensure the session ticket cookie is valid.
+		ticketClient := codersdk.New(client.URL)
+		ticketClient.HTTPClient.CheckRedirect = client.HTTPClient.CheckRedirect
+		ticketClient.HTTPClient.Transport = client.HTTPClient.Transport
+		ticketClient.HTTPClient.Jar, err = cookiejar.New(nil)
+		require.NoError(t, err)
+		ticketClient.HTTPClient.Jar.SetCookies(u, []*http.Cookie{sessionTicketCookie})
+
+		resp, err = requestWithRetries(ctx, t, ticketClient, http.MethodGet, uStr, nil)
+		require.NoError(t, err)
+		defer resp.Body.Close()
+		body, err = io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		require.Equal(t, proxyTestAppBody, string(body))
+		require.Equal(t, http.StatusOK, resp.StatusCode)
 	})
 
 	t.Run("ProxiesPort", func(t *testing.T) {

diff --git a/coderd/workspaceappsauth.go b/coderd/workspaceappsauth.go
@@ -44,6 +44,9 @@ func (r workspaceAppRequest) Validate() error {
 	if r.AccessMethod != workspaceAppAccessMethodPath && r.AccessMethod != workspaceAppAccessMethodSubdomain {
 		return xerrors.Errorf("invalid access method: %q", r.AccessMethod)
 	}
+	if r.BasePath == "" {
+		return xerrors.New("base path is required")
+	}
 	if r.UsernameOrID == "" {
 		return xerrors.New("username or ID is required")
 	}
@@ -81,23 +84,6 @@ func (api *API) resolveWorkspaceApp(rw http.ResponseWriter, r *http.Request, app
 		return nil, false
 	}
 
-	// Get the existing ticket from the request.
-	ticketCookie, err := r.Cookie(codersdk.DevURLSessionTicketCookie)
-	if err == nil {
-		ticket, err := api.parseWorkspaceAppTicket(ticketCookie.Value)
-		if err == nil {
-			if ticket.MatchesRequest(appReq) {
-				// The request has a ticket, which is a valid ticket signed by
-				// us, and matches the app that the user was trying to access.
-				return nil, true
-			}
-		}
-	}
-
-	// There's no ticket or it's invalid, so we need to check auth using the
-	// session token, validate auth and access to the app, then generate a new
-	// ticket.
-
 	if appReq.WorkspaceAndAgent != "" {
 		// workspace.agent
 		workspaceAndAgent := strings.SplitN(appReq.WorkspaceAndAgent, ".", 2)
@@ -115,6 +101,23 @@ func (api *API) resolveWorkspaceApp(rw http.ResponseWriter, r *http.Request, app
 		}
 	}
 
+	// Get the existing ticket from the request.
+	ticketCookie, err := r.Cookie(codersdk.DevURLSessionTicketCookie)
+	if err == nil {
+		ticket, err := api.parseWorkspaceAppTicket(ticketCookie.Value)
+		if err == nil {
+			if ticket.MatchesRequest(appReq) {
+				// The request has a ticket, which is a valid ticket signed by
+				// us, and matches the app that the user was trying to access.
+				return &ticket, true
+			}
+		}
+	}
+
+	// There's no ticket or it's invalid, so we need to check auth using the
+	// session token, validate auth and access to the app, then generate a new
+	// ticket.
+	//
 	// We use the regular API key extraction middleware here to avoid any
 	// differences in behavior between the two.
 	var (
@@ -583,7 +586,7 @@ func (api *API) parseWorkspaceAppTicket(ticketStr string) (workspaceAppTicket, e
 		return workspaceAppTicket{}, xerrors.Errorf("parse JWS: %w", err)
 	}
 
-	output, err := object.Verify(api.AppSigningKey)
+	output, err := object.Verify(&api.AppSigningKey.PublicKey)
 	if err != nil {
 		return workspaceAppTicket{}, xerrors.Errorf("verify JWS: %w", err)
 	}