Skip to content

[fga] WorkspaceService.getWorkspace(s) #18565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 24, 2023
Merged

[fga] WorkspaceService.getWorkspace(s) #18565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eaf66ba
[server] WorkspaceService.getWorkspace(s)
geropl Aug 21, 2023
68926c3
review comments
geropl Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 30 additions & 38 deletions components/server/src/workspace/gitpod-server-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,11 +846,11 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

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

const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const latestInstancePromise = this.workspaceDb.trace(ctx).findCurrentInstance(workspaceId);
const result = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace, latestInstance } = result;

const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
await this.guardAccess({ kind: "workspace", subject: workspace, teamMembers: teamMembers }, "get");
const latestInstance = await latestInstancePromise;
if (!!latestInstance) {
await this.guardAccess(
{
Expand All @@ -864,8 +864,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
}

return {
workspace,
latestInstance: this.censorInstance(latestInstance),
...result,
latestInstance: this.censorInstance(result.latestInstance),
};
}

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

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

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

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

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

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

// (gpl) We keep this check here for backwards compatibility, it should be superfluous in the future
const instance = await this.workspaceService.getCurrentInstance(user.id, workspace.id);
if (instance && instance.status.phase !== "stopped") {
traceWI(ctx, { instanceId: instance.id });

Expand Down Expand Up @@ -951,7 +947,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
const user = await this.checkUser("stopWorkspace", undefined, { workspaceId });
const logCtx = { userId: user.id, workspaceId };

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

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

const ws = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: ws }, "update");
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "update");

switch (action) {
case "pin":
ws.pinned = true;
workspace.pinned = true;
break;
case "unpin":
ws.pinned = false;
workspace.pinned = false;
break;
case "toggle":
ws.pinned = !ws.pinned;
workspace.pinned = !workspace.pinned;
break;
}

await this.workspaceService.setPinned(user.id, ws.id, ws.pinned);
await this.workspaceService.setPinned(user.id, workspace.id, workspace.pinned);
}

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

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

const ws = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: ws }, "delete");
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "delete");

await this.workspaceService.deleteWorkspace(user.id, workspaceId, "user");
}
Expand All @@ -1067,7 +1063,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

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

const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "update");

await this.workspaceService.setDescription(user.id, workspaceId, description);
Expand All @@ -1081,22 +1077,17 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

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

const res = await this.workspaceDb.trace(ctx).find({
limit: 20,
...options,
userId: user.id,
includeHeadless: false,
});
await Promise.all(res.map((ws) => this.guardAccess({ kind: "workspace", subject: ws.workspace }, "get")));
const result = await this.workspaceService.getWorkspaces(user.id, options);
await Promise.all(result.map((ws) => this.guardAccess({ kind: "workspace", subject: ws.workspace }, "get")));
await Promise.all(
res.map((ws) =>
result.map((ws) =>
this.guardAccess(
{ kind: "workspaceInstance", subject: ws.latestInstance, workspace: ws.workspace },
"get",
),
),
);
return res;
return result;
}

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

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

const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
return user.id == workspace.ownerId;
}
Expand All @@ -1128,7 +1119,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

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

const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");

try {
Expand All @@ -1149,7 +1140,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

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

const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");

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

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

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

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

const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
const runningInstance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
if (!runningInstance) {
log.debug({ userId: user.id, workspaceId }, "Cannot open port for workspace with no running instance", {
Expand Down Expand Up @@ -1889,12 +1880,13 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
});
}

// TODO(gpl): Remove after FGA rollout
private async internGetCurrentWorkspaceInstance(
ctx: TraceContext,
user: User,
workspaceId: string,
): Promise<{ workspace: Workspace; instance: WorkspaceInstance | undefined }> {
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);

const instance = await this.workspaceDb.trace(ctx).findRunningInstance(workspaceId);
return { instance, workspace };
Expand Down Expand Up @@ -2017,7 +2009,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

async getWorkspaceEnvVars(ctx: TraceContext, workspaceId: string): Promise<EnvVarWithValue[]> {
const user = await this.checkUser("getWorkspaceEnvVars");
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
const envVars = await this.envVarService.resolveEnvVariables(
workspace.ownerId,
Expand Down
57 changes: 44 additions & 13 deletions components/server/src/workspace/workspace-service.spec.db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ describe("WorkspaceService", async () => {
const svc = container.get(WorkspaceService);
const ws = await createTestWorkspace(svc, org, owner, project);

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

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

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

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

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

const strangerFoundWorkspace = await svc.getWorkspace(stranger.id, ws.id);
expect(strangerFoundWorkspace?.id, "stranger has access to shared workspace").to.equal(ws.id);
const { workspace: strangerWs } = await svc.getWorkspace(stranger.id, ws.id);
expect(strangerWs.id, "stranger has access to shared workspace").to.equal(ws.id);
});

it("should getWorkspaces", async () => {
const svc = container.get(WorkspaceService);
await createTestWorkspace(svc, org, owner, project);

const ownerResult = await svc.getWorkspaces(owner.id, {});
expect(ownerResult).to.have.lengthOf(1);

const memberResult = await svc.getWorkspaces(member.id, {});
expect(memberResult).to.have.lengthOf(0);

const strangerResult = await svc.getWorkspaces(stranger.id, {});
expect(strangerResult).to.have.lengthOf(0);
});

it("should getWorkspaces - shared", async () => {
const svc = container.get(WorkspaceService);
const ws = await createTestWorkspace(svc, org, owner, project);

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

const ownerResult = await svc.getWorkspaces(owner.id, {});
expect(ownerResult, "owner").to.have.lengthOf(1);

// getWorkspaces is limited to the user's own workspaces atm
const memberResult = await svc.getWorkspaces(member.id, {});
expect(memberResult, "member").to.have.lengthOf(0);

const strangerResult = await svc.getWorkspaces(stranger.id, {});
expect(strangerResult, "stranger").to.have.lengthOf(0);
});

it("should getOwnerToken", async () => {
Expand Down Expand Up @@ -299,7 +330,7 @@ describe("WorkspaceService", async () => {
// svc.getWorkspace(owner.id, ws.id),
// "getWorkspace should return NOT_FOUND after deletion",
// );
const ws2 = await svc.getWorkspace(owner.id, ws.id);
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
expect(ws2.softDeleted, "workspace should be marked as 'softDeleted'").to.equal("user");
});

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

await expectError(ErrorCodes.NOT_FOUND, svc.setPinned(stranger.id, ws.id, true));
await svc.setPinned(owner.id, ws.id, true);
const ws2 = await svc.getWorkspace(owner.id, ws.id);
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
expect(ws2.pinned, "workspace should be pinned").to.equal(true);
});

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

await svc.setDescription(owner.id, ws.id, desc);
const ws2 = await svc.getWorkspace(owner.id, ws.id);
const { workspace: ws2 } = await svc.getWorkspace(owner.id, ws.id);
expect(ws2.description).to.equal(desc);

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

// owner can share workspace
await svc.controlAdmission(owner.id, ws.id, "everyone");
const wsActual = await svc.getWorkspace(owner.id, ws.id);
const { workspace: wsActual } = await svc.getWorkspace(owner.id, ws.id);
expect(wsActual.shareable, "owner should be able to share by default").to.equal(true);
});

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

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

Expand Down
Loading