Add organization_id check to

ListWorkspacesRequest_Scope in WorkspaceServiceAPI
gitpod-io · Nov 8, 2023 · 8934773 · 8934773
1 parent 80f6d03
commit 8934773
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 105 deletions.
diff --git a/components/dashboard/src/service/json-rpc-workspace-client.ts b/components/dashboard/src/service/json-rpc-workspace-client.ts
@@ -11,6 +11,7 @@ import {
     GetWorkspaceRequest,
     GetWorkspaceResponse,
     ListWorkspacesRequest,
+    ListWorkspacesRequest_Scope,
     ListWorkspacesResponse,
 } from "@gitpod/public-api/lib/gitpod/v1/workspace_pb";
 import { converter } from "./public-api";
@@ -33,6 +34,13 @@ export class JsonRpcWorkspaceClient implements PromiseClient<typeof WorkspaceSer
         request: PartialMessage<ListWorkspacesRequest>,
         _options?: CallOptions,
     ): Promise<ListWorkspacesResponse> {
+        request.scope = request.scope ?? ListWorkspacesRequest_Scope.MY_WORKSPACES_IN_ORGANIZATION;
+        if (
+            request.scope === ListWorkspacesRequest_Scope.MY_WORKSPACES_IN_ORGANIZATION ||
+            request.scope === ListWorkspacesRequest_Scope.ALL_WORKSPACES_IN_ORGANIZATION
+        ) {
+            throw new ConnectError("organization_id is required", Code.InvalidArgument);
+        }
         const server = getGitpodService().server;
         const pageSize = request.pagination?.pageSize || 100;
         const workspaces = await server.getWorkspaces({

diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts
@@ -263,75 +263,6 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> imp
         return { total, rows };
     }
 
-    public async findWorkspaces(
-        userId: string,
-        organizationId: string,
-        options: {
-            offset?: number | undefined;
-            limit?: number | undefined;
-            pinned?: boolean | undefined;
-        },
-    ): Promise<{ total: number; rows: WorkspaceInfo[] }> {
-        /**
-         * With this query we want to list all user workspaces by lastActivity and include the latestWorkspaceInstance (if present).
-         * Implementation notes:
-         *  - Explanation for ORDER BY wsiRunning DESC:
-         *    - we want running workspaces to always be on the top. wsiRunning is non-NULL if a workspace is running,
-         *      so sorting will bump it to the top
-         *  - Explanation for ORDER BY GREATEST(...):
-         *    - we want to sort workspaces by last activity
-         *    - all fields are string fields, defaulting to empty string (not NULL!), containing ISO date strings (which are sortable by date)
-         *      thus GREATEST gives us the highest (newest) timestamp on the running instance which correlates to the last activity on that workspace
-         */
-        const repo = await this.getWorkspaceRepo();
-        const qb = repo
-            .createQueryBuilder("ws")
-            // We need to put the subquery into the join condition (ON) here to be able to reference `ws.id` which is
-            // not possible in a subquery on JOIN (e.g. 'LEFT JOIN (SELECT ... WHERE i.workspaceId = ws.id)')
-            .leftJoinAndMapOne(
-                "ws.latestInstance",
-                DBWorkspaceInstance,
-                "wsi",
-                `wsi.id = (SELECT i.id FROM d_b_workspace_instance AS i WHERE i.workspaceId = ws.id ORDER BY i.creationTime DESC LIMIT 1)`,
-            )
-            .leftJoin(
-                (qb) => {
-                    return qb
-                        .select("workspaceId")
-                        .from(DBWorkspaceInstance, "i2")
-                        .where('i2.phasePersisted = "running"');
-                },
-                "wsiRunning",
-                "ws.id = wsiRunning.workspaceId",
-            )
-            .where("ws.ownerId = :userId", { userId: userId })
-            .andWhere("ws.softDeletedTime = ''") // enables usage of: ind_softDeletion
-            .andWhere("ws.softDeleted IS NULL")
-            .andWhere("ws.deleted != TRUE")
-            .orderBy("wsiRunning.workspaceId", "DESC")
-            .addOrderBy("ws.pinned", "DESC")
-            .addOrderBy("GREATEST(ws.creationTime, wsi.creationTime, wsi.startedTime, wsi.stoppedTime)", "DESC")
-            .skip(options.offset || 0)
-            .limit(options.limit || 50);
-        if (options.pinned !== undefined) {
-            qb.andWhere("ws.pinned = :pinned", { pinned: options.pinned });
-        }
-        if (organizationId) {
-            qb.andWhere("ws.organizationId = :organizationId", { organizationId });
-        }
-        const total = await qb.getCount();
-        const rawResults = (await qb.getMany()) as any as (Workspace & { latestInstance?: WorkspaceInstance })[]; // see leftJoinAndMapOne above
-        const rows = rawResults.map((r) => {
-            const workspace = { ...r };
-            delete workspace.latestInstance;
-            return {
-                workspace,
-                latestInstance: r.latestInstance,
-            };
-        });
-        return { total, rows };
-    }
-
     public async updateLastHeartbeat(
         instanceId: string,
         userId: string,

diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts
@@ -815,35 +815,6 @@ class WorkspaceDBSpec {
         expect(result.total).to.eq(0);
     }
 
-    @test(timeout(10000))
-    public async newFindWorkspacesByOrganizationId() {
-        await this.db.store(this.ws);
-        await this.db.store(this.ws2);
-        await this.db.store(this.ws3);
-
-        let result = await this.db.findWorkspaces(this.userId, this.orgidA, {});
-        expect(result.total).to.eq(2);
-        for (const ws of result.rows) {
-            expect(ws.workspace.organizationId).to.equal(this.orgidA);
-        }
-
-        result = await this.db.findWorkspaces(this.userId, this.orgidA, { limit: 1 });
-        expect(result.total).to.eq(2);
-        expect(result.rows.length).to.eq(1);
-        for (const ws of result.rows) {
-            expect(ws.workspace.organizationId).to.equal(this.orgidA);
-        }
-
-        result = await this.db.findWorkspaces(this.userId, this.orgidB, {});
-        expect(result.total).to.eq(1);
-        for (const ws of result.rows) {
-            expect(ws.workspace.organizationId).to.equal(this.orgidB);
-        }
-
-        result = await this.db.findWorkspaces(this.userId, "no-org", {});
-        expect(result.total).to.eq(0);
-    }
-
     @test(timeout(10000))
     public async hardDeleteWorkspace() {
         await this.db.store(this.ws);

diff --git a/components/gitpod-db/src/workspace-db.ts b/components/gitpod-db/src/workspace-db.ts
@@ -82,11 +82,6 @@ export interface WorkspaceDB {
     findById(id: string): Promise<MaybeWorkspace>;
     findByInstanceId(id: string): Promise<MaybeWorkspace>;
     find(options: FindWorkspacesOptions): Promise<{ total: number; rows: WorkspaceInfo[] }>;
-    findWorkspaces(
-        userId: string,
-        organizationId: string,
-        options: { offset?: number; limit?: number; pinned?: boolean },
-    ): Promise<{ total: number; rows: WorkspaceInfo[] }>;
     findWorkspacePortsAuthDataById(workspaceId: string): Promise<WorkspacePortsAuthData | undefined>;
 
     storeInstance(instance: WorkspaceInstance): Promise<WorkspaceInstance>;

diff --git a/components/server/src/api/workspace-service-api.ts b/components/server/src/api/workspace-service-api.ts
@@ -4,12 +4,13 @@
  * See License.AGPL.txt in the project root for license information.
  */
 
-import { HandlerContext, ServiceImpl } from "@connectrpc/connect";
+import { Code, ConnectError, HandlerContext, ServiceImpl } from "@connectrpc/connect";
 import { WorkspaceService as WorkspaceServiceInterface } from "@gitpod/public-api/lib/gitpod/v1/workspace_connect";
 import {
     GetWorkspaceRequest,
     GetWorkspaceResponse,
     ListWorkspacesRequest,
+    ListWorkspacesRequest_Scope,
     ListWorkspacesResponse,
 } from "@gitpod/public-api/lib/gitpod/v1/workspace_pb";
 import { inject, injectable } from "inversify";
@@ -36,7 +37,13 @@ export class WorkspaceServiceAPI implements ServiceImpl<typeof WorkspaceServiceI
         // TODO: pagination check - max min pageSize ...
         // TODO: implement req.scope req.sorts
         // TODO: if scope is `SCOPE_ALL_WORKSPACES_IN_INSTALLATION` check admin permission
-
+        req.scope = req.scope ?? ListWorkspacesRequest_Scope.MY_WORKSPACES_IN_ORGANIZATION;
+        if (
+            req.scope === ListWorkspacesRequest_Scope.MY_WORKSPACES_IN_ORGANIZATION ||
+            req.scope === ListWorkspacesRequest_Scope.ALL_WORKSPACES_IN_ORGANIZATION
+        ) {
+            throw new ConnectError("organization_id is required", Code.InvalidArgument);
+        }
         const pageSize = req.pagination?.pageSize ?? 100;
         const workspaces = await this.workspaceService.getWorkspaces(context.user.id, {
             limit: pageSize,