Skip to content

Commit

Permalink
[server] Restrict snapshot access based on repository access
Browse files Browse the repository at this point in the history
Also refactor:
- Simplify GuardedSnapshot.workspace
- WorkspaceLogAccessGuard → RepositoryResourceGuard
- RepositoryService.canAccessHeadlessLogs → RepositoryProvider.hasReadAccess
  • Loading branch information
jankeromnes committed Feb 18, 2022
1 parent 6f8bbba commit bab69f7
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 111 deletions.
5 changes: 2 additions & 3 deletions components/gitpod-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -815,15 +815,14 @@ export namespace WithSnapshot {
}
}

export interface WithPrebuild {
snapshotBucketId: string;
export interface WithPrebuild extends WithSnapshot {
prebuildWorkspaceId: string;
wasPrebuilt: true;
}
export namespace WithPrebuild {
export function is(context: any): context is WithPrebuild {
return context
&& 'snapshotBucketId' in context
&& WithSnapshot.is(context)
&& 'prebuildWorkspaceId' in context
&& 'wasPrebuilt' in context;
}
Expand Down
3 changes: 0 additions & 3 deletions components/server/ee/src/auth/host-container-mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { injectable, interfaces } from "inversify";
import { HostContainerMapping } from "../../../src/auth/host-container-mapping";
import { gitlabContainerModuleEE } from "../gitlab/container-module";
import { bitbucketContainerModuleEE } from "../bitbucket/container-module";
import { gitHubContainerModuleEE } from "../github/container-module";

@injectable()
export class HostContainerMappingEE extends HostContainerMapping {
Expand All @@ -24,8 +23,6 @@ export class HostContainerMappingEE extends HostContainerMapping {
// case "BitbucketServer":
// FIXME
// return (modules || []).concat([bitbucketContainerModuleEE]);
case "GitHub":
return (modules || []).concat([gitHubContainerModuleEE]);
default:
return modules;
}
Expand Down
13 changes: 0 additions & 13 deletions components/server/ee/src/github/container-module.ts

This file was deleted.

36 changes: 0 additions & 36 deletions components/server/ee/src/prebuilds/github-service.ts

This file was deleted.

21 changes: 1 addition & 20 deletions components/server/ee/src/prebuilds/gitlab-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { RepositoryService } from "../../../src/repohost/repo-service";
import { CommitContext, User, WorkspaceContext } from "@gitpod/gitpod-protocol";
import { User } from "@gitpod/gitpod-protocol";
import { inject, injectable } from "inversify";
import { GitLabApi, GitLab } from "../../../src/gitlab/api";
import { AuthProviderParams } from "../../../src/auth/auth-provider";
Expand Down Expand Up @@ -66,25 +66,6 @@ export class GitlabService extends RepositoryService {
log.info('Installed Webhook for ' + cloneUrl, { cloneUrl, userId: user.id });
}

async canAccessHeadlessLogs(user: User, context: WorkspaceContext): Promise<boolean> {
if (!CommitContext.is(context)) {
return false;
}
const { owner, name: repoName } = context.repository;

try {
// If we can "see" a project we are allowed to access it's headless logs
const api = await this.api.create(user);
const response = await api.Projects.show(`${owner}/${repoName}`);
if (GitLab.ApiError.is(response)) {
return false;
}
return true;
} catch (err) {
return false;
}
}

protected getHookUrl() {
return this.config.hostUrl.with({
pathname: GitLabApp.path
Expand Down
4 changes: 2 additions & 2 deletions components/server/ee/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
if (!workspace || workspace.ownerId !== userId) {
throw new ResponseError(ErrorCodes.NOT_FOUND, `Workspace ${workspaceId} does not exist.`);
}
await this.guardAccess({ kind: "snapshot", subject: undefined, workspaceOwnerID: workspace.ownerId, workspaceID: workspace.id }, "create");
await this.guardAccess({ kind: "snapshot", subject: undefined, workspace }, "create");

return workspace;
}
Expand Down Expand Up @@ -406,7 +406,7 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
}

const snapshots = await this.workspaceDb.trace(ctx).findSnapshotsByWorkspaceId(workspaceId);
await Promise.all(snapshots.map(s => this.guardAccess({ kind: "snapshot", subject: s, workspaceOwnerID: workspace.ownerId }, "get")));
await Promise.all(snapshots.map(s => this.guardAccess({ kind: "snapshot", subject: s, workspace }, "get")));

return snapshots.map(s => s.id);
}
Expand Down
6 changes: 3 additions & 3 deletions components/server/src/auth/resource-access.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ import { UserEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
guard: new TokenResourceGuard(workspaceResource.subject.ownerId, [
"resource:" + ScopedResourceGuard.marshalResourceScope({ kind: "snapshot", subjectID: ScopedResourceGuard.SNAPSHOT_WORKSPACE_SUBJECT_ID_PREFIX + workspaceResource.subject.id, operations: ["create"] }),
]),
resource: { kind: "snapshot", subject: undefined, workspaceID: workspaceResource.subject.id, workspaceOwnerID: workspaceResource.subject.ownerId },
resource: { kind: "snapshot", subject: undefined, workspace: workspaceResource.subject },
operation: "create",
expectation: true,
},
Expand All @@ -187,7 +187,7 @@ import { UserEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
guard: new TokenResourceGuard(workspaceResource.subject.ownerId, [
"resource:" + ScopedResourceGuard.marshalResourceScope({ kind: "snapshot", subjectID: workspaceResource.subject.id, operations: ["create"] }),
]),
resource: { kind: "snapshot", subject: undefined, workspaceID: workspaceResource.subject.id, workspaceOwnerID: workspaceResource.subject.ownerId },
resource: { kind: "snapshot", subject: undefined, workspace: workspaceResource.subject },
operation: "create",
expectation: false,
},
Expand All @@ -196,7 +196,7 @@ import { UserEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
guard: new TokenResourceGuard(workspaceResource.subject.ownerId, [
"resource:" + ScopedResourceGuard.marshalResourceScope({ kind: "snapshot", subjectID: workspaceResource.subject.id, operations: ["create"] }),
]),
resource: { kind: "snapshot", subject: undefined, workspaceID: workspaceResource.subject.id, workspaceOwnerID: "other_owner" },
resource: { kind: "snapshot", subject: undefined, workspace: { ...workspaceResource.subject, ownerId: "other_owner" } },
operation: "create",
expectation: false,
},
Expand Down
34 changes: 17 additions & 17 deletions components/server/src/auth/resource-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* See License-AGPL.txt in the project root for license information.
*/

import { ContextURL, GitpodToken, Snapshot, Team, TeamMemberInfo, Token, User, UserEnvVar, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
import { CommitContext, ContextURL, GitpodToken, Snapshot, Team, TeamMemberInfo, Token, User, UserEnvVar, Workspace, WorkspaceInstance } from "@gitpod/gitpod-protocol";
import { HostContextProvider } from "./host-context-provider";

declare var resourceInstance: GuardedResource;
Expand Down Expand Up @@ -61,9 +61,8 @@ export interface GuardedUser {

export interface GuardedSnapshot {
kind: "snapshot";
subject: Snapshot | undefined;
workspaceOwnerID: string;
workspaceID?: string;
subject?: Snapshot;
workspace: Workspace;
}

export interface GuardedUserStorage {
Expand Down Expand Up @@ -177,7 +176,7 @@ export class OwnerResourceGuard implements ResourceAccessGuard {
case "gitpodToken":
return resource.subject.user.id === this.userId;
case "snapshot":
return resource.workspaceOwnerID === this.userId;
return resource.workspace.ownerId === this.userId;
case "token":
return resource.tokenOwnerID === this.userId;
case "user":
Expand Down Expand Up @@ -360,10 +359,7 @@ export namespace ScopedResourceGuard {
if (resource.subject) {
return resource.subject.id;
}
if (resource.workspaceID) {
return SNAPSHOT_WORKSPACE_SUBJECT_ID_PREFIX + resource.workspaceID;
}
return undefined;
return SNAPSHOT_WORKSPACE_SUBJECT_ID_PREFIX + resource.workspace.id;
case "token":
return resource.subject.value;
case "user":
Expand Down Expand Up @@ -464,23 +460,23 @@ export namespace TokenResourceGuard {

}

export class WorkspaceLogAccessGuard implements ResourceAccessGuard {
export class RepositoryResourceGuard implements ResourceAccessGuard {
constructor(
protected readonly user: User,
protected readonly hostContextProvider: HostContextProvider) {}

async canAccess(resource: GuardedResource, operation: ResourceAccessOp): Promise<boolean> {
if (resource.kind !== 'workspaceLog') {
if (resource.kind !== 'workspaceLog' && resource.kind !== 'snapshot') {
return false;
}
// only get operations are supported
if (operation !== 'get') {
return false;
}

// Check if user can access repositories headless logs
const ws = resource.subject;
const contextURL = ContextURL.getNormalizedURL(ws);
// Check if user has at least read access to the repository
const workspace = resource.kind === 'snapshot' ? resource.workspace : resource.subject;
const contextURL = ContextURL.getNormalizedURL(workspace);
if (!contextURL) {
throw new Error(`unable to parse ContextURL: ${contextURL}`);
}
Expand All @@ -489,10 +485,14 @@ export class WorkspaceLogAccessGuard implements ResourceAccessGuard {
throw new Error(`no HostContext found for hostname: ${contextURL.hostname}`);
}

const svcs = hostContext.services;
if (!svcs) {
const services = hostContext.services;
if (!services) {
throw new Error(`no services found in HostContext for hostname: ${contextURL.hostname}`);
}
return svcs.repositoryService.canAccessHeadlessLogs(this.user, ws.context);
if (!CommitContext.is(workspace.context)) {
return false;
}
const { owner, name: repo } = workspace.context.repository;
return services.repositoryProvider.hasReadAccess(this.user, owner, repo);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import { RepositoryProvider } from '../repohost/repository-provider';
@injectable()
export class BitbucketServerRepositoryProvider implements RepositoryProvider {

getUserRepos(user: User): Promise<string[]> {
throw new Error("getUserRepos not implemented.");
}

async getRepo(user: User, owner: string, name: string): Promise<Repository> {
// const api = await this.apiFactory.create(user);
// const repo = (await api.repositories.get({ workspace: owner, repo_slug: name })).data;
Expand Down Expand Up @@ -100,4 +96,13 @@ export class BitbucketServerRepositoryProvider implements RepositoryProvider {
// authorAvatarUrl: commit.author?.user?.links?.avatar?.href,
// };
}

getUserRepos(user: User): Promise<string[]> {
throw new Error("getUserRepos not implemented.");
}

hasReadAccess(user: User, owner: string, repo: string): Promise<boolean> {
throw new Error("Method not implemented.");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { BitbucketApiFactory } from './bitbucket-api-factory';

@injectable()
export class BitbucketRepositoryProvider implements RepositoryProvider {

@inject(BitbucketApiFactory) protected readonly apiFactory: BitbucketApiFactory;

async getRepo(user: User, owner: string, name: string): Promise<Repository> {
Expand Down Expand Up @@ -103,4 +102,9 @@ export class BitbucketRepositoryProvider implements RepositoryProvider {
// FIXME(janx): Not implemented yet
return [];
}

async hasReadAccess(user: User, owner: string, repo: string): Promise<boolean> {
// FIXME(janx): Not implemented yet
return false;
}
}
17 changes: 17 additions & 0 deletions components/server/src/github/github-repository-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,21 @@ export class GithubRepositoryProvider implements RepositoryProvider {
}`);
return (result.data.viewer?.repositoriesContributedTo?.edges || []).map((edge: any) => edge.node.url)
}

async hasReadAccess(user: User, owner: string, repo: string): Promise<boolean> {
try {
// If you have no "viewerPermission" on a repository you may not read it
// Ref: https://docs.github.com/en/graphql/reference/enums#repositorypermission
const result: any = await this.githubQueryApi.runQuery(user, `
query {
repository(name: "${repo}", owner: "${owner}") {
viewerPermission
}
}
`);
return result.data.repository !== null;
} catch (err) {
return false;
}
}
}
14 changes: 14 additions & 0 deletions components/server/src/gitlab/gitlab-repository-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,18 @@ export class GitlabRepositoryProvider implements RepositoryProvider {
// FIXME(janx): Not implemented yet
return [];
}

async hasReadAccess(user: User, owner: string, repo: string): Promise<boolean> {
try {
// If we can "see" a project we are allowed to read it
const api = await this.gitlab.create(user);
const response = await api.Projects.show(`${owner}/${repo}`);
if (GitLab.ApiError.is(response)) {
return false;
}
return true;
} catch (err) {
return false;
}
}
}
5 changes: 1 addition & 4 deletions components/server/src/repohost/repo-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* See License-AGPL.txt in the project root for license information.
*/

import { User, WorkspaceContext } from "@gitpod/gitpod-protocol";
import { User } from "@gitpod/gitpod-protocol";
import { injectable } from "inversify";

@injectable()
Expand All @@ -18,7 +18,4 @@ export class RepositoryService {
throw new Error('unsupported');
}

async canAccessHeadlessLogs(user: User, context: WorkspaceContext): Promise<boolean> {
return false;
}
}
1 change: 1 addition & 0 deletions components/server/src/repohost/repository-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ export interface RepositoryProvider {
getBranches(user: User, owner: string, repo: string): Promise<Branch[]>;
getCommitInfo(user: User, owner: string, repo: string, ref: string): Promise<CommitInfo | undefined>;
getUserRepos(user: User): Promise<string[]>;
hasReadAccess(user: User, owner: string, repo: string): Promise<boolean>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { ErrorCodes as RPCErrorCodes, MessageConnection, ResponseError } from "v
import { AllAccessFunctionGuard, FunctionAccessGuard, WithFunctionAccessGuard } from "../auth/function-access";
import { HostContextProvider } from "../auth/host-context-provider";
import { RateLimiter, RateLimiterConfig, UserRateLimiter } from "../auth/rate-limiter";
import { CompositeResourceAccessGuard, OwnerResourceGuard, ResourceAccessGuard, SharedWorkspaceAccessGuard, TeamMemberResourceGuard, WithResourceAccessGuard, WorkspaceLogAccessGuard } from "../auth/resource-access";
import { CompositeResourceAccessGuard, OwnerResourceGuard, ResourceAccessGuard, SharedWorkspaceAccessGuard, TeamMemberResourceGuard, WithResourceAccessGuard, RepositoryResourceGuard } from "../auth/resource-access";
import { takeFirst } from "../express-util";
import { increaseApiCallCounter, increaseApiConnectionClosedCounter, increaseApiConnectionCounter, increaseApiCallUserCounter, observeAPICallsDuration, apiCallDurationHistogram } from "../prometheus-metrics";
import { GitpodServerImpl } from "../workspace/gitpod-server-impl";
Expand Down Expand Up @@ -191,7 +191,7 @@ export class WebsocketConnectionManager implements ConnectionHandler {
new OwnerResourceGuard(user.id),
new TeamMemberResourceGuard(user.id),
new SharedWorkspaceAccessGuard(),
new WorkspaceLogAccessGuard(user, this.hostContextProvider),
new RepositoryResourceGuard(user, this.hostContextProvider),
]);
} else {
resourceGuard = { canAccess: async () => false };
Expand Down
Loading

0 comments on commit bab69f7

Please sign in to comment.