Skip to content

[server] FGA checks for all admin*Workspace methods #18569

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

Merged
merged 2 commits into from
Aug 24, 2023
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
9 changes: 8 additions & 1 deletion components/server/src/authorization/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,14 @@ export type WorkspaceResourceType = "workspace";

export type WorkspaceRelation = "org" | "owner" | "shared";

export type WorkspacePermission = "access" | "start" | "stop" | "delete" | "read_info" | "create_snapshot";
export type WorkspacePermission =
| "access"
| "start"
| "stop"
| "delete"
| "read_info"
| "create_snapshot"
| "admin_control";

export const rel = {
user(id: string) {
Expand Down
34 changes: 28 additions & 6 deletions components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1988,7 +1988,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

// we use the workspacService which checks if the requesting user has access to the workspace. If that is the case they have access to snapshots as well.
// below is the old permission check which would also check if the user has access to the snapshot itself. This is not the case anymore.
const workspace = await this.workspaceService.getWorkspace(user.id, workspaceId);
const { workspace } = await this.workspaceService.getWorkspace(user.id, workspaceId);
if (workspace.ownerId !== user.id) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, `Workspace ${workspaceId} does not exist.`);
}
Expand Down Expand Up @@ -2785,9 +2785,9 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
): Promise<AdminGetListResult<WorkspaceAndInstance>> {
traceAPIParams(ctx, { req });

await this.guardAdminAccess("adminGetWorkspaces", { req }, Permission.ADMIN_WORKSPACES);
const admin = await this.guardAdminAccess("adminGetWorkspaces", { req }, Permission.ADMIN_WORKSPACES);

return await this.workspaceDb
const wss = await this.workspaceDb
.trace(ctx)
.findAllWorkspaceAndInstances(
req.offset,
Expand All @@ -2796,12 +2796,27 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
req.orderDir === "asc" ? "ASC" : "DESC",
req,
);

await Promise.all(
wss.rows.map(async (row) => {
if (!(await this.auth.hasPermissionOnWorkspace(admin.id, "access", row.workspaceId))) {
wss.total--;
wss.rows = wss.rows.filter((ws) => ws.workspaceId !== row.workspaceId);
}
}),
);
return wss;
}

async adminGetWorkspace(ctx: TraceContext, workspaceId: string): Promise<WorkspaceAndInstance> {
traceAPIParams(ctx, { workspaceId });

await this.guardAdminAccess("adminGetWorkspace", { id: workspaceId }, Permission.ADMIN_WORKSPACES);
const admin = await this.guardAdminAccess(
"adminGetWorkspace",
{ id: workspaceId },
Permission.ADMIN_WORKSPACES,
);
await this.auth.checkPermissionOnWorkspace(admin.id, "access", workspaceId);

const result = await this.workspaceDb.trace(ctx).findWorkspaceAndInstance(workspaceId);
if (!result) {
Expand All @@ -2813,7 +2828,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
async adminGetWorkspaceInstances(ctx: TraceContext, workspaceId: string): Promise<WorkspaceInstance[]> {
traceAPIParams(ctx, { workspaceId });

await this.guardAdminAccess("adminGetWorkspaceInstances", { id: workspaceId }, Permission.ADMIN_WORKSPACES);
const admin = await this.guardAdminAccess(
"adminGetWorkspaceInstances",
{ id: workspaceId },
Permission.ADMIN_WORKSPACES,
);
await this.auth.checkPermissionOnWorkspace(admin.id, "access", workspaceId);

const result = await this.workspaceDb.trace(ctx).findInstances(workspaceId);
return result || [];
Expand All @@ -2827,6 +2847,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
{ id: workspaceId },
Permission.ADMIN_WORKSPACES,
);
await this.auth.checkPermissionOnWorkspace(admin.id, "admin_control", workspaceId);

const workspace = await this.workspaceDb.trace(ctx).findById(workspaceId);
if (workspace) {
Expand All @@ -2844,11 +2865,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
async adminRestoreSoftDeletedWorkspace(ctx: TraceContext, workspaceId: string): Promise<void> {
traceAPIParams(ctx, { workspaceId });

await this.guardAdminAccess(
const admin = await this.guardAdminAccess(
"adminRestoreSoftDeletedWorkspace",
{ id: workspaceId },
Permission.ADMIN_WORKSPACES,
);
await this.auth.checkPermissionOnWorkspace(admin.id, "admin_control", workspaceId);

await this.workspaceDb.trace(ctx).transaction(async (db) => {
const ws = await db.findById(workspaceId);
Expand Down
7 changes: 4 additions & 3 deletions components/spicedb/schema/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ schema: |-
relation shared: user:*

// Whether a user can access a workspace (with an IDE)
//+ (hasAccessToRepository && isPrebuild)
permission access = owner + shared
permission access = owner + shared + org->installation_admin

// Note: All of this is modelled after current behavior.
// There are a lot of improvements we can make here in the light of Organizations, but we explicitly do that as a separate step
Expand All @@ -127,10 +126,12 @@ schema: |-
permission delete = owner

// Whether a user can read basic info/metadata of a workspace
//+ (hasAccessToRepository && isPrebuild)
permission read_info = owner + shared + org->member

permission create_snapshot = owner & org->snapshoter

// Whether someone is allowed to do administrative tasks on a workspace, e.g. un-delete etc.
permission admin_control = org->installation_admin
}
# relationships to be used for assertions & validation
relationships: |-
Expand Down