From 46a6fc50482b53ac1915d0a135c7ff9f49f30888 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 26 Jul 2022 20:04:29 +0100 Subject: [PATCH] refactor groups into pro and some other pr comments --- packages/backend-core/src/errors/licensing.js | 4 +- .../src/events/publishers/group.ts | 10 ++- .../server/src/api/routes/tests/user.spec.js | 15 ++++ packages/server/src/utilities/global.js | 29 ++++++-- packages/types/src/documents/global/user.ts | 4 +- .../types/src/documents/global/userGroup.ts | 2 +- packages/types/src/sdk/events/event.ts | 8 +-- packages/types/src/sdk/events/userGroup.ts | 4 +- .../src/api/controllers/global/users.ts | 72 +------------------ packages/worker/src/sdk/users/index.ts | 1 - packages/worker/src/sdk/users/users.ts | 34 ++++++--- 11 files changed, 77 insertions(+), 106 deletions(-) diff --git a/packages/backend-core/src/errors/licensing.js b/packages/backend-core/src/errors/licensing.js index ca36f12a23..822adf4304 100644 --- a/packages/backend-core/src/errors/licensing.js +++ b/packages/backend-core/src/errors/licensing.js @@ -28,9 +28,9 @@ class UsageLimitError extends HTTPError { } class FeatureDisabledError extends HTTPError { - constructor(message, limitName) { + constructor(message, featureName) { super(message, 400, codes.FEATURE_DISABLED, type) - this.limitName = limitName + this.featureName = featureName } } diff --git a/packages/backend-core/src/events/publishers/group.ts b/packages/backend-core/src/events/publishers/group.ts index 7197e4d6b0..ab805c5610 100644 --- a/packages/backend-core/src/events/publishers/group.ts +++ b/packages/backend-core/src/events/publishers/group.ts @@ -7,7 +7,7 @@ import { GroupUpdatedEvent, GroupUsersAddedEvent, GroupUsersDeletedEvent, - GroupsAddedOnboarding, + GroupAddedOnboardingEvent, UserGroupRoles, } from "@budibase/types" @@ -34,24 +34,22 @@ export async function deleted(group: UserGroup) { export async function usersAdded(emails: string[], group: UserGroup) { const properties: GroupUsersAddedEvent = { - emails, count: emails.length, groupId: group._id as string, } - await publishEvent(Event.USER_GROUP_USER_ADDED, properties) + await publishEvent(Event.USER_GROUP_USERS_ADDED, properties) } export async function usersDeleted(emails: string[], group: UserGroup) { const properties: GroupUsersDeletedEvent = { - emails, count: emails.length, groupId: group._id as string, } - await publishEvent(Event.USER_GROUP_USER_REMOVED, properties) + await publishEvent(Event.USER_GROUP_USERS_REMOVED, properties) } export async function createdOnboarding(groupId: string) { - const properties: GroupsAddedOnboarding = { + const properties: GroupAddedOnboardingEvent = { groupId: groupId, onboarding: true, } diff --git a/packages/server/src/api/routes/tests/user.spec.js b/packages/server/src/api/routes/tests/user.spec.js index 04b7297df3..e699f818d6 100644 --- a/packages/server/src/api/routes/tests/user.spec.js +++ b/packages/server/src/api/routes/tests/user.spec.js @@ -24,6 +24,21 @@ describe("/users", () => { describe("fetch", () => { + it("returns a list of users from an instance db", async () => { + await config.createUser("uuidx") + await config.createUser("uuidy") + const res = await request + .get(`/api/users/metadata`) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) + + expect(res.body.length).toBe(3) + expect(res.body.find(u => u._id === `ro_ta_users_us_uuidx`)).toBeDefined() + expect(res.body.find(u => u._id === `ro_ta_users_us_uuidy`)).toBeDefined() + }) + + it("should apply authorization to endpoint", async () => { await config.createUser() await checkPermissionsEndpoint({ diff --git a/packages/server/src/utilities/global.js b/packages/server/src/utilities/global.js index 81c64b9cde..abf10aff20 100644 --- a/packages/server/src/utilities/global.js +++ b/packages/server/src/utilities/global.js @@ -14,8 +14,9 @@ const env = require("../environment") const { getAppId } = require("@budibase/backend-core/context") const { groups } = require("@budibase/pro") -exports.updateAppRole = async (user, { appId } = {}) => { +exports.updateAppRole = (user, { appId } = {}) => { appId = appId || getAppId() + if (!user || !user.roles) { return user } @@ -31,21 +32,33 @@ exports.updateAppRole = async (user, { appId } = {}) => { // if a role wasn't found then either set as admin (builder) or public (everyone else) if (!user.roleId && user.builder && user.builder.global) { user.roleId = BUILTIN_ROLE_IDS.ADMIN - } else if (!user.roleId && !user?.userGroups?.length) { + } else if (!user.roleId) { user.roleId = BUILTIN_ROLE_IDS.PUBLIC - } else if (!user.roleId && user?.userGroups?.length) { - let roleId = await groups.getGroupRoleId(user, appId) - user.roleId = roleId } + delete user.roles return user } -function processUser(user, { appId } = {}) { +async function checkGroupRoles(user, { appId } = {}) { + if (!user.roleId) { + let roleId = await groups.getGroupRoleId(user, appId) + user.roleId = roleId + } + + return user +} + +async function processUser(user, { appId } = {}) { if (user) { delete user.password } - return exports.updateAppRole(user, { appId }) + user = await exports.updateAppRole(user, { appId }) + if (user?.userGroups?.length) { + user = await checkGroupRoles(user, { appId }) + } + + return user } exports.getCachedSelf = async (ctx, appId) => { @@ -93,6 +106,8 @@ exports.getGlobalUsers = async (users = null) => { if (!appId) { return globalUsers } + console.log("maybe??") + return globalUsers.map(user => exports.updateAppRole(user)) } diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index 1203958f2d..41fcc4aa7b 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -14,8 +14,8 @@ export interface User extends Document { password?: string status?: string - createdAt?: number - userGroups?: string[] // override the default createdAt behaviour - users sdk historically set this to Date.now() + createdAt?: number // override the default createdAt behaviour - users sdk historically set this to Date.now() + userGroups?: string[] } export interface UserRoles { diff --git a/packages/types/src/documents/global/userGroup.ts b/packages/types/src/documents/global/userGroup.ts index eda0cdfeca..e37286222e 100644 --- a/packages/types/src/documents/global/userGroup.ts +++ b/packages/types/src/documents/global/userGroup.ts @@ -5,7 +5,7 @@ export interface UserGroup extends Document { icon: string color: string users: User[] - apps: any + apps: any[] roles: UserGroupRoles createdAt?: number } diff --git a/packages/types/src/sdk/events/event.ts b/packages/types/src/sdk/events/event.ts index 2d63addc0e..7ec1bf157e 100644 --- a/packages/types/src/sdk/events/event.ts +++ b/packages/types/src/sdk/events/event.ts @@ -155,10 +155,10 @@ export enum Event { USER_GROUP_CREATED = "user_group:created", USER_GROUP_UPDATED = "user_group:updated", USER_GROUP_DELETED = "user_group:deleted", - USER_GROUP_USER_ADDED = "user_group_user:added", - USER_GROUP_USER_REMOVED = "user_group_user:deleted", - USER_GROUP_PERMISSIONS_EDITED = "user_group_permissions:edited", - USER_GROUP_ONBOARDING = "user_group_onboarding:added", + USER_GROUP_USERS_ADDED = "user_group:user_added", + USER_GROUP_USERS_REMOVED = "user_group_:users_deleted", + USER_GROUP_PERMISSIONS_EDITED = "user_group_:permissions_edited", + USER_GROUP_ONBOARDING = "user_group_:onboarding_added", } // properties added at the final stage of the event pipeline diff --git a/packages/types/src/sdk/events/userGroup.ts b/packages/types/src/sdk/events/userGroup.ts index fbcb13cf1e..2ce642e274 100644 --- a/packages/types/src/sdk/events/userGroup.ts +++ b/packages/types/src/sdk/events/userGroup.ts @@ -13,18 +13,16 @@ export interface GroupDeletedEvent extends BaseEvent { } export interface GroupUsersAddedEvent extends BaseEvent { - emails: string[] count: number groupId: string } export interface GroupUsersDeletedEvent extends BaseEvent { - emails: string[] count: number groupId: string } -export interface GroupsAddedOnboarding extends BaseEvent { +export interface GroupAddedOnboardingEvent extends BaseEvent { groupId: string onboarding: boolean } diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 618d815229..a6458e0432 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -13,7 +13,7 @@ import { cache, } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" - +import { groups as groupUtils } from "@budibase/pro" const MAX_USERS_UPLOAD_LIMIT = 1000 export const save = async (ctx: any) => { @@ -46,34 +46,8 @@ export const bulkCreate = async (ctx: any) => { try { let response = await users.bulkCreate(newUsersRequested, groups) + await groupUtils.bulkSaveGroupUsers(groupsToSave, response) - if (groupsToSave.length) { - let groupsPromises: any = [] - groupsToSave.forEach(async (userGroup: UserGroup) => { - userGroup.users = [...userGroup.users, ...response] - groupsPromises.push(db.put(userGroup)) - }) - - const groupResults = await Promise.all(groupsPromises) - await db.bulkDocs(groupResults) - - let eventFns = [] - for (const group of groupResults) { - eventFns.push(() => { - events.group.usersAdded( - response.map(u => u.email), - group - ) - }) - eventFns.push(() => { - events.group.createdOnboarding(group._id as string) - }) - } - - for (const fn of eventFns) { - await fn() - } - } ctx.body = response } catch (err: any) { ctx.throw(err.status || 400, err) @@ -142,27 +116,9 @@ export const adminUser = async (ctx: any) => { export const destroy = async (ctx: any) => { const id = ctx.params.id - const db = tenancy.getGlobalDB() - let user: User = await db.get(id) - let groups = user.userGroups await users.destroy(id, ctx.user) - // Remove asssosicated groups - if (groups) { - let groupsPromises = [] - for (const groupId of groups) { - let group = await db.get(groupId) - let updatedUsersGroup = group.users.filter( - (groupUser: any) => groupUser.email !== user.email - ) - group.users = updatedUsersGroup - groupsPromises.push(db.put(group)) - } - - await db.bulkDocs(groupsPromises) - } - ctx.body = { message: `User ${id} deleted.`, } @@ -170,30 +126,9 @@ export const destroy = async (ctx: any) => { export const bulkDelete = async (ctx: any) => { const { userIds } = ctx.request.body - const db = tenancy.getGlobalDB() try { let { groupsToModify, usersResponse } = await users.bulkDelete(userIds) - - // if there are groups to delete, do it here - if (Object.keys(groupsToModify).length) { - let groups = ( - await db.allDocs({ - include_docs: true, - keys: Object.keys(groupsToModify), - }) - ).rows.map((group: any) => group.doc) - - let groupsPromises = [] - for (const group of groups) { - let updatedUsersGroup = group.users.filter( - (groupUser: any) => !groupsToModify[group._id].includes(groupUser._id) - ) - group.users = updatedUsersGroup - groupsPromises.push(db.put(group)) - } - - await db.bulkDocs(groupsPromises) - } + await groupUtils.bulkDeleteGroupUsers(groupsToModify) ctx.body = { message: `${usersResponse.length} user(s) deleted`, @@ -314,7 +249,6 @@ export const inviteAccept = async (ctx: any) => { return saved }) } catch (err: any) { - console.log(err) if (err.code === errors.codes.USAGE_LIMIT_EXCEEDED) { // explicitly re-throw limit exceeded errors ctx.throw(400, err) diff --git a/packages/worker/src/sdk/users/index.ts b/packages/worker/src/sdk/users/index.ts index c30935a198..056d6e5675 100644 --- a/packages/worker/src/sdk/users/index.ts +++ b/packages/worker/src/sdk/users/index.ts @@ -1,2 +1 @@ export * from "./users" -export * from "./events" diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index 1cb140c15a..17de383ceb 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -3,7 +3,6 @@ import { quotas } from "@budibase/pro" import * as apps from "../../utilities/appService" import * as eventHelpers from "./events" import { - events, tenancy, utils, db as dbUtils, @@ -16,8 +15,8 @@ import { accounts, migrations, } from "@budibase/backend-core" -import { MigrationType, UserGroup } from "@budibase/types" -import { build } from "joi" +import { MigrationType, User } from "@budibase/types" +import { groups as groupUtils } from "@budibase/pro/" const PAGE_LIMIT = 8 @@ -107,10 +106,11 @@ export const buildUser = async ( ) => { let { password, _id } = user - // get the password, make sure one is defined let hashedPassword if (password) { hashedPassword = opts.hashPassword ? await utils.hash(password) : password + } else if (dbUser) { + hashedPassword = dbUser.password } else if (opts.requirePassword) { throw "Password must be specified." } @@ -125,7 +125,6 @@ export const buildUser = async ( password: hashedPassword, tenantId, } - // make sure the roles object is always present if (!user.roles) { user.roles = {} @@ -202,6 +201,7 @@ export const save = async ( const putUserFn = () => { return db.put(builtUser) } + console.log(builtUser) if (eventHelpers.isAddingBuilder(builtUser, dbUser)) { response = await quotas.addDeveloper(putUserFn) } else { @@ -244,7 +244,10 @@ export const addTenant = async ( } } -export const bulkCreate = async (newUsersRequested: any[], groups: any) => { +export const bulkCreate = async ( + newUsersRequested: User[], + groups: string[] +) => { const db = tenancy.getGlobalDB() const tenantId = tenancy.getTenantId() @@ -291,16 +294,18 @@ export const bulkCreate = async (newUsersRequested: any[], groups: any) => { }) const usersToBulkSave = await Promise.all(usersToSave) - await quotas.addDevelopers(() => db.bulkDocs(usersToBulkSave), builderCount) + const response = await quotas.addDevelopers( + () => db.bulkDocs(usersToBulkSave), + builderCount + ) // Post processing of bulk added users, i.e events and cache operations for (const user of usersToBulkSave) { - delete user.password - await eventHelpers.handleSaveEvents(user, null) - await apps.syncUserInApps(user._id) + //await eventHelpers.handleSaveEvents(user, null) + //await apps.syncUserInApps(user._id) } - return usersToBulkSave + return response } export const bulkDelete = async (userIds: any) => { @@ -356,6 +361,7 @@ export const bulkDelete = async (userIds: any) => { export const destroy = async (id: string, currentUser: any) => { const db = tenancy.getGlobalDB() const dbUser = await db.get(id) + let groups = dbUser.userGroups if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) { // root account holder can't be deleted from inside budibase @@ -371,7 +377,13 @@ export const destroy = async (id: string, currentUser: any) => { } await deprovisioning.removeUserFromInfoDB(dbUser) + await db.remove(dbUser._id, dbUser._rev) + + if (groups) { + await groupUtils.deleteGroupUsers(groups, dbUser) + } + await eventHelpers.handleDeleteEvents(dbUser) await quotas.removeUser(dbUser) await cache.user.invalidateUser(dbUser._id)