Skip to content

Commit e01d3f4

Browse files
jdomeracki-codergeokatf0ssel
authored
fix(coderd/workspaceapps): verify workspace owner matches app username (#26085) (#26267)
Cherry-pick backport to `release/2.29`. --------- Co-authored-by: George K <george@coder.com> Co-authored-by: Garrett Delfosse <delfossegarrett@gmail.com>
1 parent 3868714 commit e01d3f4

3 files changed

Lines changed: 75 additions & 0 deletions

File tree

coderd/workspaceapps/apptest/apptest.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,34 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
12471247
assertWorkspaceLastUsedAtNotUpdated(ctx, t, appDetails)
12481248
})
12491249

1250+
// Security (PLAT-260): must 404 when the URL username segment
1251+
// names a different owner than the resolved workspace.
1252+
t.Run("WorkspaceUUIDOwnerMismatchShould404", func(t *testing.T) {
1253+
t.Parallel()
1254+
1255+
appDetails := setupProxyTest(t, nil)
1256+
otherUserClient, otherUser := coderdtest.CreateAnotherUser(t, appDetails.SDKClient, appDetails.FirstUser.OrganizationID, rbac.RoleMember())
1257+
appClient := appDetails.AppClient(t)
1258+
appClient.SetSessionToken(otherUserClient.SessionToken())
1259+
1260+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1261+
defer cancel()
1262+
1263+
forgedApp := appDetails.Apps.Public
1264+
forgedApp.Username = otherUser.Username
1265+
forgedApp.WorkspaceName = appDetails.Workspace.ID.String()
1266+
1267+
resp, err := requestWithRetries(ctx, t, appClient, http.MethodGet, appDetails.SubdomainAppURL(forgedApp).String(), nil)
1268+
require.NoError(t, err)
1269+
defer resp.Body.Close()
1270+
require.Equal(t, http.StatusNotFound, resp.StatusCode)
1271+
1272+
body, err := io.ReadAll(resp.Body)
1273+
require.NoError(t, err)
1274+
require.Contains(t, string(body), "404 - Application Not Found")
1275+
assertWorkspaceLastUsedAtNotUpdated(ctx, t, appDetails)
1276+
})
1277+
12501278
t.Run("RedirectsWithSlash", func(t *testing.T) {
12511279
t.Parallel()
12521280

coderd/workspaceapps/db_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,50 @@ func Test_ResolveRequest(t *testing.T) {
916916
require.Len(t, connLogger.ConnectionLogs(), 0)
917917
})
918918

919+
// Security (PLAT-260): a UUID workspace lookup must reject when
920+
// the URL's username segment names a different owner. Otherwise a
921+
// same-owner origin can be spoofed for credentialed cross-origin
922+
// reads.
923+
t.Run("WorkspaceUUIDOwnerMismatch", func(t *testing.T) {
924+
t.Parallel()
925+
926+
req := (workspaceapps.Request{
927+
AccessMethod: workspaceapps.AccessMethodPath,
928+
BasePath: "/app",
929+
UsernameOrID: secondUser.Username,
930+
WorkspaceNameOrID: workspace.ID.String(),
931+
AgentNameOrID: agentName,
932+
AppSlugOrPort: appNamePublic,
933+
}).Normalize()
934+
935+
connLogger := connectionlog.NewFake()
936+
auditableIP := testutil.RandomIPv6(t)
937+
938+
rw := httptest.NewRecorder()
939+
r := httptest.NewRequest("GET", "/app", nil)
940+
r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken())
941+
r.RemoteAddr = auditableIP
942+
943+
token, ok := workspaceappsResolveRequest(t, connLogger, rw, r, workspaceapps.ResolveRequestOptions{
944+
Logger: api.Logger,
945+
SignedTokenProvider: api.WorkspaceAppsProvider,
946+
DashboardURL: api.AccessURL,
947+
PathAppBaseURL: api.AccessURL,
948+
AppHostname: api.AppHostname,
949+
AppRequest: req,
950+
})
951+
require.False(t, ok)
952+
require.Nil(t, token)
953+
954+
w := rw.Result()
955+
defer w.Body.Close()
956+
b, err := io.ReadAll(w.Body)
957+
require.NoError(t, err)
958+
require.Contains(t, string(b), "404 - Application Not Found")
959+
require.Equal(t, http.StatusNotFound, w.StatusCode)
960+
require.Len(t, connLogger.ConnectionLogs(), 0)
961+
})
962+
919963
t.Run("RedirectSubdomainAuth", func(t *testing.T) {
920964
t.Parallel()
921965

coderd/workspaceapps/request.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,9 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR
249249
)
250250
if workspaceID, uuidErr := uuid.Parse(r.WorkspaceNameOrID); uuidErr == nil {
251251
workspace, workspaceErr = db.GetWorkspaceByID(ctx, workspaceID)
252+
if workspaceErr == nil && workspace.OwnerID != user.ID {
253+
workspaceErr = sql.ErrNoRows
254+
}
252255
} else {
253256
workspace, workspaceErr = db.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
254257
OwnerID: user.ID,

0 commit comments

Comments
 (0)