From d591acf2d3378dec224cc1e50432c5cd5d822a96 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Tue, 23 Aug 2022 09:37:13 +0100 Subject: [PATCH] User fixes wip --- packages/backend-core/src/db/constants.ts | 2 + packages/backend-core/src/db/views.js | 69 +++++-- packages/types/src/api/web/index.ts | 1 + packages/types/src/api/web/user.ts | 10 + packages/types/src/documents/global/user.ts | 19 ++ .../types/src/documents/platform/index.ts | 1 + .../types/src/documents/platform/users.ts | 9 + .../src/api/controllers/global/users.ts | 4 +- .../tests/{users.spec.js => users.spec.ts} | 180 +++++++++++------- packages/worker/src/sdk/users/users.ts | 72 ++++++- packages/worker/src/tests/index.js | 12 -- packages/worker/src/tests/index.ts | 15 ++ .../tests/structures/{index.js => index.ts} | 8 +- packages/worker/src/tests/structures/users.ts | 10 +- 14 files changed, 296 insertions(+), 116 deletions(-) create mode 100644 packages/types/src/api/web/user.ts create mode 100644 packages/types/src/documents/platform/users.ts rename packages/worker/src/api/routes/tests/{users.spec.js => users.spec.ts} (79%) delete mode 100644 packages/worker/src/tests/index.js create mode 100644 packages/worker/src/tests/index.ts rename packages/worker/src/tests/structures/{index.js => index.ts} (54%) diff --git a/packages/backend-core/src/db/constants.ts b/packages/backend-core/src/db/constants.ts index 460476da24..b03382ad36 100644 --- a/packages/backend-core/src/db/constants.ts +++ b/packages/backend-core/src/db/constants.ts @@ -18,6 +18,7 @@ export enum ViewName { LINK = "by_link", ROUTING = "screen_routes", AUTOMATION_LOGS = "automation_logs", + ACCOUNT_BY_EMAIL = "account_by_email", } export const DeprecatedViews = { @@ -41,6 +42,7 @@ export enum DocumentType { MIGRATIONS = "migrations", DEV_INFO = "devinfo", AUTOMATION_LOG = "log_au", + ACCOUNT = "acc", } export const StaticDatabases = { diff --git a/packages/backend-core/src/db/views.js b/packages/backend-core/src/db/views.js index 3a45611a8f..c0a91581ac 100644 --- a/packages/backend-core/src/db/views.js +++ b/packages/backend-core/src/db/views.js @@ -5,6 +5,8 @@ const { SEPARATOR, } = require("./utils") const { getGlobalDB } = require("../tenancy") +const { StaticDatabases } = require("./constants") +const { doWithDB } = require("./"); const DESIGN_DB = "_design/database" @@ -56,6 +58,31 @@ exports.createNewUserEmailView = async () => { await db.put(designDoc) } +exports.createAccountEmailView = async () => { + await doWithDB(StaticDatabases.PLATFORM_INFO.name, async (db) => { + let designDoc + try { + designDoc = await db.get(DESIGN_DB) + } catch (err) { + // no design doc, make one + designDoc = DesignDoc() + } + const view = { + // if using variables in a map function need to inject them before use + map: `function(doc) { + if (doc._id.startsWith("${DocumentType.ACCOUNT}${SEPARATOR}")) { + emit(doc.email.toLowerCase(), doc.tenantId) + } + }`, + } + designDoc.views = { + ...designDoc.views, + [ViewName.ACCOUNT_BY_EMAIL]: view, + } + await db.put(designDoc) + }) +} + exports.createUserAppView = async () => { const db = getGlobalDB() let designDoc @@ -128,23 +155,17 @@ exports.createUserBuildersView = async () => { await db.put(designDoc) } -exports.queryGlobalView = async (viewName, params, db = null) => { - const CreateFuncByName = { - [ViewName.USER_BY_EMAIL]: exports.createNewUserEmailView, - [ViewName.BY_API_KEY]: exports.createApiKeyView, - [ViewName.USER_BY_BUILDERS]: exports.createUserBuildersView, - [ViewName.USER_BY_APP]: exports.createUserAppView, - } - // can pass DB in if working with something specific - if (!db) { - db = getGlobalDB() - } +exports.queryView = async (viewName, params, db, CreateFuncByName) => { try { let response = (await db.query(`database/${viewName}`, params)).rows response = response.map(resp => params.include_docs ? resp.doc : resp.value ) - return response.length <= 1 ? response[0] : response + if (params.arrayResponse) { + return response + } else { + return response.length <= 1 ? response[0] : response + } } catch (err) { if (err != null && err.name === "not_found") { const createFunc = CreateFuncByName[viewName] @@ -156,3 +177,27 @@ exports.queryGlobalView = async (viewName, params, db = null) => { } } } + +exports.queryPlatformView = async (viewName, params) => { + const CreateFuncByName = { + [ViewName.ACCOUNT_BY_EMAIL]: exports.createAccountEmailView, + } + + return doWithDB(StaticDatabases.PLATFORM_INFO.name, async (db) => { + return exports.queryView(viewName, params, db, CreateFuncByName) + }) +} + +exports.queryGlobalView = async (viewName, params, db = null) => { + const CreateFuncByName = { + [ViewName.USER_BY_EMAIL]: exports.createNewUserEmailView, + [ViewName.BY_API_KEY]: exports.createApiKeyView, + [ViewName.USER_BY_BUILDERS]: exports.createUserBuildersView, + [ViewName.USER_BY_APP]: exports.createUserAppView, + } + // can pass DB in if working with something specific + if (!db) { + db = getGlobalDB() + } + return exports.queryView(viewName, params, db, CreateFuncByName) +} diff --git a/packages/types/src/api/web/index.ts b/packages/types/src/api/web/index.ts index b2258fe18e..2a3a01106f 100644 --- a/packages/types/src/api/web/index.ts +++ b/packages/types/src/api/web/index.ts @@ -1 +1,2 @@ export * from "./analytics" +export * from "./user" \ No newline at end of file diff --git a/packages/types/src/api/web/user.ts b/packages/types/src/api/web/user.ts new file mode 100644 index 0000000000..3e9d410473 --- /dev/null +++ b/packages/types/src/api/web/user.ts @@ -0,0 +1,10 @@ +import { User } from "../../documents" + +export interface BulkCreateUsersRequest { + users: User[] + groups: any[] +} + +export interface BulkDeleteUsersRequest { + userIds: string[] +} \ No newline at end of file diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index c9255a1bb1..1dc1f4f228 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -15,8 +15,27 @@ export interface User extends Document { status?: string createdAt?: number // override the default createdAt behaviour - users sdk historically set this to Date.now() userGroups?: string[] + forceResetPassword?: boolean } export interface UserRoles { [key: string]: string } + + +// utility types + +export interface BuilderUser extends User { + builder: { + global: boolean + } +} + +export interface AdminUser extends User { + admin: { + global: boolean + }, + builder: { + global: boolean + } +} \ No newline at end of file diff --git a/packages/types/src/documents/platform/index.ts b/packages/types/src/documents/platform/index.ts index ba329f1bd0..0438f720f4 100644 --- a/packages/types/src/documents/platform/index.ts +++ b/packages/types/src/documents/platform/index.ts @@ -1 +1,2 @@ export * from "./info" +export * from "./users" \ No newline at end of file diff --git a/packages/types/src/documents/platform/users.ts b/packages/types/src/documents/platform/users.ts new file mode 100644 index 0000000000..1cf5377965 --- /dev/null +++ b/packages/types/src/documents/platform/users.ts @@ -0,0 +1,9 @@ +import { Document } from "../document"; + +/** + * doc id is user email + */ +export interface PlatformUserByEmail extends Document { + tenantId: string + userId: string +} diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 1f9af3514b..3938a6427a 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -46,8 +46,8 @@ export const bulkCreate = async (ctx: any) => { } try { - let response = await users.bulkCreate(newUsersRequested, groups) - await groupUtils.bulkSaveGroupUsers(groupsToSave, response) + const response = await users.bulkCreate(newUsersRequested, groups) + await groupUtils.bulkSaveGroupUsers(groupsToSave, response.successful) ctx.body = response } catch (err: any) { diff --git a/packages/worker/src/api/routes/tests/users.spec.js b/packages/worker/src/api/routes/tests/users.spec.ts similarity index 79% rename from packages/worker/src/api/routes/tests/users.spec.js rename to packages/worker/src/api/routes/tests/users.spec.ts index 5813dd3852..c95a7a7c4d 100644 --- a/packages/worker/src/api/routes/tests/users.spec.js +++ b/packages/worker/src/api/routes/tests/users.spec.ts @@ -1,7 +1,9 @@ jest.mock("nodemailer") -const { config, request, mocks, structures } = require("../../../tests") +import { config, request, mocks, structures } from "../../../tests" const sendMailMock = mocks.email.mock() -const { events } = require("@budibase/backend-core") +import { events } from "@budibase/backend-core" +import { User, BulkCreateUsersRequest, BulkDeleteUsersRequest } from "@budibase/types" + describe("/api/global/users", () => { beforeAll(async () => { @@ -12,6 +14,10 @@ describe("/api/global/users", () => { await config.afterAll() }) + beforeEach(() => { + jest.clearAllMocks() + }) + const sendUserInvite = async () => { await config.saveSmtpConfig() await config.saveSettingsConfig() @@ -31,35 +37,75 @@ describe("/api/global/users", () => { return { code, res } } - it("should be able to generate an invitation", async () => { - const { code, res } = await sendUserInvite() + describe("invite", () => { + it("should be able to generate an invitation", async () => { + const { code, res } = await sendUserInvite() - expect(res.body).toEqual({ message: "Invitation has been sent." }) - expect(sendMailMock).toHaveBeenCalled() - expect(code).toBeDefined() - expect(events.user.invited).toBeCalledTimes(1) + expect(res.body).toEqual({ message: "Invitation has been sent." }) + expect(sendMailMock).toHaveBeenCalled() + expect(code).toBeDefined() + expect(events.user.invited).toBeCalledTimes(1) + }) + + it("should be able to create new user from invite", async () => { + const { code } = await sendUserInvite() + + const res = await request + .post(`/api/global/users/invite/accept`) + .send({ + password: "newpassword", + inviteCode: code, + }) + .expect("Content-Type", /json/) + .expect(200) + expect(res.body._id).toBeDefined() + const user = await config.getUser("invite@test.com") + expect(user).toBeDefined() + expect(user._id).toEqual(res.body._id) + expect(events.user.inviteAccepted).toBeCalledTimes(1) + expect(events.user.inviteAccepted).toBeCalledWith(user) + }) }) - it("should be able to create new user from invite", async () => { - const { code } = await sendUserInvite() - + const bulkCreateUsers = async (users: User[], groups: any[] = []) => { + const body: BulkCreateUsersRequest = { users, groups } const res = await request - .post(`/api/global/users/invite/accept`) - .send({ - password: "newpassword", - inviteCode: code, - }) + .post(`/api/global/users/bulkCreate`) + .send(body) + .set(config.defaultHeaders()) .expect("Content-Type", /json/) .expect(200) - expect(res.body._id).toBeDefined() - const user = await config.getUser("invite@test.com") - expect(user).toBeDefined() - expect(user._id).toEqual(res.body._id) - expect(events.user.inviteAccepted).toBeCalledTimes(1) - expect(events.user.inviteAccepted).toBeCalledWith(user) + return res.body + } + + describe("bulkCreate", () => { + + it("should ignore users existing in the same tenant", async () => { + await bulkCreateUsers(toCreate) + }) + + it("should ignore users existing in other tenants", async () => { + await bulkCreateUsers(toCreate) + }) + + it("should ignore accounts using the same email", async () => { + await bulkCreateUsers(toCreate) + }) + + it("should be able to bulkCreate users with different permissions", async () => { + const builder = structures.users.builderUser({ email: "bulkbasic@test.com" }) + const admin = structures.users.adminUser({ email: "bulkadmin@test.com" }) + const user = structures.users.user({ email: "bulkuser@test.com" }) + + await bulkCreateUsers([builder, admin, user]) + + expect(events.user.created).toBeCalledTimes(3) + expect(events.user.permissionAdminAssigned).toBeCalledTimes(1) + expect(events.user.permissionBuilderAssigned).toBeCalledTimes(1) + }) }) - const createUser = async (user) => { + const createUser = async (user: User) => { const existing = await config.getUser(user.email) if (existing) { await deleteUser(existing._id) @@ -67,13 +113,13 @@ describe("/api/global/users", () => { return saveUser(user) } - const updateUser = async (user) => { + const updateUser = async (user: User) => { const existing = await config.getUser(user.email) user._id = existing._id return saveUser(user) } - const saveUser = async (user) => { + const saveUser = async (user: User) => { const res = await request .post(`/api/global/users`) .send(user) @@ -83,30 +129,20 @@ describe("/api/global/users", () => { return res.body } - - const bulkCreateUsers = async (users) => { - const res = await request - .post(`/api/global/users/bulkCreate`) - .send(users) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) - return res.body - } - - const bulkDeleteUsers = async (users) => { + const bulkDeleteUsers = async (users: User[]) => { + const body: BulkDeleteUsersRequest = { + userIds: users.map(u => u._id!) + } const res = await request .post(`/api/global/users/bulkDelete`) - .send(users) + .send(body) .set(config.defaultHeaders()) .expect("Content-Type", /json/) .expect(200) return res.body } - - - const deleteUser = async (email) => { + const deleteUser = async (email: string) => { const user = await config.getUser(email) if (user) { await request @@ -119,7 +155,6 @@ describe("/api/global/users", () => { describe("create", () => { it("should be able to create a basic user", async () => { - jest.clearAllMocks() const user = structures.users.user({ email: "basic@test.com" }) await createUser(user) @@ -129,23 +164,8 @@ describe("/api/global/users", () => { expect(events.user.permissionAdminAssigned).not.toBeCalled() }) - it("should be able to bulkCreate users with different permissions", async () => { - jest.clearAllMocks() - const builder = structures.users.builderUser({ email: "bulkbasic@test.com" }) - const admin = structures.users.adminUser({ email: "bulkadmin@test.com" }) - const user = structures.users.user({ email: "bulkuser@test.com" }) - - let toCreate = { users: [builder, admin, user], groups: [] } - await bulkCreateUsers(toCreate) - - expect(events.user.created).toBeCalledTimes(3) - expect(events.user.permissionAdminAssigned).toBeCalledTimes(1) - expect(events.user.permissionBuilderAssigned).toBeCalledTimes(1) - }) - it("should be able to create an admin user", async () => { - jest.clearAllMocks() const user = structures.users.adminUser({ email: "admin@test.com" }) await createUser(user) @@ -156,7 +176,6 @@ describe("/api/global/users", () => { }) it("should be able to create a builder user", async () => { - jest.clearAllMocks() const user = structures.users.builderUser({ email: "builder@test.com" }) await createUser(user) @@ -167,7 +186,6 @@ describe("/api/global/users", () => { }) it("should be able to assign app roles", async () => { - jest.clearAllMocks() const user = structures.users.user({ email: "assign-roles@test.com" }) user.roles = { "app_123": "role1", @@ -230,7 +248,7 @@ describe("/api/global/users", () => { }) it("should be able to update a basic user to a builder user", async () => { - let user = structures.users.user({ email: "basic-update-builder@test.com" }) + const user = structures.users.user({ email: "basic-update-builder@test.com" }) await createUser(user) jest.clearAllMocks() @@ -243,7 +261,7 @@ describe("/api/global/users", () => { }) it("should be able to update an admin user to a basic user", async () => { - let user = structures.users.adminUser({ email: "admin-update-basic@test.com" }) + const user = structures.users.adminUser({ email: "admin-update-basic@test.com" }) await createUser(user) jest.clearAllMocks() @@ -257,7 +275,7 @@ describe("/api/global/users", () => { }) it("should be able to update an builder user to a basic user", async () => { - let user = structures.users.builderUser({ email: "builder-update-basic@test.com" }) + const user = structures.users.builderUser({ email: "builder-update-basic@test.com" }) await createUser(user) jest.clearAllMocks() @@ -334,6 +352,29 @@ describe("/api/global/users", () => { }) }) + describe("bulkDelete", () => { + + it("should not be able to bulkDelete account admin as admin", async () => { + + }) + + it("should not be able to bulkDelete account owner as account owner", async () => { + + }) + + it("should be able to bulk delete users with different permissions", async () => { + const builder = structures.users.builderUser({ email: "basic@test.com" }) + const admin = structures.users.adminUser({ email: "admin@test.com" }) + const user = structures.users.user({ email: "user@test.com" }) + + const createdUsers = await bulkCreateUsers([builder, admin, user]) + await bulkDeleteUsers(createdUsers) + expect(events.user.deleted).toBeCalledTimes(3) + expect(events.user.permissionAdminRemoved).toBeCalledTimes(1) + expect(events.user.permissionBuilderRemoved).toBeCalledTimes(1) + }) + }) + describe("destroy", () => { it("should be able to destroy a basic user", async () => { let user = structures.users.user({ email: "destroy@test.com" }) @@ -371,18 +412,11 @@ describe("/api/global/users", () => { expect(events.user.permissionAdminRemoved).not.toBeCalled() }) - it("should be able to bulk delete users with different permissions", async () => { - jest.clearAllMocks() - const builder = structures.users.builderUser({ email: "basic@test.com" }) - const admin = structures.users.adminUser({ email: "admin@test.com" }) - const user = structures.users.user({ email: "user@test.com" }) + it("should not be able to destroy account admin as admin", async () => { - let toCreate = { users: [builder, admin, user], groups: [] } - let createdUsers = await bulkCreateUsers(toCreate) - await bulkDeleteUsers({ userIds: [createdUsers[0]._id, createdUsers[1]._id, createdUsers[2]._id] }) - expect(events.user.deleted).toBeCalledTimes(3) - expect(events.user.permissionAdminRemoved).toBeCalledTimes(1) - expect(events.user.permissionBuilderRemoved).toBeCalledTimes(1) + }) + + it("should not be able to destroy account owner as account owner", async () => { }) diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index 58c2decabf..cc55c0b70b 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -14,8 +14,10 @@ import { HTTPError, accounts, migrations, + StaticDatabases, + ViewName } from "@budibase/backend-core" -import { MigrationType, User } from "@budibase/types" +import { MigrationType, PlatformUserByEmail, User, Account } from "@budibase/types" import { groups as groupUtils } from "@budibase/pro" const PAGE_LIMIT = 8 @@ -247,6 +249,54 @@ export const addTenant = async ( } } +const getExistingTenantUsers = async (emails: string[]): Promise => { + return dbUtils.queryGlobalView(ViewName.USER_BY_EMAIL, { + keys: emails, + include_docs: true, + arrayResponse: true + }) +} + +const getExistingPlatformUsers = async (emails: string[]): Promise => { + return dbUtils.doWithDB(StaticDatabases.PLATFORM_INFO.name, async (infoDb: any) => { + const response = await infoDb.allDocs({ + keys: emails, + include_docs: true, + }) + return response.rows.map((row: any) => row.doc) + }) +} + +const getExistingAccounts = async (emails: string[]): Promise => { + return dbUtils.queryPlatformView(ViewName.ACCOUNT_BY_EMAIL, { + keys: emails, + include_docs: true, + arrayResponse: true + }) +} + +/** + * Apply a system-wide search on emails: + * - in tenant + * - cross tenant + * - accounts + * return an array of emails that match the supplied emails. + */ +const searchExistingEmails = async (emails: string[]) => { + let matchedEmails: string[] = [] + + const existingTenantUsers = await getExistingTenantUsers(emails) + matchedEmails.push(...existingTenantUsers.map((user: User) => user.email)) + + const existingPlatformUsers = await getExistingPlatformUsers(emails) + matchedEmails.push(...existingPlatformUsers.map((user: PlatformUserByEmail) => user._id!)) + + const existingAccounts = await getExistingAccounts(emails) + matchedEmails.push(...existingAccounts.map((account: Account) => account.email)) + + return matchedEmails +} + export const bulkCreate = async ( newUsersRequested: User[], groups: string[] @@ -257,19 +307,16 @@ export const bulkCreate = async ( let usersToSave: any[] = [] let newUsers: any[] = [] - const allUsers = await db.allDocs( - dbUtils.getGlobalUserParams(null, { - include_docs: true, - }) - ) - let mapped = allUsers.rows.map((row: any) => row.id) + const emails = newUsersRequested.map((user: User) => user.email) + const existingEmails = await searchExistingEmails(emails) + const unsuccessful: { email: string, reason: string }[] = [] - const currentUserEmails = mapped.map((x: any) => x.email) || [] for (const newUser of newUsersRequested) { if ( newUsers.find((x: any) => x.email === newUser.email) || - currentUserEmails.includes(newUser.email) + existingEmails.includes(newUser.email) ) { + unsuccessful.push({ email: newUser.email, reason: `Email address ${newUser.email} already in use.` }) continue } newUser.userGroups = groups @@ -307,12 +354,17 @@ export const bulkCreate = async ( await apps.syncUserInApps(user._id) } - return usersToBulkSave.map(user => { + const saved = usersToBulkSave.map(user => { return { _id: user._id, email: user.email, } }) + + return { + successful: saved, + unsuccessful + } } export const bulkDelete = async (userIds: any) => { diff --git a/packages/worker/src/tests/index.js b/packages/worker/src/tests/index.js deleted file mode 100644 index 9aa88dc444..0000000000 --- a/packages/worker/src/tests/index.js +++ /dev/null @@ -1,12 +0,0 @@ -const TestConfiguration = require("./TestConfiguration") -const structures = require("./structures") -const mocks = require("./mocks") -const config = new TestConfiguration() -const request = config.getRequest() - -module.exports = { - structures, - mocks, - config, - request, -} diff --git a/packages/worker/src/tests/index.ts b/packages/worker/src/tests/index.ts new file mode 100644 index 0000000000..de4c1b7bcd --- /dev/null +++ b/packages/worker/src/tests/index.ts @@ -0,0 +1,15 @@ +import TestConfiguration from "./TestConfiguration" +import structures from "./structures" +import mocks from "./mocks" + +const config = new TestConfiguration() +const request = config.getRequest() + +const pkg = { + structures, + mocks, + config, + request, +} + +export = pkg diff --git a/packages/worker/src/tests/structures/index.js b/packages/worker/src/tests/structures/index.ts similarity index 54% rename from packages/worker/src/tests/structures/index.js rename to packages/worker/src/tests/structures/index.ts index 3212ae606d..61e683f621 100644 --- a/packages/worker/src/tests/structures/index.js +++ b/packages/worker/src/tests/structures/index.ts @@ -1,11 +1,11 @@ -const configs = require("./configs") -const users = require("./users") -const groups = require("./groups") +import configs from "./configs" +import * as users from "./users" +import * as groups from "./groups" const TENANT_ID = "default" const CSRF_TOKEN = "e3727778-7af0-4226-b5eb-f43cbe60a306" -module.exports = { +export = { configs, users, TENANT_ID, diff --git a/packages/worker/src/tests/structures/users.ts b/packages/worker/src/tests/structures/users.ts index dce771aaa7..21aa86663c 100644 --- a/packages/worker/src/tests/structures/users.ts +++ b/packages/worker/src/tests/structures/users.ts @@ -1,6 +1,7 @@ export const email = "test@test.com" +import { AdminUser, BuilderUser, User } from "@budibase/types" -export const user = (userProps: any) => { +export const user = (userProps: any): User => { return { email: "test@test.com", password: "test", @@ -9,16 +10,19 @@ export const user = (userProps: any) => { } } -export const adminUser = (userProps: any) => { +export const adminUser = (userProps: any): AdminUser => { return { ...user(userProps), admin: { global: true, }, + builder: { + global: true + } } } -export const builderUser = (userProps: any) => { +export const builderUser = (userProps: any): BuilderUser => { return { ...user(userProps), builder: {