diff --git a/coderd/workspaceapps/apptest/apptest.go b/coderd/workspaceapps/apptest/apptest.go index d73336cedc..f0993e8f02 100644 --- a/coderd/workspaceapps/apptest/apptest.go +++ b/coderd/workspaceapps/apptest/apptest.go @@ -1247,6 +1247,34 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) { assertWorkspaceLastUsedAtNotUpdated(t, appDetails, testutil.WaitLong) }) + // Security (PLAT-260): must 404 when the URL username segment + // names a different owner than the resolved workspace. + t.Run("WorkspaceUUIDOwnerMismatchShould404", func(t *testing.T) { + t.Parallel() + + appDetails := setupProxyTest(t, nil) + otherUserClient, otherUser := coderdtest.CreateAnotherUser(t, appDetails.SDKClient, appDetails.FirstUser.OrganizationID, rbac.RoleMember()) + appClient := appDetails.AppClient(t) + appClient.SetSessionToken(otherUserClient.SessionToken()) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + forgedApp := appDetails.Apps.Public + forgedApp.Username = otherUser.Username + forgedApp.WorkspaceName = appDetails.Workspace.ID.String() + + resp, err := requestWithRetries(ctx, t, appClient, http.MethodGet, appDetails.SubdomainAppURL(forgedApp).String(), nil) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusNotFound, resp.StatusCode) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Contains(t, string(body), "404 - Application Not Found") + assertWorkspaceLastUsedAtNotUpdated(t, appDetails, testutil.WaitLong) + }) + t.Run("RedirectsWithSlash", func(t *testing.T) { t.Parallel() diff --git a/coderd/workspaceapps/db_test.go b/coderd/workspaceapps/db_test.go index 1631c7d403..5e341a7bc8 100644 --- a/coderd/workspaceapps/db_test.go +++ b/coderd/workspaceapps/db_test.go @@ -908,6 +908,50 @@ func Test_ResolveRequest(t *testing.T) { require.Len(t, connLogger.ConnectionLogs(), 0) }) + // Security (PLAT-260): a UUID workspace lookup must reject when + // the URL's username segment names a different owner. Otherwise a + // same-owner origin can be spoofed for credentialed cross-origin + // reads. + t.Run("WorkspaceUUIDOwnerMismatch", func(t *testing.T) { + t.Parallel() + + req := (workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: secondUser.Username, + WorkspaceNameOrID: workspace.ID.String(), + AgentNameOrID: agentName, + AppSlugOrPort: appNamePublic, + }).Normalize() + + connLogger := connectionlog.NewFake() + auditableIP := testutil.RandomIPv6(t) + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + r.RemoteAddr = auditableIP + + token, ok := workspaceappsResolveRequest(t, connLogger, rw, r, workspaceapps.ResolveRequestOptions{ + Logger: api.Logger, + SignedTokenProvider: api.WorkspaceAppsProvider, + DashboardURL: api.AccessURL, + PathAppBaseURL: api.AccessURL, + AppHostname: api.AppHostname, + AppRequest: req, + }) + require.False(t, ok) + require.Nil(t, token) + + w := rw.Result() + defer w.Body.Close() + b, err := io.ReadAll(w.Body) + require.NoError(t, err) + require.Contains(t, string(b), "404 - Application Not Found") + require.Equal(t, http.StatusNotFound, w.StatusCode) + require.Len(t, connLogger.ConnectionLogs(), 0) + }) + t.Run("RedirectSubdomainAuth", func(t *testing.T) { t.Parallel() diff --git a/coderd/workspaceapps/request.go b/coderd/workspaceapps/request.go index 980ec7c3a6..c0c85e74f3 100644 --- a/coderd/workspaceapps/request.go +++ b/coderd/workspaceapps/request.go @@ -248,6 +248,9 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR ) if workspaceID, uuidErr := uuid.Parse(r.WorkspaceNameOrID); uuidErr == nil { workspace, workspaceErr = db.GetWorkspaceByID(ctx, workspaceID) + if workspaceErr == nil && workspace.OwnerID != user.ID { + workspaceErr = sql.ErrNoRows + } } else { workspace, workspaceErr = db.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{ OwnerID: user.ID,