Skip to content

[fga] check some admin functions #18562

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 1 commit into from
Aug 21, 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
2 changes: 1 addition & 1 deletion components/gitpod-db/src/team-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface TeamDB extends TransactionalDB<TeamDB> {
limit: number,
orderBy: keyof Team,
orderDir: "ASC" | "DESC",
searchTerm: string,
searchTerm?: string,
): Promise<{ total: number; rows: Team[] }>;
findTeamById(teamId: string): Promise<Team | undefined>;
findTeamByMembershipId(membershipId: string): Promise<Team | undefined>;
Expand Down
10 changes: 7 additions & 3 deletions components/gitpod-db/src/typeorm/team-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,13 @@ export class TeamDBImpl extends TransactionalDBImpl<TeamDB> implements TeamDB {
searchTerm?: string,
): Promise<{ total: number; rows: Team[] }> {
const teamRepo = await this.getTeamRepo();
const queryBuilder = teamRepo
.createQueryBuilder("team")
.where("LOWER(team.name) LIKE LOWER(:searchTerm)", { searchTerm: `%${searchTerm}%` })
let queryBuilder = teamRepo.createQueryBuilder("team");
if (searchTerm) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

queryBuilder = queryBuilder.where("LOWER(team.name) LIKE LOWER(:searchTerm)", {
searchTerm: `%${searchTerm}%`,
});
}
queryBuilder = queryBuilder
.andWhere("deleted = 0")
.andWhere("markedDeleted = 0")
.skip(offset)
Expand Down
27 changes: 27 additions & 0 deletions components/server/src/authorization/authorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { Project, TeamMemberRole } from "@gitpod/gitpod-protocol";
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
import {
AllResourceTypes,
InstallationID,
InstallationPermission,
OrganizationPermission,
Permission,
ProjectPermission,
Expand Down Expand Up @@ -54,6 +56,31 @@ export const SYSTEM_USER = "SYSTEM_USER";
export class Authorizer {
constructor(private authorizer: SpiceDBAuthorizer) {}

async hasPermissionOnInstallation(userId: string, permission: InstallationPermission): Promise<boolean> {
if (userId === SYSTEM_USER) {
return true;
}

const req = v1.CheckPermissionRequest.create({
subject: subject("user", userId),
permission,
resource: object("installation", InstallationID),
consistency,
});

return this.authorizer.check(req, { userId });
}

async checkPermissionOnInstallation(userId: string, permission: InstallationPermission): Promise<void> {
if (await this.hasPermissionOnInstallation(userId, permission)) {
return;
}
throw new ApplicationError(
ErrorCodes.PERMISSION_DENIED,
`User ${userId} does not have permission '${permission}' on the installation.`,
);
}

async hasPermissionOnOrganization(
userId: string,
permission: OrganizationPermission,
Expand Down
5 changes: 3 additions & 2 deletions components/server/src/authorization/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { v1 } from "@authzed/authzed-node";

const InstallationID = "1";
export const InstallationID = "1";

export type ResourceType =
| UserResourceType
Expand Down Expand Up @@ -37,6 +37,7 @@ export type UserPermission =
| "write_info"
| "delete"
| "make_admin"
| "admin_control"
| "read_ssh"
| "write_ssh"
| "read_tokens"
Expand All @@ -48,7 +49,7 @@ export type InstallationResourceType = "installation";

export type InstallationRelation = "member" | "admin";

export type InstallationPermission = "create_organization";
export type InstallationPermission = "create_organization" | "configure";

export type OrganizationResourceType = "organization";

Expand Down
16 changes: 16 additions & 0 deletions components/server/src/orgs/organization-service.spec.db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,20 @@ describe("OrganizationService", async () => {
expect(members.length).to.eq(1);
expect(members.some((m) => m.userId === owner.id && m.role === "owner")).to.be.true;
});

it("should listOrganizations", async () => {
const strangerOrg = await os.createOrganization(stranger.id, "stranger-org");
let orgs = await os.listOrganizations(owner.id, {});
expect(orgs.rows[0].id).to.eq(org.id);
expect(orgs.total).to.eq(1);

orgs = await os.listOrganizations(stranger.id, {});
expect(orgs.rows[0].id).to.eq(strangerOrg.id);
expect(orgs.total).to.eq(1);

orgs = await os.listOrganizations(admin.id, {});
expect(orgs.rows.some((org) => org.id === org.id)).to.be.true;
expect(orgs.rows.some((org) => org.id === strangerOrg.id)).to.be.true;
expect(orgs.total).to.eq(2);
});
});
31 changes: 31 additions & 0 deletions components/server/src/orgs/organization-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,37 @@ export class OrganizationService {
@inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
) {}

async listOrganizations(
userId: string,
req: {
offset?: number;
limit?: number;
orderBy?: keyof Organization;
orderDir?: "asc" | "desc";
searchTerm?: string;
},
): Promise<{ total: number; rows: Organization[] }> {
const result = await this.teamDB.findTeams(
req.offset || 0,
req.limit || 50,
req.orderBy || "creationTime",
req.orderDir === "asc" ? "ASC" : "DESC",
req.searchTerm,
);

await Promise.all(
result.rows.map(async (org) => {
// if the user doesn't see the org, filter it out
if (!(await this.auth.hasPermissionOnOrganization(userId, "read_info", org.id))) {
result.total--;
result.rows = result.rows.filter((o) => o.id !== org.id);
}
}),
);

return result;
}

async listOrganizationsByMember(userId: string, memberId: string): Promise<Organization[]> {
//TODO check if user has access to member
const orgs = await this.teamDB.findTeamsByUser(memberId);
Expand Down
8 changes: 3 additions & 5 deletions components/server/src/user/user-authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import { AuthUser } from "../auth/auth-provider";
import { TokenService } from "./token-service";
import { EmailAddressAlreadyTakenException, SelectAccountException } from "../auth/errors";
import { SelectAccountPayload } from "@gitpod/gitpod-protocol/lib/auth";
import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
import { UserService } from "./user-service";
import { Authorizer } from "../authorization/authorizer";

export interface CreateUserParams {
organizationId?: string;
Expand All @@ -37,14 +37,12 @@ export class UserAuthentication {
@inject(UserDB) private readonly userDb: UserDB,
@inject(UserService) private readonly userService: UserService,
@inject(HostContextProvider) private readonly hostContextProvider: HostContextProvider,
@inject(Authorizer) private readonly authorizer: Authorizer,
) {}

async blockUser(userId: string, targetUserId: string, block: boolean): Promise<User> {
await this.authorizer.checkPermissionOnUser(userId, "admin_control", targetUserId);
const target = await this.userService.findUserById(userId, targetUserId);
if (!target) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found");
}

target.blocked = !!block;
return await this.userDb.storeUser(target);
}
Expand Down
36 changes: 21 additions & 15 deletions components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2625,7 +2625,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
): Promise<AdminGetListResult<BlockedRepository>> {
traceAPIParams(ctx, { req: censor(req, "searchTerm") }); // searchTerm may contain PII

await this.guardAdminAccess("adminGetBlockedRepositories", { req }, Permission.ADMIN_USERS);
const admin = await this.guardAdminAccess("adminGetBlockedRepositories", { req }, Permission.ADMIN_USERS);
await this.auth.checkPermissionOnInstallation(admin.id, "configure");

try {
const res = await this.blockedRepostoryDB.findAllBlockedRepositories(
Expand All @@ -2648,15 +2649,21 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
): Promise<BlockedRepository> {
traceAPIParams(ctx, { urlRegexp, blockUser });

await this.guardAdminAccess("adminCreateBlockedRepository", { urlRegexp, blockUser }, Permission.ADMIN_USERS);
const admin = await this.guardAdminAccess(
"adminCreateBlockedRepository",
{ urlRegexp, blockUser },
Permission.ADMIN_USERS,
);
await this.auth.checkPermissionOnInstallation(admin.id, "configure");

return await this.blockedRepostoryDB.createBlockedRepository(urlRegexp, blockUser);
}

async adminDeleteBlockedRepository(ctx: TraceContext, id: number): Promise<void> {
traceAPIParams(ctx, { id });

await this.guardAdminAccess("adminDeleteBlockedRepository", { id }, Permission.ADMIN_USERS);
const admin = await this.guardAdminAccess("adminDeleteBlockedRepository", { id }, Permission.ADMIN_USERS);
await this.auth.checkPermissionOnInstallation(admin.id, "configure");

await this.blockedRepostoryDB.deleteBlockedRepository(id);
}
Expand Down Expand Up @@ -2698,6 +2705,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

async adminVerifyUser(ctx: TraceContext, userId: string): Promise<User> {
const admin = await this.guardAdminAccess("adminVerifyUser", { id: userId }, Permission.ADMIN_USERS);
await this.auth.checkPermissionOnUser(admin.id, "admin_control", userId);
const user = await this.userService.findUserById(admin.id, userId);

this.verificationService.markVerified(user);
Expand Down Expand Up @@ -2738,6 +2746,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
{ req },
Permission.ADMIN_USERS,
);
await this.auth.checkPermissionOnUser(admin.id, "admin_control", req.id);

const target = await this.userService.findUserById(admin.id, req.id);

const featureSettings: UserFeatureSettings = target.featureFlags || {};
Expand Down Expand Up @@ -2869,15 +2879,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
}

async adminGetTeams(ctx: TraceContext, req: AdminGetListRequest<Team>): Promise<AdminGetListResult<Team>> {
await this.guardAdminAccess("adminGetTeams", { req }, Permission.ADMIN_WORKSPACES);

return await this.teamDB.findTeams(
req.offset,
req.limit,
req.orderBy,
req.orderDir === "asc" ? "ASC" : "DESC",
req.searchTerm as string,
);
const admin = await this.guardAdminAccess("adminGetTeams", { req }, Permission.ADMIN_WORKSPACES);
return this.organizationService.listOrganizations(admin.id, req);
}

async adminGetTeamById(ctx: TraceContext, id: string): Promise<Team | undefined> {
Expand Down Expand Up @@ -3602,16 +3605,17 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
async adminGetBillingMode(ctx: TraceContextWithSpan, attributionId: string): Promise<BillingMode> {
traceAPIParams(ctx, { attributionId });

const user = await this.checkAndBlockUser("adminGetBillingMode");
if (!this.authorizationService.hasPermission(user, Permission.ADMIN_USERS)) {
const admin = await this.checkAndBlockUser("adminGetBillingMode");
if (!this.authorizationService.hasPermission(admin, Permission.ADMIN_USERS)) {
throw new ApplicationError(ErrorCodes.PERMISSION_DENIED, "not allowed");
}

const parsedAttributionId = AttributionId.parse(attributionId);
if (!parsedAttributionId) {
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Unable to parse attributionId");
}
return this.billingModes.getBillingMode(user.id, parsedAttributionId.teamId);
await this.auth.checkPermissionOnOrganization(admin.id, "read_billing", attributionId);
return this.billingModes.getBillingMode(admin.id, parsedAttributionId.teamId);
}

async adminGetCostCenter(ctx: TraceContext, attributionId: string): Promise<CostCenterJSON | undefined> {
Expand Down Expand Up @@ -3679,6 +3683,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
async adminGetBlockedEmailDomains(ctx: TraceContextWithSpan): Promise<EmailDomainFilterEntry[]> {
const user = await this.checkAndBlockUser("adminGetBlockedEmailDomains");
await this.guardAdminAccess("adminGetBlockedEmailDomains", { id: user.id }, Permission.ADMIN_USERS);
await this.auth.checkPermissionOnInstallation(user.id, "configure");
return await this.emailDomainFilterdb.getFilterEntries();
}

Expand All @@ -3688,6 +3693,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
): Promise<void> {
const user = await this.checkAndBlockUser("adminSaveBlockedEmailDomain");
await this.guardAdminAccess("adminSaveBlockedEmailDomain", { id: user.id }, Permission.ADMIN_USERS);
await this.auth.checkPermissionOnInstallation(user.id, "configure");
await this.emailDomainFilterdb.storeFilterEntry(domainFilterentry);
}
}
2 changes: 1 addition & 1 deletion components/spicedb/codegen/codegen.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func GenerateDefinition(schema *compiler.CompiledSchema) string {

import { v1 } from "@authzed/authzed-node";

const InstallationID = "1";
export const InstallationID = "1";

` + resource + `;

Expand Down
6 changes: 6 additions & 0 deletions components/spicedb/schema/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ schema: |-
permission read_info = self + organization->member + organization->owner + installation->admin
permission write_info = self
permission delete = self

permission make_admin = installation->admin + organization->installation_admin

// administrate is for changes such as blocking or verifiying, i.e. things that only admins can do on user
permission admin_control = installation->admin + organization->installation_admin

permission read_ssh = self
permission write_ssh = self

Expand All @@ -36,6 +40,8 @@ schema: |-
// orgs can only be created by installation-level users
permission create_organization = member + admin

// any global runtime configurations, such as the list of blocked repositories
permission configure = admin
}

definition organization {
Expand Down