From 2d948a63200440dd42f89e328826043035e79f3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 29 Sep 2023 16:52:42 +0100 Subject: [PATCH 1/3] feat: Grant platformAdmin user update permissions --- hasura.planx.uk/metadata/tables.yaml | 7 +++++++ hasura.planx.uk/tests/users.test.js | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index 31a2b27ac6..ec848ee84c 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -1148,3 +1148,10 @@ filter: id: _eq: x-hasura-user-id + update_permissions: + - role: platformAdmin + permission: + columns: + - email + filter: {} + check: {} diff --git a/hasura.planx.uk/tests/users.test.js b/hasura.planx.uk/tests/users.test.js index 1b6be0c1d9..91149556e6 100644 --- a/hasura.planx.uk/tests/users.test.js +++ b/hasura.planx.uk/tests/users.test.js @@ -39,13 +39,13 @@ describe("users", () => { expect(i.queries).toContain("users"); }); - test("can creates users", () => { + test("can create and update users", () => { expect(i.mutations).toContain("insert_users"); + expect(i.mutations).toContain("update_users_by_pk"); + expect(i.mutations).toContain("update_users"); }); - test("cannot update or delete users", () => { - expect(i.mutations).not.toContain("update_users_by_pk"); - expect(i.mutations).not.toContain("update_users"); + test("cannot delete users", () => { expect(i.mutations).not.toContain("delete_users"); }); }); From 6575cdb0027a38c747b17fbe315ac66e9187d7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 29 Sep 2023 16:53:47 +0100 Subject: [PATCH 2/3] feat: Delete user endpoint --- api.planx.uk/modules/user/controller.ts | 30 +++++++++++++++++++++++++ api.planx.uk/modules/user/docs.yaml | 19 ++++++++++++++++ api.planx.uk/modules/user/routes.ts | 8 ++++++- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/api.planx.uk/modules/user/controller.ts b/api.planx.uk/modules/user/controller.ts index e7bc07f2a2..9a3355c2d6 100644 --- a/api.planx.uk/modules/user/controller.ts +++ b/api.planx.uk/modules/user/controller.ts @@ -32,3 +32,33 @@ export const createUser: CreateUser = async (req, res, next) => { ); } }; + +export const deleteUserSchema = z.object({ + params: z.object({ + email: z.string().trim().email().toLowerCase(), + }), +}); + +export type DeleteUser = ValidatedRequestHandler< + typeof deleteUserSchema, + UserResponse +>; + +export const deleteUser: DeleteUser = async (req, res, next) => { + try { + const { email } = req.params; + const $client = getClient(); + + const user = await $client.user.getByEmail(email); + if (!user) throw Error(`No user matching email ${email} found`); + + const isSuccessful = await $client.user.delete(user.id); + if (!isSuccessful) throw Error("Request to delete user failed"); + + return res.send({ message: "Successfully deleted user" }); + } catch (error) { + return next( + new ServerError({ message: "Failed to delete user", cause: error }), + ); + } +}; diff --git a/api.planx.uk/modules/user/docs.yaml b/api.planx.uk/modules/user/docs.yaml index d1c77ebcce..398c56e409 100644 --- a/api.planx.uk/modules/user/docs.yaml +++ b/api.planx.uk/modules/user/docs.yaml @@ -41,3 +41,22 @@ paths: $ref: "#/components/responses/SuccessMessage" "500": $ref: "#/components/responses/ErrorMessage" + /user/{email}: + delete: + summary: Delete a user + description: "Requires authentication via a Cloudflare WARP client + \n\n + Please login at [https://api.editor.planx.uk/user](https://api.editor.planx.uk/user)" + tags: ["user"] + parameters: + - in: path + name: email + type: string + format: email + example: bilbo@bagend.sh + description: Email address of the user to be deleted + responses: + "200": + $ref: "#/components/responses/SuccessMessage" + "500": + $ref: "#/components/responses/ErrorMessage" diff --git a/api.planx.uk/modules/user/routes.ts b/api.planx.uk/modules/user/routes.ts index c569a70308..68991ea58e 100644 --- a/api.planx.uk/modules/user/routes.ts +++ b/api.planx.uk/modules/user/routes.ts @@ -1,11 +1,17 @@ import { Router } from "express"; import { usePlatformAdminAuth } from "../auth/middleware"; import { validate } from "../../shared/middleware/validate"; -import { createUserSchema, createUser } from "./controller"; +import { + createUserSchema, + createUser, + deleteUserSchema, + deleteUser, +} from "./controller"; const router = Router(); router.use(usePlatformAdminAuth); router.put("/", validate(createUserSchema), createUser); +router.delete("/:email", validate(deleteUserSchema), deleteUser); export default router; From c6a7a4e34aa19b024955fb8ccf4183f37065d7d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 29 Sep 2023 17:09:54 +0100 Subject: [PATCH 3/3] test: Add API tests for new endpoint --- api.planx.uk/modules/user/index.test.ts | 67 +++++++++++++++++++++++-- api.planx.uk/package.json | 2 +- api.planx.uk/pnpm-lock.yaml | 15 +++--- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/api.planx.uk/modules/user/index.test.ts b/api.planx.uk/modules/user/index.test.ts index 4290fb65ec..5ee527b459 100644 --- a/api.planx.uk/modules/user/index.test.ts +++ b/api.planx.uk/modules/user/index.test.ts @@ -2,20 +2,24 @@ import supertest from "supertest"; import app from "../../server"; import { authHeader } from "../../tests/mockJWT"; -const mockCreateUser = jest.fn(); - const mockUser = { firstName: "Bilbo", lastName: "Baggins", - email: "bilbo@bagend.sh", + email: "bilbo.baggins@example.com", isPlatformAdmin: false, }; +const mockCreateUser = jest.fn(); +const mockDeleteUser = jest.fn(); +const mockGetByEmail = jest.fn().mockResolvedValue(mockUser); + jest.mock("@opensystemslab/planx-core", () => { return { CoreDomainClient: jest.fn().mockImplementation(() => ({ user: { create: () => mockCreateUser(), + delete: () => mockDeleteUser(), + getByEmail: () => mockGetByEmail(), }, })), }; @@ -66,3 +70,60 @@ describe("Create user endpoint", () => { }); }); }); + +describe("Delete user endpoint", () => { + it("requires authentication", async () => { + await supertest(app).delete("/user/bilbo.baggins@example.com").expect(401); + }); + + it("requires the 'platformAdmin' role", async () => { + await supertest(app) + .delete("/user/bilbo.baggins@example.com") + .set(authHeader({ role: "teamEditor" })) + .expect(403); + }); + + it("handles an invalid email", async () => { + mockGetByEmail.mockResolvedValueOnce(null); + + await supertest(app) + .delete("/user/bilbo.baggins@example.com") + .set(auth) + .expect(500) + .then((res) => { + expect(mockGetByEmail).toHaveBeenCalled(); + expect(res.body).toHaveProperty("error"); + expect(res.body.error).toMatch(/Failed to delete user/); + }); + }); + + it("handles a failure to delete the user", async () => { + mockDeleteUser.mockResolvedValueOnce(false); + + await supertest(app) + .delete("/user/bilbo.baggins@example.com") + .set(auth) + .expect(500) + .then((res) => { + expect(mockGetByEmail).toHaveBeenCalled(); + expect(mockDeleteUser).toHaveBeenCalled(); + expect(res.body).toHaveProperty("error"); + expect(res.body.error).toMatch(/Failed to delete user/); + }); + }); + + it("can successfully delete a user", async () => { + mockDeleteUser.mockResolvedValue(true); + + await supertest(app) + .delete("/user/bilbo.baggins@example.com") + .set(auth) + .expect(200) + .then((res) => { + expect(mockGetByEmail).toHaveBeenCalled(); + expect(mockDeleteUser).toHaveBeenCalled(); + expect(res.body).toHaveProperty("message"); + expect(res.body.message).toMatch(/Successfully deleted user/); + }); + }); +}); diff --git a/api.planx.uk/package.json b/api.planx.uk/package.json index f1824aeee8..ab32a5b60a 100644 --- a/api.planx.uk/package.json +++ b/api.planx.uk/package.json @@ -4,7 +4,7 @@ "private": true, "dependencies": { "@airbrake/node": "^2.1.8", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#44420b9", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#3d395fa", "@types/isomorphic-fetch": "^0.0.36", "adm-zip": "^0.5.10", "aws-sdk": "^2.1467.0", diff --git a/api.planx.uk/pnpm-lock.yaml b/api.planx.uk/pnpm-lock.yaml index 0999a3f57e..4f5b4079d4 100644 --- a/api.planx.uk/pnpm-lock.yaml +++ b/api.planx.uk/pnpm-lock.yaml @@ -9,8 +9,8 @@ dependencies: specifier: ^2.1.8 version: 2.1.8 '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#44420b9 - version: github.com/theopensystemslab/planx-core/44420b9 + specifier: git+https://github.com/theopensystemslab/planx-core#3d395fa + version: github.com/theopensystemslab/planx-core/3d395fa '@types/isomorphic-fetch': specifier: ^0.0.36 version: 0.0.36 @@ -1970,7 +1970,7 @@ packages: resolution: {integrity: sha512-ZUxbzKl0IfJILTS6t7ip5fQQM/J3TJYubDm3nMbgubNNYS62eXeUpoLUC8/7fJNiFYHTrGPQn7hspDUzIHX3UA==} dependencies: '@types/minimatch': 5.1.2 - '@types/node': 16.18.38 + '@types/node': 18.18.1 dev: false /@types/graceful-fs@4.1.6: @@ -2057,7 +2057,6 @@ packages: /@types/node@18.18.1: resolution: {integrity: sha512-3G42sxmm0fF2+Vtb9TJQpnjmP+uKlWvFa8KoEGquh4gqRmoUG/N0ufuhikw6HEsdG2G2oIKhog1GCTfz9v5NdQ==} - dev: true /@types/node@20.4.1: resolution: {integrity: sha512-JIzsAvJeA/5iY6Y/OxZbv1lUcc8dNSE77lb2gnBH+/PJ3lFR1Ccvgwl5JWnHAkNHcRsT0TbpVOsiMKZ1F/yyJg==} @@ -5416,7 +5415,7 @@ packages: resolution: {integrity: sha512-S5wqyz0DXnNJPd/xfIzZ5Xnp1HrJWBczg8mMfMpN78OJ5eDxXyf+Ygld9wX1DnUWbIbhM1YDY95NjR4CBXkb2g==} engines: {node: '>= 10.14.2'} dependencies: - '@types/node': 16.18.38 + '@types/node': 18.18.1 graceful-fs: 4.2.11 dev: true @@ -5515,7 +5514,7 @@ packages: resolution: {integrity: sha512-KWYVV1c4i+jbMpaBC+U++4Va0cp8OisU185o73T1vo99hqi7w8tSJfUXYswwqqrjzwxa6KpRK54WhPvwf5w6PQ==} engines: {node: '>= 10.13.0'} dependencies: - '@types/node': 16.18.38 + '@types/node': 18.18.1 merge-stream: 2.0.0 supports-color: 7.2.0 dev: true @@ -8116,8 +8115,8 @@ packages: resolution: {integrity: sha512-wvWkphh5WQsJbVk1tbx1l1Ly4yg+XecD+Mq280uBGt9wa5BKSWf4Mhp6GmrkPixhMxmabYY7RbzlwVP32pbGCg==} dev: false - github.com/theopensystemslab/planx-core/44420b9: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/44420b9} + github.com/theopensystemslab/planx-core/3d395fa: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/3d395fa} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true