Skip to content

Commit 9b06d2e

Browse files
svenefftingesvenefftinge
authored
[fga] migrated user service methods (#18461)
1 parent 73533e4 commit 9b06d2e

File tree

5 files changed

+216
-49
lines changed

5 files changed

+216
-49
lines changed

‎components/server/src/orgs/usage-service.spec.db.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,12 @@ describe("UsageService", async () => {
8282
authId: "1234",
8383
},
8484
});
85-
await userService.setAdminRole(BUILTIN_INSTLLATION_ADMIN_USER_ID, admin.id, true);
85+
await userService.updateRoleOrPermission(BUILTIN_INSTLLATION_ADMIN_USER_ID, admin.id, [
86+
{
87+
role: "admin",
88+
add: true,
89+
},
90+
]);
8691

8792
us = container.get<UsageService>(UsageService);
8893
await us.getCostCenter(owner.id, org.id);

‎components/server/src/user/user-service.spec.db.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,101 @@ describe("UserService", async () => {
106106
expect(updated.avatarUrl).is.undefined;
107107
expect(updated.additionalData?.disabledClosedTimeout).to.be.true;
108108
});
109+
110+
it("should updateWorkspaceTimeoutSetting", async () => {
111+
await userService.updateWorkspaceTimeoutSetting(user.id, user.id, {
112+
disabledClosedTimeout: true,
113+
});
114+
let updated = await userService.findUserById(user.id, user.id);
115+
expect(updated.additionalData?.disabledClosedTimeout).to.be.true;
116+
117+
await userService.updateWorkspaceTimeoutSetting(user.id, user.id, {
118+
workspaceTimeout: "60m",
119+
});
120+
updated = await userService.findUserById(user.id, user.id);
121+
expect(updated.additionalData?.workspaceTimeout).to.eq("60m");
122+
123+
await expectError(
124+
ErrorCodes.BAD_REQUEST,
125+
userService.updateWorkspaceTimeoutSetting(user.id, user.id, {
126+
workspaceTimeout: "invalid",
127+
}),
128+
);
129+
130+
await expectError(
131+
ErrorCodes.PERMISSION_DENIED,
132+
userService.updateWorkspaceTimeoutSetting(user2.id, user.id, {
133+
workspaceTimeout: "10m",
134+
}),
135+
);
136+
137+
await expectError(
138+
ErrorCodes.NOT_FOUND,
139+
userService.updateWorkspaceTimeoutSetting(nonOrgUser.id, user.id, {
140+
workspaceTimeout: "10m",
141+
}),
142+
);
143+
});
144+
145+
it("should updateRoleOrPermission", async () => {
146+
await expectError(
147+
ErrorCodes.PERMISSION_DENIED,
148+
userService.updateRoleOrPermission(user.id, user.id, [
149+
{
150+
role: "admin",
151+
add: false,
152+
},
153+
]),
154+
);
155+
156+
await userService.updateRoleOrPermission(BUILTIN_INSTLLATION_ADMIN_USER_ID, user.id, [
157+
{
158+
role: "admin",
159+
add: true,
160+
},
161+
]);
162+
163+
let updated = await userService.findUserById(user.id, user.id);
164+
expect(new Set(updated.rolesOrPermissions).has("admin")).to.be.true;
165+
166+
// can remove role themselves now
167+
await userService.updateRoleOrPermission(user.id, user.id, [
168+
{
169+
role: "admin",
170+
add: false,
171+
},
172+
]);
173+
174+
updated = await userService.findUserById(user.id, user.id);
175+
expect(new Set(updated.rolesOrPermissions).has("admin")).to.be.false;
176+
177+
// but not add again
178+
await expectError(
179+
ErrorCodes.PERMISSION_DENIED,
180+
userService.updateRoleOrPermission(user.id, user.id, [
181+
{
182+
role: "admin",
183+
add: true,
184+
},
185+
]),
186+
);
187+
});
188+
189+
it("should listUsers", async () => {
190+
let users = await userService.listUsers(user.id, {});
191+
expect(users.total).to.eq(2);
192+
expect(users.rows.some((u) => u.id === user.id)).to.be.true;
193+
expect(users.rows.some((u) => u.id === user2.id)).to.be.true;
194+
195+
users = await userService.listUsers(BUILTIN_INSTLLATION_ADMIN_USER_ID, {});
196+
expect(users.total).to.eq(4);
197+
expect(users.rows.some((u) => u.id === user.id)).to.be.true;
198+
expect(users.rows.some((u) => u.id === user2.id)).to.be.true;
199+
expect(users.rows.some((u) => u.id === nonOrgUser.id)).to.be.true;
200+
expect(users.rows.some((u) => u.id === BUILTIN_INSTLLATION_ADMIN_USER_ID)).to.be.true;
201+
202+
users = await userService.listUsers(nonOrgUser.id, {});
203+
expect(users.total).to.eq(1);
204+
expect(users.rows.some((u) => u.id === nonOrgUser.id)).to.be.true;
205+
});
109206
});

