Skip to content

Commit 09cdbba

Browse files
committed
[fga] check some admin functions
1 parent 4bf139e commit 09cdbba

File tree

10 files changed

+116
-27
lines changed

10 files changed

+116
-27
lines changed

components/gitpod-db/src/team-db.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export interface TeamDB extends TransactionalDB<TeamDB> {
2121
limit: number,
2222
orderBy: keyof Team,
2323
orderDir: "ASC" | "DESC",
24-
searchTerm: string,
24+
searchTerm?: string,
2525
): Promise<{ total: number; rows: Team[] }>;
2626
findTeamById(teamId: string): Promise<Team | undefined>;
2727
findTeamByMembershipId(membershipId: string): Promise<Team | undefined>;

components/gitpod-db/src/typeorm/team-db-impl.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,13 @@ export class TeamDBImpl extends TransactionalDBImpl<TeamDB> implements TeamDB {
6565
searchTerm?: string,
6666
): Promise<{ total: number; rows: Team[] }> {
6767
const teamRepo = await this.getTeamRepo();
68-
const queryBuilder = teamRepo
69-
.createQueryBuilder("team")
70-
.where("LOWER(team.name) LIKE LOWER(:searchTerm)", { searchTerm: `%${searchTerm}%` })
68+
let queryBuilder = teamRepo.createQueryBuilder("team");
69+
if (searchTerm) {
70+
queryBuilder = queryBuilder.where("LOWER(team.name) LIKE LOWER(:searchTerm)", {
71+
searchTerm: `%${searchTerm}%`,
72+
});
73+
}
74+
queryBuilder = queryBuilder
7175
.andWhere("deleted = 0")
7276
.andWhere("markedDeleted = 0")
7377
.skip(offset)

components/server/src/authorization/authorizer.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import { Project, TeamMemberRole } from "@gitpod/gitpod-protocol";
1111
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1212
import {
1313
AllResourceTypes,
14+
InstallationID,
15+
InstallationPermission,
1416
OrganizationPermission,
1517
Permission,
1618
ProjectPermission,
@@ -54,6 +56,31 @@ export const SYSTEM_USER = "SYSTEM_USER";
5456
export class Authorizer {
5557
constructor(private authorizer: SpiceDBAuthorizer) {}
5658

59+
async hasPermissionOnInstallation(userId: string, permission: InstallationPermission): Promise<boolean> {
60+
if (userId === SYSTEM_USER) {
61+
return true;
62+
}
63+
64+
const req = v1.CheckPermissionRequest.create({
65+
subject: subject("user", userId),
66+
permission,
67+
resource: object("installation", InstallationID),
68+
consistency,
69+
});
70+
71+
return this.authorizer.check(req, { userId });
72+
}
73+
74+
async checkPermissionOnInstallation(userId: string, permission: InstallationPermission): Promise<void> {
75+
if (await this.hasPermissionOnInstallation(userId, permission)) {
76+
return;
77+
}
78+
throw new ApplicationError(
79+
ErrorCodes.PERMISSION_DENIED,
80+
`User ${userId} does not have permission '${permission}' on the installation.`,
81+
);
82+
}
83+
5784
async hasPermissionOnOrganization(
5885
userId: string,
5986
permission: OrganizationPermission,

components/server/src/authorization/definitions.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

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

11-
const InstallationID = "1";
11+
export const InstallationID = "1";
1212

1313
export type ResourceType =
1414
| UserResourceType
@@ -37,6 +37,7 @@ export type UserPermission =
3737
| "write_info"
3838
| "delete"
3939
| "make_admin"
40+
| "admin_control"
4041
| "read_ssh"
4142
| "write_ssh"
4243
| "read_tokens"
@@ -48,7 +49,7 @@ export type InstallationResourceType = "installation";
4849

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

51-
export type InstallationPermission = "create_organization";
52+
export type InstallationPermission = "create_organization" | "configure";
5253

5354
export type OrganizationResourceType = "organization";
5455

components/server/src/orgs/organization-service.spec.db.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,20 @@ describe("OrganizationService", async () => {
202202
expect(members.length).to.eq(1);
203203
expect(members.some((m) => m.userId === owner.id && m.role === "owner")).to.be.true;
204204
});
205+
206+
it("should listOrganizations", async () => {
207+
const strangerOrg = await os.createOrganization(stranger.id, "stranger-org");
208+
let orgs = await os.listOrganizations(owner.id, {});
209+
expect(orgs.rows[0].id).to.eq(org.id);
210+
expect(orgs.total).to.eq(1);
211+
212+
orgs = await os.listOrganizations(stranger.id, {});
213+
expect(orgs.rows[0].id).to.eq(strangerOrg.id);
214+
expect(orgs.total).to.eq(1);
215+
216+
orgs = await os.listOrganizations(admin.id, {});
217+
expect(orgs.rows.some((org) => org.id === org.id)).to.be.true;
218+
expect(orgs.rows.some((org) => org.id === strangerOrg.id)).to.be.true;
219+
expect(orgs.total).to.eq(2);
220+
});
205221
});

components/server/src/orgs/organization-service.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,37 @@ export class OrganizationService {
3030
@inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
3131
) {}
3232

33+
async listOrganizations(
34+
userId: string,
35+
req: {
36+
offset?: number;
37+
limit?: number;
38+
orderBy?: keyof Organization;
39+
orderDir?: "asc" | "desc";
40+
searchTerm?: string;
41+
},
42+
): Promise<{ total: number; rows: Organization[] }> {
43+
const result = await this.teamDB.findTeams(
44+
req.offset || 0,
45+
req.limit || 50,
46+
req.orderBy || "creationTime",
47+
req.orderDir === "asc" ? "ASC" : "DESC",
48+
req.searchTerm,
49+
);
50+
51+
await Promise.allSettled(
52+
result.rows.map(async (org) => {
53+
// if the user doesn't see the org, filter it out
54+
if (!(await this.auth.hasPermissionOnOrganization(userId, "read_info", org.id))) {
55+
result.total--;
56+
result.rows = result.rows.filter((o) => o.id !== org.id);
57+
}
58+
}),
59+
);
60+
61+
return result;
62+
}
63+
3364
async listOrganizationsByMember(userId: string, memberId: string): Promise<Organization[]> {
3465
//TODO check if user has access to member
3566
const orgs = await this.teamDB.findTeamsByUser(memberId);

components/server/src/user/user-authentication.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { AuthUser } from "../auth/auth-provider";
1414
import { TokenService } from "./token-service";
1515
import { EmailAddressAlreadyTakenException, SelectAccountException } from "../auth/errors";
1616
import { SelectAccountPayload } from "@gitpod/gitpod-protocol/lib/auth";
17-
import { ErrorCodes, ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
1817
import { UserService } from "./user-service";
18+
import { Authorizer } from "../authorization/authorizer";
1919

2020
export interface CreateUserParams {
2121
organizationId?: string;
@@ -37,14 +37,12 @@ export class UserAuthentication {
3737
@inject(UserDB) private readonly userDb: UserDB,
3838
@inject(UserService) private readonly userService: UserService,
3939
@inject(HostContextProvider) private readonly hostContextProvider: HostContextProvider,
40+
@inject(Authorizer) private readonly authorizer: Authorizer,
4041
) {}
4142

4243
async blockUser(userId: string, targetUserId: string, block: boolean): Promise<User> {
44+
await this.authorizer.checkPermissionOnUser(userId, "admin_control", targetUserId);
4345
const target = await this.userService.findUserById(userId, targetUserId);
44-
if (!target) {
45-
throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found");
46-
}
47-
4846
target.blocked = !!block;
4947
return await this.userDb.storeUser(target);
5048
}

components/server/src/workspace/gitpod-server-impl.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2625,7 +2625,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
26252625
): Promise<AdminGetListResult<BlockedRepository>> {
26262626
traceAPIParams(ctx, { req: censor(req, "searchTerm") }); // searchTerm may contain PII
26272627

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

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

2651-
await this.guardAdminAccess("adminCreateBlockedRepository", { urlRegexp, blockUser }, Permission.ADMIN_USERS);
2652+
const admin = await this.guardAdminAccess(
2653+
"adminCreateBlockedRepository",
2654+
{ urlRegexp, blockUser },
2655+
Permission.ADMIN_USERS,
2656+
);
2657+
await this.auth.checkPermissionOnInstallation(admin.id, "configure");
26522658

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

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

2659-
await this.guardAdminAccess("adminDeleteBlockedRepository", { id }, Permission.ADMIN_USERS);
2665+
const admin = await this.guardAdminAccess("adminDeleteBlockedRepository", { id }, Permission.ADMIN_USERS);
2666+
await this.auth.checkPermissionOnInstallation(admin.id, "configure");
26602667

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

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

27032711
this.verificationService.markVerified(user);
@@ -2738,6 +2746,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
27382746
{ req },
27392747
Permission.ADMIN_USERS,
27402748
);
2749+
await this.auth.checkPermissionOnUser(admin.id, "admin_control", req.id);
2750+
27412751
const target = await this.userService.findUserById(admin.id, req.id);
27422752

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

28712881
async adminGetTeams(ctx: TraceContext, req: AdminGetListRequest<Team>): Promise<AdminGetListResult<Team>> {
2872-
await this.guardAdminAccess("adminGetTeams", { req }, Permission.ADMIN_WORKSPACES);
2873-
2874-
return await this.teamDB.findTeams(
2875-
req.offset,
2876-
req.limit,
2877-
req.orderBy,
2878-
req.orderDir === "asc" ? "ASC" : "DESC",
2879-
req.searchTerm as string,
2880-
);
2882+
const admin = await this.guardAdminAccess("adminGetTeams", { req }, Permission.ADMIN_WORKSPACES);
2883+
return this.organizationService.listOrganizations(admin.id, req);
28812884
}
28822885

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

3605-
const user = await this.checkAndBlockUser("adminGetBillingMode");
3606-
if (!this.authorizationService.hasPermission(user, Permission.ADMIN_USERS)) {
3608+
const admin = await this.checkAndBlockUser("adminGetBillingMode");
3609+
if (!this.authorizationService.hasPermission(admin, Permission.ADMIN_USERS)) {
36073610
throw new ApplicationError(ErrorCodes.PERMISSION_DENIED, "not allowed");
36083611
}
36093612

36103613
const parsedAttributionId = AttributionId.parse(attributionId);
36113614
if (!parsedAttributionId) {
36123615
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Unable to parse attributionId");
36133616
}
3614-
return this.billingModes.getBillingMode(user.id, parsedAttributionId.teamId);
3617+
await this.auth.checkPermissionOnOrganization(admin.id, "read_billing", attributionId);
3618+
return this.billingModes.getBillingMode(admin.id, parsedAttributionId.teamId);
36153619
}
36163620

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

@@ -3688,6 +3693,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
36883693
): Promise<void> {
36893694
const user = await this.checkAndBlockUser("adminSaveBlockedEmailDomain");
36903695
await this.guardAdminAccess("adminSaveBlockedEmailDomain", { id: user.id }, Permission.ADMIN_USERS);
3696+
await this.auth.checkPermissionOnInstallation(user.id, "configure");
36913697
await this.emailDomainFilterdb.storeFilterEntry(domainFilterentry);
36923698
}
36933699
}

components/spicedb/codegen/codegen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func GenerateDefinition(schema *compiler.CompiledSchema) string {
127127
128128
import { v1 } from "@authzed/authzed-node";
129129
130-
const InstallationID = "1";
130+
export const InstallationID = "1";
131131
132132
` + resource + `;
133133

components/spicedb/schema/schema.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@ schema: |-
1414
permission read_info = self + organization->member + organization->owner + installation->admin
1515
permission write_info = self
1616
permission delete = self
17+
1718
permission make_admin = installation->admin + organization->installation_admin
1819
20+
// administrate is for changes such as blocking or verifiying, i.e. things that only admins can do on user
21+
permission admin_control = installation->admin + organization->installation_admin
22+
1923
permission read_ssh = self
2024
permission write_ssh = self
2125
@@ -36,6 +40,8 @@ schema: |-
3640
// orgs can only be created by installation-level users
3741
permission create_organization = member + admin
3842
43+
// any global runtime configurations, such as the list of blocked repositories
44+
permission configure = admin
3945
}
4046
4147
definition organization {

0 commit comments

Comments
 (0)