Skip to content

Commit eaf66ba

Browse files
geroplgeropl
committed
[server] WorkspaceService.getWorkspace(s)
1 parent e234839 commit eaf66ba

File tree

3 files changed

+147
-78
lines changed

3 files changed

+147
-78
lines changed

‎components/server/src/workspace/gitpod-server-impl.ts

Lines changed: 44 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -846,26 +846,26 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
846846

847847
const user = await this.checkUser("getWorkspace");
848848

849-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
850-
const latestInstancePromise = this.workspaceDb.trace(ctx).findCurrentInstance(workspaceId);
851-
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
852-
await this.guardAccess({ kind: "workspace", subject: workspace, teamMembers: teamMembers }, "get");
853-
const latestInstance = await latestInstancePromise;
854-
if (!!latestInstance) {
855-
await this.guardAccess(
856-
{
857-
kind: "workspaceInstance",
858-
subject: latestInstance,
859-
workspace,
860-
teamMembers,
861-
},
862-
"get",
863-
);
864-
}
849+
const result = await this.workspaceService.getWorkspace(user.id, workspaceId, async (workspace, instance) => {
850+
if (instance) {
851+
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
852+
await this.guardAccess(
853+
{
854+
kind: "workspaceInstance",
855+
subject: instance,
856+
workspace,
857+
teamMembers,
858+
},
859+
"get",
860+
);
861+
} else {
862+
return this.guardAccess({ kind: "workspace", subject: workspace }, "get");
863+
}
864+
});
865865

866866
return {
867-
workspace,
868-
latestInstance: this.censorInstance(latestInstance),
867+
...result,
868+
latestInstance: this.censorInstance(result.latestInstance),
869869
};
870870
}
871871

@@ -876,10 +876,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
876876
const user = await this.checkAndBlockUser("getOwnerToken");
877877

878878
//TODO this requests are only here to populate the resource guard check
879-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
880-
if (!workspace) {
881-
throw new Error("owner token not found");
882-
}
879+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
883880
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
884881

885882
const latestInstance = await this.workspaceService.getCurrentInstance(user.id, workspaceId);
@@ -895,7 +892,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
895892
const user = await this.checkAndBlockUser("getIDECredentials");
896893

897894
//TODO this requests are only here to populate the resource guard check
898-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
895+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
899896
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
900897

901898
return await this.workspaceService.getIDECredentials(user.id, workspaceId);
@@ -912,7 +909,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
912909
const user = await this.checkAndBlockUser("startWorkspace", undefined, { workspaceId });
913910

914911
// (gpl) We keep this check here for backwards compatibility, it should be superfluous in the future
915-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
912+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
916913
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
917914

918915
// (gpl) We keep this check here for backwards compatibility, it should be superfluous in the future
@@ -951,7 +948,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
951948
const user = await this.checkUser("stopWorkspace", undefined, { workspaceId });
952949
const logCtx = { userId: user.id, workspaceId };
953950

954-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
951+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
955952
if (workspace.type === "prebuild") {
956953
// If this is a team prebuild, any team member can stop it.
957954
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
@@ -1031,22 +1028,22 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
10311028

10321029
const user = await this.checkAndBlockUser("updateWorkspaceUserPin");
10331030

1034-
const ws = await this.workspaceService.getWorkspace(user.id, workspaceId);
1035-
await this.guardAccess({ kind: "workspace", subject: ws }, "update");
1031+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
1032+
await this.guardAccess({ kind: "workspace", subject: workspace }, "update");
10361033

10371034
switch (action) {
10381035
case "pin":
1039-
ws.pinned = true;
1036+
workspace.pinned = true;
10401037
break;
10411038
case "unpin":
1042-
ws.pinned = false;
1039+
workspace.pinned = false;
10431040
break;
10441041
case "toggle":
1045-
ws.pinned = !ws.pinned;
1042+
workspace.pinned = !workspace.pinned;
10461043
break;
10471044
}
10481045

1049-
await this.workspaceService.setPinned(user.id, ws.id, ws.pinned);
1046+
await this.workspaceService.setPinned(user.id, workspace.id, workspace.pinned);
10501047
}
10511048

10521049
public async deleteWorkspace(ctx: TraceContext, workspaceId: string): Promise<void> {
@@ -1055,8 +1052,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
10551052

10561053
const user = await this.checkAndBlockUser("deleteWorkspace");
10571054

1058-
const ws = await this.workspaceService.getWorkspace(user.id, workspaceId);
1059-
await this.guardAccess({ kind: "workspace", subject: ws }, "delete");
1055+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
1056+
await this.guardAccess({ kind: "workspace", subject: workspace }, "delete");
10601057

10611058
await this.workspaceService.deleteWorkspace(user.id, workspaceId, "user");
10621059
}
@@ -1067,7 +1064,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
10671064

