Skip to content
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.

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

[server] Restrict snapshot access based on repository access #8306

Merged
merged 1 commit into from
Feb 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions components/dashboard/src/start/CreateWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ export default class CreateWorkspace extends React.Component<CreateWorkspaceProp
}}>Authorize with {error.data.host}</button>
</div>;
break;
case ErrorCodes.PERMISSION_DENIED:
statusMessage = <p className="text-base text-gitpod-red w-96">Access is not allowed</p>;
break;
case ErrorCodes.USER_BLOCKED:
window.location.href = '/blocked';
return;
Expand Down
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
15 changes: 12 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,10 +196,19 @@ 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,
},
{
name: "snaphshot get",
guard: new TokenResourceGuard(workspaceResource.subject.ownerId, [
"resource:" + ScopedResourceGuard.marshalResourceScope({ kind: "snapshot", subjectID: ScopedResourceGuard.SNAPSHOT_WORKSPACE_SUBJECT_ID_PREFIX + workspaceResource.subject.id, operations: ["get"] }),
]),
resource: { kind: "snapshot", subject: undefined, workspace: workspaceResource.subject },
operation: "get",
expectation: true,
},
]

await Promise.all(tests.map(async t => {
Expand Down
41 changes: 23 additions & 18 deletions components/server/src/auth/resource-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
* 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 { UnauthorizedError } from "../errors";
import { HostContextProvider } from "./host-context-provider";

declare var resourceInstance: GuardedResource;
Expand Down Expand Up @@ -61,9 +62,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 +177,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 +360,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,35 +461,43 @@ 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}`);
}
const hostContext = this.hostContextProvider.get(contextURL.hostname);
if (!hostContext) {
throw new Error(`no HostContext found for hostname: ${contextURL.hostname}`);
}

const svcs = hostContext.services;
if (!svcs) {
const { authProvider } = hostContext;
const identity = User.getIdentity(this.user, authProvider.authProviderId);
if (!identity) {
throw UnauthorizedError.create(contextURL.hostname, authProvider.info.requirements?.default || [], "missing-identity");
}
const { services } = hostContext;
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,15 @@ export class BitbucketServerRepositoryProvider implements RepositoryProvider {
// authorAvatarUrl: commit.author?.user?.links?.avatar?.href,
// };
}

async getUserRepos(user: User): Promise<string[]> {
// TODO(janx): Not implemented yet
return [];
}

async hasReadAccess(user: User, owner: string, repo: string): Promise<boolean> {
// TODO(janx): Not implemented yet
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,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;
}
}
jankeromnes marked this conversation as resolved.
Show resolved Hide resolved
}
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;
}
}
jankeromnes marked this conversation as resolved.
Show resolved Hide resolved
}
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>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

}
Loading