Skip to content

Commit 246d8ed

Browse files
authored
[fga] check some admin functions (#18562)
1 parent e234839 commit 246d8ed

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.all(
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
@@ -2646,7 +2646,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
26462646
): Promise<AdminGetListResult<BlockedRepository>> {
26472647
traceAPIParams(ctx, { req: censor(req, "searchTerm") }); // searchTerm may contain PII
26482648

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

26512652
try {
26522653
const res = await this.blockedRepostoryDB.findAllBlockedRepositories(
@@ -2669,15 +2670,21 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
26692670
): Promise<BlockedRepository> {
26702671
traceAPIParams(ctx, { urlRegexp, blockUser });
26712672

2672-
await this.guardAdminAccess("adminCreateBlockedRepository", { urlRegexp, blockUser }, Permission.ADMIN_USERS);
2673+
const admin = await this.guardAdminAccess(
2674+
"adminCreateBlockedRepository",
2675+
{ urlRegexp, blockUser },
2676+
Permission.ADMIN_USERS,
2677+
);
2678+
await this.auth.checkPermissionOnInstallation(admin.id, "configure");
26732679

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

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

2680-
await this.guardAdminAccess("adminDeleteBlockedRepository", { id }, Permission.ADMIN_USERS);
2686+
const admin = await this.guardAdminAccess("adminDeleteBlockedRepository", { id }, Permission.ADMIN_USERS);
2687+
await this.auth.checkPermissionOnInstallation(admin.id, "configure");
26812688

26822689
await this.blockedRepostoryDB.deleteBlockedRepository(id);
26832690
}
@@ -2719,6 +2726,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
27192726

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

27242732
this.verificationService.markVerified(user);
@@ -2759,6 +2767,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
27592767
{ req },
27602768
Permission.ADMIN_USERS,
27612769
);
2770+
await this.auth.checkPermissionOnUser(admin.id, "admin_control", req.id);
2771+
27622772
const target = await this.userService.findUserById(admin.id, req.id);
27632773

27642774
const featureSettings: UserFeatureSettings = target.featureFlags || {};
@@ -2890,15 +2900,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
28902900
}
28912901

28922902
async adminGetTeams(ctx: TraceContext, req: AdminGetListRequest<Team>): Promise<AdminGetListResult<Team>> {
2893-
await this.guardAdminAccess("adminGetTeams", { req }, Permission.ADMIN_WORKSPACES);
2894-
2895-
return await this.teamDB.findTeams(
2896-
req.offset,
2897-
req.limit,
2898-
req.orderBy,
2899-
req.orderDir === "asc" ? "ASC" : "DESC",
2900-
req.searchTerm as string,
2901-
);
2903+
const admin = await this.guardAdminAccess("adminGetTeams", { req }, Permission.ADMIN_WORKSPACES);
2904+
return this.organizationService.listOrganizations(admin.id, req);
29022905
}
29032906

29042907
async adminGetTeamById(ctx: TraceContext, id: string): Promise<Team | undefined> {
@@ -3623,16 +3626,17 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
36233626
async adminGetBillingMode(ctx: TraceContextWithSpan, attributionId: string): Promise<BillingMode> {
36243627
traceAPIParams(ctx, { attributionId });
36253628

3626-
const user = await this.checkAndBlockUser("adminGetBillingMode");
3627-
if (!this.authorizationService.hasPermission(user, Permission.ADMIN_USERS)) {
3629+
const admin = await this.checkAndBlockUser("adminGetBillingMode");
3630+
if (!this.authorizationService.hasPermission(admin, Permission.ADMIN_USERS)) {
36283631
throw new ApplicationError(ErrorCodes.PERMISSION_DENIED, "not allowed");
36293632
}
36303633

36313634
const parsedAttributionId = AttributionId.parse(attributionId);
36323635
if (!parsedAttributionId) {
36333636
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Unable to parse attributionId");
36343637
}
3635-
return this.billingModes.getBillingMode(user.id, parsedAttributionId.teamId);
3638+
await this.auth.checkPermissionOnOrganization(admin.id, "read_billing", attributionId);
3639+
return this.billingModes.getBillingMode(admin.id, parsedAttributionId.teamId);
36363640
}
36373641

36383642
async adminGetCostCenter(ctx: TraceContext, attributionId: string): Promise<CostCenterJSON | undefined> {
@@ -3700,6 +3704,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
37003704
async adminGetBlockedEmailDomains(ctx: TraceContextWithSpan): Promise<EmailDomainFilterEntry[]> {
37013705
const user = await this.checkAndBlockUser("adminGetBlockedEmailDomains");
37023706
await this.guardAdminAccess("adminGetBlockedEmailDomains", { id: user.id }, Permission.ADMIN_USERS);
3707+
await this.auth.checkPermissionOnInstallation(user.id, "configure");
37033708
return await this.emailDomainFilterdb.getFilterEntries();
37043709
}
37053710

@@ -3709,6 +3714,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
37093714
): Promise<void> {
37103715
const user = await this.checkAndBlockUser("adminSaveBlockedEmailDomain");
37113716
await this.guardAdminAccess("adminSaveBlockedEmailDomain", { id: user.id }, Permission.ADMIN_USERS);
3717+
await this.auth.checkPermissionOnInstallation(user.id, "configure");
37123718
await this.emailDomainFilterdb.storeFilterEntry(domainFilterentry);
37133719
}
37143720
}

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)