Skip to content

Commit b767d6f

Browse files
geroplakosyakov
authored andcommitted
[server] WorkspaceService.startWorkspace
1 parent 4fdc6c7 commit b767d6f

File tree

9 files changed

+249
-197
lines changed

9 files changed

+249
-197
lines changed

components/server/src/api/user.spec.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { suite, test } from "@testdeck/mocha";
88
import { APIUserService } from "./user";
99
import { Container } from "inversify";
1010
import { testContainer } from "@gitpod/gitpod-db/lib";
11-
import { WorkspaceStarter } from "../workspace/workspace-starter";
1211
import { UserAuthentication } from "../user/user-authentication";
1312
import { BlockUserRequest, BlockUserResponse } from "@gitpod/public-api/lib/gitpod/experimental/v1/user_pb";
1413
import { User } from "@gitpod/gitpod-protocol";
@@ -18,24 +17,26 @@ import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
1817
import { v4 as uuidv4 } from "uuid";
1918
import { ConnectError, Code } from "@bufbuild/connect";
2019
import * as chai from "chai";
20+
import { WorkspaceService } from "../workspace/workspace-service";
2121

2222
const expect = chai.expect;
2323

2424
@suite()
2525
export class APIUserServiceSpec {
2626
private container: Container;
27-
private workspaceStarterMock: WorkspaceStarter = {
27+
private workspaceStarterMock: WorkspaceService = {
2828
stopRunningWorkspacesForUser: async (
2929
ctx: TraceContext,
30-
userID: string,
30+
userId: string,
31+
userIdToStop: string,
3132
reason: string,
3233
policy?: StopWorkspacePolicy,
3334
): Promise<Workspace[]> => {
3435
return [];
3536
},
36-
} as WorkspaceStarter;
37+
} as WorkspaceService;
3738
private userServiceMock: UserAuthentication = {
38-
blockUser: async (targetUserId: string, block: boolean): Promise<User> => {
39+
blockUser: async (userId: string, targetUserId: string, block: boolean): Promise<User> => {
3940
return {
4041
id: targetUserId,
4142
} as User;
@@ -45,7 +46,7 @@ export class APIUserServiceSpec {
4546
async before() {
4647
this.container = testContainer.createChild();
4748

48-
this.container.bind(WorkspaceStarter).toConstantValue(this.workspaceStarterMock);
49+
this.container.bind(WorkspaceService).toConstantValue(this.workspaceStarterMock);
4950
this.container.bind(UserAuthentication).toConstantValue(this.userServiceMock);
5051
this.container.bind(APIUserService).toSelf().inSingletonScope();
5152
}

components/server/src/api/user.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,16 @@ import {
2323
GetGitTokenResponse,
2424
BlockUserResponse,
2525
} from "@gitpod/public-api/lib/gitpod/experimental/v1/user_pb";
26-
import { WorkspaceStarter } from "../workspace/workspace-starter";
2726
import { UserAuthentication } from "../user/user-authentication";
27+
import { WorkspaceService } from "../workspace/workspace-service";
28+
import { SYSTEM_USER } from "../authorization/authorizer";
2829
import { validate } from "uuid";
29-
import { StopWorkspacePolicy } from "@gitpod/ws-manager/lib";
3030
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
31+
import { StopWorkspacePolicy } from "@gitpod/ws-manager/lib";
3132

3233
@injectable()
3334
export class APIUserService implements ServiceImpl<typeof UserServiceInterface> {
34-
@inject(WorkspaceStarter) protected readonly workspaceStarter: WorkspaceStarter;
35+
@inject(WorkspaceService) protected readonly workspaceService: WorkspaceService;
3536
@inject(UserAuthentication) protected readonly userService: UserAuthentication;
3637

3738
public async getAuthenticatedUser(req: GetAuthenticatedUserRequest): Promise<GetAuthenticatedUserResponse> {
@@ -73,14 +74,17 @@ export class APIUserService implements ServiceImpl<typeof UserServiceInterface>
7374

7475
// TODO: Once connect-node supports middlewares, lift the tracing into the middleware.
7576
const trace = {};
76-
await this.userService.blockUser(userId, true);
77+
// TODO for now we use SYSTEM_USER, since it is only called by internal componenets like usage
78+
// and not exposed publically, but there should be better way to get an authenticated user
79+
await this.userService.blockUser(SYSTEM_USER, userId, true);
7780
log.info(`Blocked user ${userId}.`, {
7881
userId,
7982
reason,
8083
});
8184

82-
const stoppedWorkspaces = await this.workspaceStarter.stopRunningWorkspacesForUser(
85+
const stoppedWorkspaces = await this.workspaceService.stopRunningWorkspacesForUser(
8386
trace,
87+
SYSTEM_USER,
8488
userId,
8589
reason,
8690
StopWorkspacePolicy.IMMEDIATELY,

components/server/src/authorization/authorizer.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export class Authorizer {
5959
permission: OrganizationPermission,
6060
orgId: string,
6161
): Promise<boolean> {
62-
if (userId === "SYSTEM_USER") {
62+
if (userId === SYSTEM_USER) {
6363
return true;
6464
}
6565

@@ -89,7 +89,7 @@ export class Authorizer {
8989
}
9090

9191
async hasPermissionOnProject(userId: string, permission: ProjectPermission, projectId: string): Promise<boolean> {
92-
if (userId === "SYSTEM_USER") {
92+
if (userId === SYSTEM_USER) {
9393
return true;
9494
}
9595

@@ -119,7 +119,7 @@ export class Authorizer {
119119
}
120120

121121
async hasPermissionOnUser(userId: string, permission: UserPermission, resourceUserId: string): Promise<boolean> {
122-
if (userId === "SYSTEM_USER") {
122+
if (userId === SYSTEM_USER) {
123123
return true;
124124
}
125125

@@ -152,7 +152,7 @@ export class Authorizer {
152152
permission: WorkspacePermission,
153153
workspaceId: string,
154154
): Promise<boolean> {
155-
if (userId === "SYSTEM_USER") {
155+
if (userId === SYSTEM_USER) {
156156
return true;
157157
}
158158

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ export class UserAuthentication {
3939
@inject(HostContextProvider) private readonly hostContextProvider: HostContextProvider,
4040
) {}
4141

42-
async blockUser(targetUserId: string, block: boolean): Promise<User> {
43-
const target = await this.userService.findUserById(targetUserId, targetUserId);
42+
async blockUser(userId: string, targetUserId: string, block: boolean): Promise<User> {
43+
const target = await this.userService.findUserById(userId, targetUserId);
4444
if (!target) {
4545
throw new ApplicationError(ErrorCodes.NOT_FOUND, "not found");
4646
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ import { ClientMetadata } from "../websocket/websocket-connection-manager";
2727
import * as fs from "fs/promises";
2828
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
2929
import { GitpodServerImpl } from "../workspace/gitpod-server-impl";
30-
import { WorkspaceStarter } from "../workspace/workspace-starter";
3130
import { StopWorkspacePolicy } from "@gitpod/ws-manager/lib";
3231
import { UserService } from "./user-service";
32+
import { WorkspaceService } from "../workspace/workspace-service";
3333

3434
export const ServerFactory = Symbol("ServerFactory");
3535
export type ServerFactory = () => GitpodServerImpl;
@@ -47,7 +47,7 @@ export class UserController {
4747
@inject(SessionHandler) protected readonly sessionHandler: SessionHandler;
4848
@inject(OneTimeSecretServer) protected readonly otsServer: OneTimeSecretServer;
4949
@inject(OneTimeSecretDB) protected readonly otsDb: OneTimeSecretDB;
50-
@inject(WorkspaceStarter) protected readonly workspaceStarter: WorkspaceStarter;
50+
@inject(WorkspaceService) protected readonly workspaceService: WorkspaceService;
5151
@inject(ServerFactory) private readonly serverFactory: ServerFactory;
5252

5353
get apiRouter(): express.Router {
@@ -241,8 +241,8 @@ export class UserController {
241241
// stop all running workspaces
242242
const user = req.user as User;
243243
if (user) {
244-
this.workspaceStarter
245-
.stopRunningWorkspacesForUser({}, user.id, "logout", StopWorkspacePolicy.NORMALLY)
244+
this.workspaceService
245+
.stopRunningWorkspacesForUser({}, user.id, user.id, "logout", StopWorkspacePolicy.NORMALLY)
246246
.catch((error) =>
247247
log.error(logContext, "cannot stop workspaces on logout", { error, ...logPayload }),
248248
);

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1212
import { StopWorkspacePolicy } from "@gitpod/ws-manager/lib";
1313
import { AuthProviderService } from "../auth/auth-provider-service";
1414
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
15-
import { WorkspaceStarter } from "../workspace/workspace-starter";
15+
import { WorkspaceService } from "../workspace/workspace-service";
1616

1717
@injectable()
1818
export class UserDeletionService {
@@ -22,7 +22,7 @@ export class UserDeletionService {
2222
@inject(TeamDB) private readonly teamDb: TeamDB,
2323
@inject(ProjectDB) private readonly projectDb: ProjectDB,
2424
@inject(StorageClient) private readonly storageClient: StorageClient,
25-
@inject(WorkspaceStarter) private readonly workspaceStarter: WorkspaceStarter,
25+
@inject(WorkspaceService) private readonly workspaceService: WorkspaceService,
2626
@inject(AuthProviderService) private readonly authProviderService: AuthProviderService,
2727
@inject(IAnalyticsWriter) private readonly analytics: IAnalyticsWriter,
2828
) {}
@@ -33,19 +33,20 @@ export class UserDeletionService {
3333
* To guarantee that, but also maintain traceability
3434
* we anonymize data that might contain user related/relatable data and keep the entities itself (incl. ids).
3535
*/
36-
async deleteUser(id: string): Promise<void> {
37-
const user = await this.db.findUserById(id);
36+
async deleteUser(userId: string, targetUserId: string): Promise<void> {
37+
const user = await this.db.findUserById(targetUserId);
3838
if (!user) {
39-
throw new Error(`No user with id ${id} found!`);
39+
throw new Error(`No user with id ${targetUserId} found!`);
4040
}
4141

4242
if (user.markedDeleted === true) {
43-
log.debug({ userId: id }, "Is deleted but markDeleted already set. Continuing.");
43+
log.debug({ userId: targetUserId }, "Is deleted but markDeleted already set. Continuing.");
4444
}
4545

4646
// Stop all workspaces
47-
await this.workspaceStarter.stopRunningWorkspacesForUser(
47+
await this.workspaceService.stopRunningWorkspacesForUser(
4848
{},
49+
userId,
4950
user.id,
5051
"user deleted",
5152
StopWorkspacePolicy.IMMEDIATELY,
@@ -57,7 +58,7 @@ export class UserDeletionService {
5758
try {
5859
await this.authProviderService.deleteAuthProvider(provider);
5960
} catch (error) {
60-
log.error({ userId: id }, "Failed to delete user's auth provider.", error);
61+
log.error({ userId: targetUserId }, "Failed to delete user's auth provider.", error);
6162
}
6263
}
6364

@@ -73,13 +74,13 @@ export class UserDeletionService {
7374

7475
await Promise.all([
7576
// Workspace
76-
this.anonymizeAllWorkspaces(id),
77+
this.anonymizeAllWorkspaces(targetUserId),
7778
// Bucket
78-
this.deleteUserBucket(id),
79+
this.deleteUserBucket(targetUserId),
7980
// Teams owned only by this user
80-
this.deleteSoleOwnedTeams(id),
81+
this.deleteSoleOwnedTeams(targetUserId),
8182
// Team memberships
82-
this.deleteTeamMemberships(id),
83+
this.deleteTeamMemberships(targetUserId),
8384
]);
8485

8586
// Track the deletion Event for Analytics Purposes

0 commit comments

Comments
 (0)