‎components/server/src/user/user-service.ts‎

Lines changed: 102 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,22 @@ import { inject, injectable } from "inversify";
88
import { Config } from "../config";
99
import { UserDB } from "@gitpod/gitpod-db/lib";
1010
import { Authorizer } from "../authorization/authorizer";
11-
import { AdditionalUserData, Identity, TokenEntry, User } from "@gitpod/gitpod-protocol";
11+
import {
12+
AdditionalUserData,
13+
Identity,
14+
RoleOrPermission,
15+
TokenEntry,
16+
User,
17+
WorkspaceTimeoutDuration,
18+
WorkspaceTimeoutSetting,
19+
} from "@gitpod/gitpod-protocol";
1220
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1321
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1422
import { CreateUserParams } from "./user-authentication";
1523
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
1624
import { TransactionalContext } from "@gitpod/gitpod-db/lib/typeorm/transactional-db-impl";
1725
import { RelationshipUpdater } from "../authorization/relationship-updater";
26+
import { EntitlementService } from "../billing/entitlement-service";
1827

1928
@injectable()
2029
export class UserService {
@@ -24,6 +33,7 @@ export class UserService {
2433
@inject(Authorizer) private readonly authorizer: Authorizer,
2534
@inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
2635
@inject(RelationshipUpdater) private readonly relationshipUpdater: RelationshipUpdater,
36+
@inject(EntitlementService) private readonly entitlementService: EntitlementService,
2737
) {}
2838

2939
public async createUser(
@@ -124,34 +134,104 @@ export class UserService {
124134
return user;
125135
}
126136

127-
async setAdminRole(userId: string, targetUserId: string, admin: boolean): Promise<User> {
128-
await this.authorizer.checkPermissionOnUser(userId, "make_admin", targetUserId);
129-
const target = await this.findUserById(userId, targetUserId);
130-
const rolesAndPermissions = target.rolesOrPermissions || [];
131-
const newRoles = [...rolesAndPermissions.filter((r) => r !== "admin")];
132-
if (admin) {
133-
// add admin role
134-
newRoles.push("admin");
137+
async updateWorkspaceTimeoutSetting(
138+
userId: string,
139+
targetUserId: string,
140+
setting: Partial<WorkspaceTimeoutSetting>,
141+
): Promise<void> {
142+
await this.authorizer.checkPermissionOnUser(userId, "write_info", targetUserId);
143+
144+
if (setting.workspaceTimeout) {
145+
try {
146+
WorkspaceTimeoutDuration.validate(setting.workspaceTimeout);
147+
} catch (err) {
148+
throw new ApplicationError(ErrorCodes.BAD_REQUEST, err.message);
149+
}
135150
}
136151

152+
if (!(await this.entitlementService.maySetTimeout(targetUserId))) {
153+
throw new ApplicationError(
154+
ErrorCodes.PERMISSION_DENIED,
155+
"Configure workspace timeout only available for paid user.",
156+
);
157+
}
158+
159+
const user = await this.findUserById(userId, targetUserId);
160+
AdditionalUserData.set(user, setting);
161+
await this.userDb.updateUserPartial(user);
162+
}
163+
164+
async listUsers(
165+
userId: string,
166+
req: {
167+
//
168+
offset?: number;
169+
limit?: number;
170+
orderBy?: keyof User;
171+
orderDir?: "ASC" | "DESC";
172+
searchTerm?: string;
173+
},
174+
): Promise<{ total: number; rows: User[] }> {
137175
try {
138-
return await this.userDb.transaction(async (userDb) => {
139-
target.rolesOrPermissions = newRoles;
140-
const updatedUser = await userDb.storeUser(target);
141-
if (admin) {
142-
await this.authorizer.addInstallationAdminRole(target.id);
176+
const res = await this.userDb.findAllUsers(
177+
req.offset || 0,
178+
req.limit || 100,
179+
req.orderBy || "creationDate",
180+
req.orderDir || "DESC",
181+
req.searchTerm,
182+
);
183+
const result = { total: res.total, rows: [] as User[] };
184+
for (const user of res.rows) {
185+
if (await this.authorizer.hasPermissionOnUser(userId, "read_info", user.id)) {
186+
result.rows.push(user);
143187
} else {
144-
await this.authorizer.removeInstallationAdminRole(target.id);
188+
result.total--;
145189
}
146-
return updatedUser;
147-
});
148-
} catch (err) {
149-
if (admin) {
150-
await this.authorizer.removeInstallationAdminRole(target.id);
190+
}
191+
return result;
192+
} catch (e) {
193+
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, e.toString());
194+
}
195+
}
196+
197+
async updateRoleOrPermission(
198+
userId: string,
199+
targetUserId: string,
200+
modifications: { role: RoleOrPermission; add?: boolean }[],
201+
): Promise<void> {
202+
await this.authorizer.checkPermissionOnUser(userId, "make_admin", targetUserId);
203+
const target = await this.findUserById(userId, targetUserId);
204+
const rolesOrPermissions = new Set((target.rolesOrPermissions || []) as string[]);
205+
const adminBefore = rolesOrPermissions.has("admin");
206+
modifications.forEach((e) => {
207+
if (e.add) {
208+
rolesOrPermissions.add(e.role as string);
151209
} else {
152-
await this.authorizer.addInstallationAdminRole(target.id);
210+
rolesOrPermissions.delete(e.role as string);
211+
}
212+
});
213+
target.rolesOrPermissions = Array.from(rolesOrPermissions.values()) as RoleOrPermission[];
214+
const adminAfter = new Set(target.rolesOrPermissions).has("admin");
215+
try {
216+
await this.userDb.transaction(async (userDb) => {
217+
await userDb.storeUser(target);
218+
if (adminBefore !== adminAfter) {
219+
if (adminAfter) {
220+
await this.authorizer.addInstallationAdminRole(target.id);
221+
} else {
222+
await this.authorizer.removeInstallationAdminRole(target.id);
223+
}
224+
}
225+
});
226+
} catch (error) {
227+
if (adminBefore !== adminAfter) {
228+
if (adminAfter) {
229+
await this.authorizer.removeInstallationAdminRole(target.id);
230+
} else {
231+
await this.authorizer.addInstallationAdminRole(target.id);
232+
}
153233
}
154-
throw err;
234+
throw error;
155235
}
156236
}
157237
}

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

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ import {
144144
} from "@gitpod/gitpod-protocol/lib/teams-projects-protocol";
145145
import { ClientMetadata, traceClientMetadata } from "../websocket/websocket-connection-manager";
146146
import {
147-
AdditionalUserData,
148147
EmailDomainFilterEntry,
149148
EnvVarWithValue,
150149
LinkedInProfile,
@@ -661,19 +660,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
661660
setting: Partial<WorkspaceTimeoutSetting>,
662661
): Promise<void> {
663662
traceAPIParams(ctx, { setting });
664-
if (setting.workspaceTimeout) {
665-
WorkspaceTimeoutDuration.validate(setting.workspaceTimeout);
666-
}
667-
668663
const user = await this.checkAndBlockUser("updateWorkspaceTimeoutSetting");
669664
await this.guardAccess({ kind: "user", subject: user }, "update");
670665

671-
if (!(await this.entitlementService.maySetTimeout(user.id))) {
672-
throw new Error("configure workspace timeout only available for paid user.");
673-
}
674-
675-
AdditionalUserData.set(user, setting);
676-
await this.userDB.updateUserPartial(user);
666+
await this.userService.updateWorkspaceTimeoutSetting(user.id, user.id, setting);
677667
}
678668

679669
public async sendPhoneNumberVerificationToken(
@@ -3084,20 +3074,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
30843074
async adminGetUsers(ctx: TraceContext, req: AdminGetListRequest<User>): Promise<AdminGetListResult<User>> {
30853075
traceAPIParams(ctx, { req: censor(req, "searchTerm") }); // searchTerm may contain PII
30863076

3087-
await this.guardAdminAccess("adminGetUsers", { req }, Permission.ADMIN_USERS);
3077+
const admin = await this.guardAdminAccess("adminGetUsers", { req }, Permission.ADMIN_USERS);
30883078

3089-
try {
3090-
const res = await this.userDB.findAllUsers(
3091-
req.offset,
3092-
req.limit,
3093-
req.orderBy,
3094-
req.orderDir === "asc" ? "ASC" : "DESC",
3095-
req.searchTerm,
3096-
);
3097-
return res;
3098-
} catch (e) {
3099-
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, e.toString());
3100-
}
3079+
return this.userService.listUsers(admin.id, {
3080+
...req,
3081+
orderDir: req.orderDir === "asc" ? "ASC" : "DESC",
3082+
});
31013083
}
31023084

31033085
async adminGetBlockedRepositories(
@@ -3201,7 +3183,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
32013183
});
32023184
target.rolesOrPermissions = Array.from(rolesOrPermissions.values()) as RoleOrPermission[];
32033185

3204-
return await this.userDB.storeUser(target);
3186+
await this.userService.updateRoleOrPermission(admin.id, target.id, [
3187+
...req.rpp.map((e) => ({ role: e.r, add: e.add })),
3188+
]);
3189+
return this.userService.findUserById(admin.id, req.id);
32053190
}
32063191

32073192
async adminModifyPermanentWorkspaceFeatureFlag(

‎components/spicedb/schema/schema.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ schema: |-
1313
// permissions
1414
permission read_info = self + organization->member + organization->owner + installation->admin
1515
permission write_info = self
16-
permission make_admin = installation->admin
16+
permission make_admin = installation->admin + organization->installation_admin
1717
}
1818
1919
// There's only one global installation

0 commit comments

Comments
 (0)