From 57a3ff96ea7e231357368360dd1b985d39976821 Mon Sep 17 00:00:00 2001 From: adrinr Date: Wed, 5 Apr 2023 10:56:55 +0100 Subject: [PATCH 1/5] Add new test for conflicting user email --- packages/worker/src/api/routes/global/tests/scim.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/worker/src/api/routes/global/tests/scim.spec.ts b/packages/worker/src/api/routes/global/tests/scim.spec.ts index 6c411a640d..e76e6b6cda 100644 --- a/packages/worker/src/api/routes/global/tests/scim.spec.ts +++ b/packages/worker/src/api/routes/global/tests/scim.spec.ts @@ -318,6 +318,15 @@ describe("scim", () => { await postScimUser({ body }, { expect: 500 }) }) }) + + it("creating an existing user name returns a conflict", async () => { + const body = structures.scim.createUserRequest() + + await postScimUser({ body }) + + const res = await postScimUser({ body }, { expect: 409 }) + expect((res as any).message).toBe("Email already in use") + }) }) describe("GET /api/global/scim/v2/users/:id", () => { From e1279ffecdb93fbfe9fe47923b652b1ca1a2b998 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 10 Apr 2023 20:06:47 +0100 Subject: [PATCH 2/5] Use proper errors instead of string throwing --- packages/backend-core/src/errors/errors.ts | 8 ++++++++ packages/worker/src/sdk/users/users.ts | 7 ++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/errors/errors.ts b/packages/backend-core/src/errors/errors.ts index 54ca8456ab..0a632d7551 100644 --- a/packages/backend-core/src/errors/errors.ts +++ b/packages/backend-core/src/errors/errors.ts @@ -97,3 +97,11 @@ export class InvalidAPIKeyError extends BudibaseError { ) } } + +// USERS + +export class EmailUnavailableError extends Error { + constructor(email: string) { + super(`Email already taken: '${email}'`) + } +} diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index f05c6b98d2..2150654ae9 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -16,6 +16,7 @@ import { ViewName, env as coreEnv, context, + EmailUnavailableError, } from "@budibase/backend-core" import { AccountMetadata, @@ -158,7 +159,7 @@ const validateUniqueUser = async (email: string, tenantId: string) => { if (env.MULTI_TENANCY) { const tenantUser = await getPlatformUser(email) if (tenantUser != null && tenantUser.tenantId !== tenantId) { - throw `Unavailable` + throw new EmailUnavailableError(email) } } @@ -166,7 +167,7 @@ const validateUniqueUser = async (email: string, tenantId: string) => { if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { const account = await accounts.getAccount(email) if (account && account.verified && account.tenantId !== tenantId) { - throw `Unavailable` + throw new EmailUnavailableError(email) } } } @@ -235,7 +236,7 @@ export const save = async ( // no id was specified - load from email instead dbUser = await usersCore.getGlobalUserByEmail(email) if (dbUser && dbUser._id !== _id) { - throw `Unavailable` + throw new EmailUnavailableError(email) } } From 56567de04a406eae501ae81bb8006ed0a42b71e8 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 10 Apr 2023 20:12:42 +0100 Subject: [PATCH 3/5] Fix tests --- .../worker/src/api/routes/global/tests/users.spec.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index d1afa0191e..0311669355 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -48,7 +48,7 @@ describe("/api/global/users", () => { 400 ) - expect(res.body.message).toBe("Unavailable") + expect(res.body.message).toBe(`Unavailable`) expect(sendMailMock).toHaveBeenCalledTimes(0) expect(code).toBeUndefined() expect(events.user.invited).toBeCalledTimes(0) @@ -225,7 +225,7 @@ describe("/api/global/users", () => { const response = await config.api.users.saveUser(user, 400) - expect(response.body.message).toBe(`Unavailable`) + expect(response.body.message).toBe(`Email already taken: '${user.email}'`) expect(events.user.created).toBeCalledTimes(0) }) @@ -237,7 +237,9 @@ describe("/api/global/users", () => { delete user._id const response = await config.api.users.saveUser(user, 400) - expect(response.body.message).toBe(`Unavailable`) + expect(response.body.message).toBe( + `Email already taken: '${user.email}'` + ) expect(events.user.created).toBeCalledTimes(0) }) }) @@ -249,7 +251,7 @@ describe("/api/global/users", () => { const response = await config.api.users.saveUser(user, 400) - expect(response.body.message).toBe(`Unavailable`) + expect(response.body.message).toBe(`Email already taken: '${user.email}'`) expect(events.user.created).toBeCalledTimes(0) }) From c77ed3d737e3469668de5cacf608ad823f7c4588 Mon Sep 17 00:00:00 2001 From: adrinr Date: Mon, 10 Apr 2023 20:17:51 +0100 Subject: [PATCH 4/5] Renames --- packages/backend-core/src/errors/errors.ts | 2 +- .../worker/src/api/routes/global/tests/users.spec.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/errors/errors.ts b/packages/backend-core/src/errors/errors.ts index 0a632d7551..4e1f1abbb5 100644 --- a/packages/backend-core/src/errors/errors.ts +++ b/packages/backend-core/src/errors/errors.ts @@ -102,6 +102,6 @@ export class InvalidAPIKeyError extends BudibaseError { export class EmailUnavailableError extends Error { constructor(email: string) { - super(`Email already taken: '${email}'`) + super(`Email already in use: '${email}'`) } } diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index 0311669355..52d77cbae6 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -225,7 +225,9 @@ describe("/api/global/users", () => { const response = await config.api.users.saveUser(user, 400) - expect(response.body.message).toBe(`Email already taken: '${user.email}'`) + expect(response.body.message).toBe( + `Email already in use: '${user.email}'` + ) expect(events.user.created).toBeCalledTimes(0) }) @@ -238,7 +240,7 @@ describe("/api/global/users", () => { const response = await config.api.users.saveUser(user, 400) expect(response.body.message).toBe( - `Email already taken: '${user.email}'` + `Email already in use: '${user.email}'` ) expect(events.user.created).toBeCalledTimes(0) }) @@ -251,7 +253,9 @@ describe("/api/global/users", () => { const response = await config.api.users.saveUser(user, 400) - expect(response.body.message).toBe(`Email already taken: '${user.email}'`) + expect(response.body.message).toBe( + `Email already in use: '${user.email}'` + ) expect(events.user.created).toBeCalledTimes(0) }) From 9a330b85f89989dc47b937b8f12706feee101936 Mon Sep 17 00:00:00 2001 From: adrinr Date: Tue, 11 Apr 2023 14:20:15 +0100 Subject: [PATCH 5/5] Fix timeouts --- packages/server/src/api/routes/tests/automation.spec.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/automation.spec.js b/packages/server/src/api/routes/tests/automation.spec.js index 2eae5e2a61..47daa8975b 100644 --- a/packages/server/src/api/routes/tests/automation.spec.js +++ b/packages/server/src/api/routes/tests/automation.spec.js @@ -19,11 +19,14 @@ describe("/automations", () => { afterAll(setup.afterAll) - // For some reason this cannot be a beforeAll or the test "tests the automation successfully" fail - beforeEach(async () => { + beforeAll(async () => { await config.init() }) + beforeEach(() => { + events.automation.deleted.mockClear() + }) + describe("get definitions", () => { it("returns a list of definitions for actions", async () => { const res = await request