10681065
const user = await this.checkAndBlockUser("setWorkspaceDescription");
10691066

1070-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1067+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
10711068
await this.guardAccess({ kind: "workspace", subject: workspace }, "update");
10721069

10731070
await this.workspaceService.setDescription(user.id, workspaceId, description);
@@ -1081,22 +1078,13 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
10811078

10821079
const user = await this.checkUser("getWorkspaces");
10831080

1084-
const res = await this.workspaceDb.trace(ctx).find({
1085-
limit: 20,
1086-
...options,
1087-
userId: user.id,
1088-
includeHeadless: false,
1081+
return this.workspaceService.getWorkspaces(user.id, options, async (workspace, instance) => {
1082+
if (instance) {
1083+
return this.guardAccess({ kind: "workspaceInstance", subject: instance, workspace: workspace }, "get");
1084+
} else {
1085+
return this.guardAccess({ kind: "workspace", subject: workspace }, "get");
1086+
}
10891087
});
1090-
await Promise.all(res.map((ws) => this.guardAccess({ kind: "workspace", subject: ws.workspace }, "get")));
1091-
await Promise.all(
1092-
res.map((ws) =>
1093-
this.guardAccess(
1094-
{ kind: "workspaceInstance", subject: ws.latestInstance, workspace: ws.workspace },
1095-
"get",
1096-
),
1097-
),
1098-
);
1099-
return res;
11001088
}
11011089

11021090
public async isWorkspaceOwner(ctx: TraceContext, workspaceId: string): Promise<boolean> {
@@ -1105,7 +1093,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
11051093

11061094
const user = await this.checkUser("isWorkspaceOwner", undefined, { workspaceId });
11071095

1108-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1096+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
11091097
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
11101098
return user.id == workspace.ownerId;
11111099
}
@@ -1128,7 +1116,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
11281116

11291117
const user = await this.checkUser("getWorkspaceOwner");
11301118

1131-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1119+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
11321120
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
11331121

11341122
try {
@@ -1149,7 +1137,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
11491137

11501138
const user = await this.checkAndBlockUser("getWorkspaceUsers", undefined, { workspaceId });
11511139

1152-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1140+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
11531141
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
11541142

11551143
// Note: there's no need to try and guard the users below, they're not complete users but just enough to
@@ -1800,7 +1788,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
18001788

18011789
const user = await this.checkAndBlockUser("updateGitStatus");
18021790

1803-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1791+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
18041792
const instance = await this.workspaceService.getCurrentInstance(user.id, workspaceId);
18051793
traceWI(ctx, { instanceId: instance.id });
18061794
await this.guardAccess({ kind: "workspaceInstance", subject: instance, workspace }, "update");
@@ -1818,7 +1806,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
18181806

18191807
const user = await this.checkAndBlockUser("openPort");
18201808

1821-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1809+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
18221810
const runningInstance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
18231811
if (!runningInstance) {
18241812
log.debug({ userId: user.id, workspaceId }, "Cannot open port for workspace with no running instance", {
@@ -1894,7 +1882,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
18941882
user: User,
18951883
workspaceId: string,
18961884
): Promise<{ workspace: Workspace; instance: WorkspaceInstance | undefined }> {
1897-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1885+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
18981886

18991887
const instance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
19001888
return { instance, workspace };
@@ -2017,7 +2005,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
20172005

20182006
async getWorkspaceEnvVars(ctx: TraceContext, workspaceId: string): Promise<EnvVarWithValue[]> {
20192007
const user = await this.checkUser("getWorkspaceEnvVars");
2020-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
2008+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
20212009
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
20222010
const envVars = await this.envVarService.resolveEnvVariables(
20232011
workspace.ownerId,

‎components/server/src/workspace/workspace-service.spec.db.ts

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ describe("WorkspaceService", async () => {
183183
const svc = container.get(WorkspaceService);
184184
const ws = await createTestWorkspace(svc, org, owner, project);
185185

186-
const foundWorkspace = await svc.getWorkspace(owner.id, ws.id);
187-
expect(foundWorkspace?.id).to.equal(ws.id);
186+
const { workspace: ownerWs } = await svc.getWorkspace(owner.id, ws.id);
187+
expect(ownerWs.id).to.equal(ws.id);
188188

189189
await expectError(ErrorCodes.PERMISSION_DENIED, svc.getWorkspace(member.id, ws.id));
190190
await expectError(ErrorCodes.NOT_FOUND, svc.getWorkspace(stranger.id, ws.id));
@@ -196,14 +196,45 @@ describe("WorkspaceService", async () => {
196196

197197
await svc.controlAdmission(owner.id, ws.id, "everyone");
198198

199-
const foundWorkspace = await svc.getWorkspace(owner.id, ws.id);
200-
expect(foundWorkspace?.id, "owner has access to shared workspace").to.equal(ws.id);
199+
const { workspace: ownerWs } = await svc.getWorkspace(owner.id, ws.id);
200+
expect(ownerWs.id, "owner has access to shared workspace").to.equal(ws.id);
201201

202-
const memberFoundWorkspace = await svc.getWorkspace(member.id, ws.id);
203-
expect(memberFoundWorkspace?.id, "member has access to shared workspace").to.equal(ws.id);
202+
const { workspace: memberWs } = await svc.getWorkspace(member.id, ws.id);
203+
expect(memberWs.id, "member has access to shared workspace").to.equal(ws.id);
204204

205-
const strangerFoundWorkspace = await svc.getWorkspace(stranger.id, ws.id);
206-
expect(strangerFoundWorkspace?.id, "stranger has access to shared workspace").to.equal(ws.id);
205+
const { workspace: strangerWs } = await svc.getWorkspace(stranger.id, ws.id);
206+
expect(strangerWs.id, "stranger has access to shared workspace").to.equal(ws.id);
207+
});
208+
209+
it("should getWorkspaces", async () => {
210+
const svc = container.get(WorkspaceService);
211+
await createTestWorkspace(svc, org, owner, project);
212+
213+
const ownerResult = await svc.getWorkspaces(owner.id, {});
214+
expect(ownerResult).to.have.lengthOf(1);
215+
216+
const memberResult = await svc.getWorkspaces(member.id, {});
217+
expect(memberResult).to.have.lengthOf(0);
218+
219+
const strangerResult = await svc.getWorkspaces(stranger.id, {});
220+
expect(strangerResult).to.have.lengthOf(0);
221+
});
222+
223+
it("should getWorkspaces - shared", async () => {
224+
const svc = container.get(WorkspaceService);
225+
const ws = await createTestWorkspace(svc, org, owner, project);
226+
227+
await svc.controlAdmission(owner.id, ws.id, "everyone");
228+
229+
const ownerResult = await svc.getWorkspaces(owner.id, {});
230+
expect(ownerResult, "owner").to.have.lengthOf(1);
231+
232+
// getWorkspaces is limited to the user's own workspaces atm
233+
const memberResult = await svc.getWorkspaces(member.id, {});
234+
expect(memberResult, "member").to.have.lengthOf(0);
235+
236+
const strangerResult = await svc.getWorkspaces(stranger.id, {});
237+
expect(strangerResult, "stranger").to.have.lengthOf(0);
207238
});
208239

209240
it("should getOwnerToken", async () => {
@@ -299,7 +330,7 @@ describe("WorkspaceService", async () => {
299330
// svc.getWorkspace(owner.id, ws.id),
300331
// "getWorkspace should return NOT_FOUND after deletion",
301332
// );
302-
const ws2 = await svc.getWorkspace(owner.id, ws.id);
333+
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
303334
expect(ws2.softDeleted, "workspace should be marked as 'softDeleted'").to.equal("user");
304335
});
305336

@@ -353,7 +384,7 @@ describe("WorkspaceService", async () => {
353384

354385
await expectError(ErrorCodes.NOT_FOUND, svc.setPinned(stranger.id, ws.id, true));
355386
await svc.setPinned(owner.id, ws.id, true);
356-
const ws2 = await svc.getWorkspace(owner.id, ws.id);
387+
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
357388
expect(ws2.pinned, "workspace should be pinned").to.equal(true);
358389
});
359390

@@ -363,7 +394,7 @@ describe("WorkspaceService", async () => {
363394
const desc = "Some description";
364395

365396
await svc.setDescription(owner.id, ws.id, desc);
366-
const ws2 = await svc.getWorkspace(owner.id, ws.id);
397+
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
367398
expect(ws2.description).to.equal(desc);
368399

369400
await expectError(ErrorCodes.NOT_FOUND, svc.setDescription(stranger.id, ws.id, desc));
@@ -516,7 +547,7 @@ describe("WorkspaceService", async () => {
516547

517548
// owner can share workspace
518549
await svc.controlAdmission(owner.id, ws.id, "everyone");
519-
const wsActual = await svc.getWorkspace(owner.id, ws.id);
550+
const { workspace: wsActual } = await svc.getWorkspace(owner.id, ws.id);
520551
expect(wsActual.shareable, "owner should be able to share by default").to.equal(true);
521552
});
522553

@@ -540,7 +571,7 @@ describe("WorkspaceService", async () => {
540571
"invalid admission level should fail",
541572
);
542573

543-
const wsActual = await svc.getWorkspace(owner.id, ws.id);
574+
const { workspace: wsActual } = await svc.getWorkspace(owner.id, ws.id);
544575
expect(!!wsActual.shareable, "shareable should still be false").to.equal(false);
545576
});
546577

0 commit comments

Comments
 (0)