Skip to content

Commit d9dbf82

Browse files
geroplgeropl
committed
[server] WorkspaceService.getWorkspace
1 parent e234839 commit d9dbf82

File tree

3 files changed

+104
-63
lines changed

3 files changed

+104
-63
lines changed

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

Lines changed: 38 additions & 41 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);
@@ -1105,7 +1102,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
11051102

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

1108-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1105+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
11091106
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
11101107
return user.id == workspace.ownerId;
11111108
}
@@ -1128,7 +1125,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
11281125

11291126
const user = await this.checkUser("getWorkspaceOwner");
11301127

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

11341131
try {
@@ -1149,7 +1146,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
11491146

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

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

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

18011798
const user = await this.checkAndBlockUser("updateGitStatus");
18021799

1803-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1800+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
18041801
const instance = await this.workspaceService.getCurrentInstance(user.id, workspaceId);
18051802
traceWI(ctx, { instanceId: instance.id });
18061803
await this.guardAccess({ kind: "workspaceInstance", subject: instance, workspace }, "update");
@@ -1818,7 +1815,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
18181815

18191816
const user = await this.checkAndBlockUser("openPort");
18201817

1821-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1818+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
18221819
const runningInstance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
18231820
if (!runningInstance) {
18241821
log.debug({ userId: user.id, workspaceId }, "Cannot open port for workspace with no running instance", {
@@ -1894,7 +1891,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
18941891
user: User,
18951892
workspaceId: string,
18961893
): Promise<{ workspace: Workspace; instance: WorkspaceInstance | undefined }> {
1897-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
1894+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
18981895

18991896
const instance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
19001897
return { instance, workspace };
@@ -2017,7 +2014,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
20172014

20182015
async getWorkspaceEnvVars(ctx: TraceContext, workspaceId: string): Promise<EnvVarWithValue[]> {
20192016
const user = await this.checkUser("getWorkspaceEnvVars");
2020-
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
2017+
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
20212018
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
20222019
const envVars = await this.envVarService.resolveEnvVariables(
20232020
workspace.ownerId,

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

Lines changed: 40 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,41 @@ 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 getWorkspaceInfo", async () => {
210+
const svc = container.get(WorkspaceService);
211+
const ws = await createTestWorkspace(svc, org, owner, project);
212+
213+
const { workspace: ownerWs } = await svc.getWorkspace(owner.id, ws.id);
214+
expect(ownerWs.id).to.equal(ws.id);
215+
216+
await expectError(ErrorCodes.PERMISSION_DENIED, svc.getWorkspace(member.id, ws.id));
217+
await expectError(ErrorCodes.NOT_FOUND, svc.getWorkspace(stranger.id, ws.id));
218+
});
219+
220+
it("should getWorkspaceInfo - shared", async () => {
221+
const svc = container.get(WorkspaceService);
222+
const ws = await createTestWorkspace(svc, org, owner, project);
223+
224+
await svc.controlAdmission(owner.id, ws.id, "everyone");
225+
226+
const { workspace: ownerWs } = await svc.getWorkspace(owner.id, ws.id);
227+
expect(ownerWs?.id, "owner has access to shared workspace").to.equal(ws.id);
228+
229+
const { workspace: memberWs } = await svc.getWorkspace(member.id, ws.id);
230+
expect(memberWs?.id, "member has access to shared workspace").to.equal(ws.id);
231+
232+
const { workspace: strangerWs } = await svc.getWorkspace(stranger.id, ws.id);
233+
expect(strangerWs?.id, "stranger has access to shared workspace").to.equal(ws.id);
207234
});
208235

209236
it("should getOwnerToken", async () => {
@@ -299,7 +326,7 @@ describe("WorkspaceService", async () => {
299326
// svc.getWorkspace(owner.id, ws.id),
300327
// "getWorkspace should return NOT_FOUND after deletion",
301328
// );
302-
const ws2 = await svc.getWorkspace(owner.id, ws.id);
329+
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
303330
expect(ws2.softDeleted, "workspace should be marked as 'softDeleted'").to.equal("user");
304331
});
305332

@@ -353,7 +380,7 @@ describe("WorkspaceService", async () => {
353380

354381
await expectError(ErrorCodes.NOT_FOUND, svc.setPinned(stranger.id, ws.id, true));
355382
await svc.setPinned(owner.id, ws.id, true);
356-
const ws2 = await svc.getWorkspace(owner.id, ws.id);
383+
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
357384
expect(ws2.pinned, "workspace should be pinned").to.equal(true);
358385
});
359386

@@ -363,7 +390,7 @@ describe("WorkspaceService", async () => {
363390
const desc = "Some description";
364391

365392
await svc.setDescription(owner.id, ws.id, desc);
366-
const ws2 = await svc.getWorkspace(owner.id, ws.id);
393+
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
367394
expect(ws2.description).to.equal(desc);
368395

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

517544
// owner can share workspace
518545
await svc.controlAdmission(owner.id, ws.id, "everyone");
519-
const wsActual = await svc.getWorkspace(owner.id, ws.id);
546+
const { workspace: wsActual } = await svc.getWorkspace(owner.id, ws.id);
520547
expect(wsActual.shareable, "owner should be able to share by default").to.equal(true);
521548
});
522549

@@ -540,7 +567,7 @@ describe("WorkspaceService", async () => {
540567
"invalid admission level should fail",
541568
);
542569

543-
const wsActual = await svc.getWorkspace(owner.id, ws.id);
570+
const { workspace: wsActual } = await svc.getWorkspace(owner.id, ws.id);
544571
expect(!!wsActual.shareable, "shareable should still be false").to.equal(false);
545572
});
546573

0 commit comments

Comments
 (0)