From 0fe8d247ebc5180ae5eeaefa76ee6aa7d28349b6 Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Tue, 5 Jul 2022 08:21:59 +0000 Subject: [PATCH 1/2] [server] Fix resource guard tests --- .../server/src/auth/resource-access.spec.ts | 36 +++++++++++++++---- components/server/src/auth/resource-access.ts | 19 +++++----- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/components/server/src/auth/resource-access.spec.ts b/components/server/src/auth/resource-access.spec.ts index 4cb20d6172085b..262783fb1bc1a5 100644 --- a/components/server/src/auth/resource-access.spec.ts +++ b/components/server/src/auth/resource-access.spec.ts @@ -18,17 +18,21 @@ import { GuardedWorkspace, CompositeResourceAccessGuard, OwnerResourceGuard, - ResourceAccessGuard, GuardedResourceKind, + RepositoryResourceGuard, + SharedWorkspaceAccessGuard, } from "./resource-access"; -import { User, UserEnvVar, Workspace, WorkspaceType } from "@gitpod/gitpod-protocol/lib/protocol"; +import { PrebuiltWorkspace, User, UserEnvVar, Workspace, WorkspaceType } from "@gitpod/gitpod-protocol/lib/protocol"; import { TeamMemberInfo, TeamMemberRole, WorkspaceInstance } from "@gitpod/gitpod-protocol"; +import { HostContextProvider } from "./host-context-provider"; -class MockedRepositoryResourceGuard implements ResourceAccessGuard { - constructor(protected response: boolean) {} +class MockedRepositoryResourceGuard extends RepositoryResourceGuard { + constructor(protected repositoryAccess: boolean) { + super({} as User, {} as HostContextProvider); + } - async canAccess(resource: GuardedResource, operation: ResourceAccessOp): Promise { - return this.response; + protected async hasAccessToRepos(workspace: Workspace): Promise { + return this.repositoryAccess; } } @@ -588,6 +592,17 @@ class TestResourceAccess { workspaceImage: "gitpod/workspace-full:latest", }; }; + const createPrebuild = (): PrebuiltWorkspace => { + return { + id: "pws-123", + buildWorkspaceId: workspaceId, + cloneURL: "https://github.com/gitpod-io/gitpod", + commit: "sha123123213", + creationTime: new Date(2000, 1, 2).toISOString(), + state: "available", + statusVersion: 1, + }; + }; const tests: { name: string; @@ -911,6 +926,7 @@ class TestResourceAccess { const resourceGuard = new CompositeResourceAccessGuard([ new OwnerResourceGuard(user.id), new TeamMemberResourceGuard(user.id), + new SharedWorkspaceAccessGuard(), new MockedRepositoryResourceGuard(!!t.repositoryAccess), ]); const teamMembers: TeamMemberInfo[] = []; @@ -922,7 +938,7 @@ class TestResourceAccess { }); } - const kind: GuardedResourceKind = "workspaceInstance"; + const kind: GuardedResourceKind = t.resourceKind; let resource: GuardedResource | undefined = undefined; if (kind === "workspaceInstance") { const instance = createInstance(); @@ -931,6 +947,12 @@ class TestResourceAccess { resource = { kind, subject: workspace, teamMembers }; } else if (kind === "workspace") { resource = { kind, subject: workspace, teamMembers }; + } else if (kind === "prebuild") { + if (workspace.type !== "prebuild") { + throw new Error("invalid test data: PWS requires workspace to be of type prebuild!"); + } + const prebuild = createPrebuild(); + resource = { kind, subject: prebuild, workspace, teamMembers }; } if (!resource) { throw new Error("unhandled GuardedResourceKind" + kind); diff --git a/components/server/src/auth/resource-access.ts b/components/server/src/auth/resource-access.ts index b24e48a9f1d01e..1ce0b466ed6b02 100644 --- a/components/server/src/auth/resource-access.ts +++ b/components/server/src/auth/resource-access.ts @@ -163,6 +163,8 @@ export class TeamMemberResourceGuard implements ResourceAccessGuard { return await this.hasAccessToWorkspace(resource.workspace, resource.teamMembers); case "workspaceLog": return await this.hasAccessToWorkspace(resource.subject, resource.teamMembers); + case "prebuild": + return !!resource.teamMembers?.some((m) => m.userId === this.userId); } return false; } @@ -217,17 +219,9 @@ export class OwnerResourceGuard implements ResourceAccessGuard { return resource.members.some((m) => m.userId === this.userId && m.role === "owner"); } case "workspaceLog": - // Owners may do everything, team members can "get" - return ( - resource.subject.ownerId === this.userId || - (operation === "get" && !!resource.teamMembers?.some((m) => m.userId === this.userId)) - ); + return resource.subject.ownerId === this.userId; case "prebuild": - // Owners may do everything, team members can "get" - return ( - resource.workspace.ownerId === this.userId || - (operation === "get" && !!resource.teamMembers?.some((m) => m.userId === this.userId)) - ); + return resource.workspace.ownerId === this.userId; } } } @@ -498,12 +492,17 @@ export class RepositoryResourceGuard implements ResourceAccessGuard { break; case "prebuild": workspace = resource.workspace; + break; default: // We do not handle resource kinds here! return false; } // Check if user has at least read access to the repository + return this.hasAccessToRepos(workspace); + } + + protected async hasAccessToRepos(workspace: Workspace): Promise { const repos: Repository[] = []; if (CommitContext.is(workspace.context)) { repos.push(workspace.context.repository); From 578b07b964a660a46f00ac49a0d1d364dcaf2549 Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Tue, 5 Jul 2022 08:23:48 +0000 Subject: [PATCH 2/2] [server] Align access level of Workspaces of type "prebuild" with Prebuilds --- .../server/src/auth/resource-access.spec.ts | 37 +++++++++++++++++++ components/server/src/auth/resource-access.ts | 7 ++++ 2 files changed, 44 insertions(+) diff --git a/components/server/src/auth/resource-access.spec.ts b/components/server/src/auth/resource-access.spec.ts index 262783fb1bc1a5..02e4770ef8dba0 100644 --- a/components/server/src/auth/resource-access.spec.ts +++ b/components/server/src/auth/resource-access.spec.ts @@ -782,6 +782,43 @@ class TestResourceAccess { teamRole: "owner", expectation: true, }, + // prebuild workspace with repo access + { + name: "prebuild workspace get owner", + resourceKind: "workspace", + workspaceType: "prebuild", + isOwner: true, + teamRole: undefined, + repositoryAccess: true, + expectation: true, + }, + { + name: "prebuild workspace get other", + resourceKind: "workspace", + workspaceType: "prebuild", + isOwner: false, + teamRole: undefined, + repositoryAccess: true, + expectation: true, + }, + { + name: "prebuild workspace get team member", + resourceKind: "workspace", + workspaceType: "prebuild", + isOwner: false, + teamRole: "member", + repositoryAccess: true, + expectation: true, + }, + { + name: "prebuild workspace get team owner (same as member)", + resourceKind: "workspace", + workspaceType: "prebuild", + isOwner: false, + teamRole: "owner", + repositoryAccess: true, + expectation: true, + }, // regular instance { name: "regular workspaceInstance get owner", diff --git a/components/server/src/auth/resource-access.ts b/components/server/src/auth/resource-access.ts index 1ce0b466ed6b02..e299e81740986f 100644 --- a/components/server/src/auth/resource-access.ts +++ b/components/server/src/auth/resource-access.ts @@ -484,6 +484,13 @@ export class RepositoryResourceGuard implements ResourceAccessGuard { // Get Workspace from GuardedResource let workspace: Workspace; switch (resource.kind) { + case "workspace": + workspace = resource.subject; + if (workspace.type !== "prebuild") { + return false; + } + // We're only allowed to access prebuild workspaces with the repository guard + break; case "workspaceLog": workspace = resource.subject; break;