From 3214abb89a041763a8c661b2983705581acf3a6e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 9 Apr 2021 15:11:49 +0100 Subject: [PATCH 1/4] Global users now working through the server, all requests proxied. --- package.json | 2 +- packages/auth/src/db/utils.js | 7 +- .../src/components/backend/DataTable/api.js | 5 +- .../components/start/CreateAppModal.svelte | 2 +- .../server/src/api/controllers/application.js | 2 +- packages/server/src/api/controllers/auth.js | 4 +- .../server/src/api/controllers/hosting.js | 2 +- packages/server/src/api/controllers/role.js | 4 +- packages/server/src/api/controllers/row.js | 18 +-- .../src/api/controllers/static/index.js | 2 +- packages/server/src/api/controllers/user.js | 98 ++++----------- packages/server/src/db/utils.js | 21 +++- .../server/src/utilities/builder/hosting.js | 29 +---- .../server/src/utilities/workerRequests.js | 116 ++++++++++++++++++ .../worker/src/api/controllers/admin/index.js | 31 +++-- packages/worker/src/api/routes/app.js | 4 +- .../worker/src/middleware/authenticated.js | 32 ++--- 17 files changed, 219 insertions(+), 160 deletions(-) create mode 100644 packages/server/src/utilities/workerRequests.js diff --git a/package.json b/package.json index b2f572d202..a4b0993fde 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "clean": "lerna clean", "kill-port": "kill-port 4001", "dev": "yarn run kill-port && lerna link && lerna run --parallel dev:builder --concurrency 1", - "dev:noserver": "lerna link && lerna run --parallel dev:builder --concurrency 1 --ignore @budibase/server", + "dev:noserver": "lerna link && lerna run dev:stack:up && lerna run --parallel dev:builder --concurrency 1 --ignore @budibase/server --ignore @budibase/worker", "test": "lerna run test", "lint": "eslint packages", "lint:fix": "eslint --fix packages", diff --git a/packages/auth/src/db/utils.js b/packages/auth/src/db/utils.js index 9b6f5d7103..871423df03 100644 --- a/packages/auth/src/db/utils.js +++ b/packages/auth/src/db/utils.js @@ -26,13 +26,12 @@ exports.generateUserID = email => { * Gets parameters for retrieving users, this is a utility function for the getDocParams function. */ exports.getUserParams = (email = "", otherProps = {}) => { + if (!email) { + email = "" + } return { ...otherProps, startkey: `${DocumentTypes.USER}${SEPARATOR}${email}`, endkey: `${DocumentTypes.USER}${SEPARATOR}${email}${UNICODE_MAX}`, } } - -exports.getEmailFromUserID = id => { - return id.split(`${DocumentTypes.USER}${SEPARATOR}`)[1] -} \ No newline at end of file diff --git a/packages/builder/src/components/backend/DataTable/api.js b/packages/builder/src/components/backend/DataTable/api.js index 91ebc19b26..43c6856540 100644 --- a/packages/builder/src/components/backend/DataTable/api.js +++ b/packages/builder/src/components/backend/DataTable/api.js @@ -1,7 +1,7 @@ import api from "builderStore/api" export async function createUser(user) { - const CREATE_USER_URL = `/api/users` + const CREATE_USER_URL = `/api/users/metadata` const response = await api.post(CREATE_USER_URL, user) return await response.json() } @@ -15,8 +15,7 @@ export async function saveRow(row, tableId) { export async function deleteRow(row) { const DELETE_ROWS_URL = `/api/${row.tableId}/rows/${row._id}/${row._rev}` - const response = await api.delete(DELETE_ROWS_URL) - return response + return api.delete(DELETE_ROWS_URL) } export async function fetchDataForView(view) { diff --git a/packages/builder/src/components/start/CreateAppModal.svelte b/packages/builder/src/components/start/CreateAppModal.svelte index babe7bd0df..c727283b29 100644 --- a/packages/builder/src/components/start/CreateAppModal.svelte +++ b/packages/builder/src/components/start/CreateAppModal.svelte @@ -157,7 +157,7 @@ password: $createAppStore.values.password, roleId: $createAppStore.values.roleId, } - const userResp = await api.post(`/api/users`, user) + const userResp = await api.post(`/api/users/metadata`, user) const json = await userResp.json() $goto(`./${appJson._id}`) } catch (error) { diff --git a/packages/server/src/api/controllers/application.js b/packages/server/src/api/controllers/application.js index cacde4bb00..042474c5b6 100644 --- a/packages/server/src/api/controllers/application.js +++ b/packages/server/src/api/controllers/application.js @@ -74,7 +74,7 @@ async function getAppUrlIfNotInUse(ctx) { if (!env.SELF_HOSTED) { return url } - const deployedApps = await getDeployedApps() + const deployedApps = await getDeployedApps(ctx) if ( deployedApps[url] != null && deployedApps[url].appId !== ctx.params.appId diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index da68aa485b..6e662f7b9a 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/packages/server/src/api/controllers/auth.js @@ -3,7 +3,7 @@ const CouchDB = require("../../db") const bcrypt = require("../../utilities/bcrypt") const env = require("../../environment") const { getAPIKey } = require("../../utilities/usageQuota") -const { generateUserID } = require("../../db/utils") +const { generateUserMetadataID } = require("../../db/utils") const { setCookie } = require("../../utilities") const { outputProcessing } = require("../../utilities/rowProcessor") const { ViewNames } = require("../../db/utils") @@ -27,7 +27,7 @@ exports.authenticate = async ctx => { let dbUser try { - dbUser = await db.get(generateUserID(email)) + dbUser = await db.get(generateUserMetadataID(email)) } catch (_) { // do not want to throw a 404 - as this could be // used to determine valid emails diff --git a/packages/server/src/api/controllers/hosting.js b/packages/server/src/api/controllers/hosting.js index 4b070cf75b..8b7b31e00a 100644 --- a/packages/server/src/api/controllers/hosting.js +++ b/packages/server/src/api/controllers/hosting.js @@ -40,5 +40,5 @@ exports.fetchUrls = async ctx => { } exports.getDeployedApps = async ctx => { - ctx.body = await getDeployedApps() + ctx.body = await getDeployedApps(ctx) } diff --git a/packages/server/src/api/controllers/role.js b/packages/server/src/api/controllers/role.js index 11f81c1219..d27272a21a 100644 --- a/packages/server/src/api/controllers/role.js +++ b/packages/server/src/api/controllers/role.js @@ -10,7 +10,7 @@ const { const { generateRoleID, getRoleParams, - getUserParams, + getUserMetadataParams, ViewNames, } = require("../../db/utils") @@ -112,7 +112,7 @@ exports.destroy = async function(ctx) { // first check no users actively attached to role const users = ( await db.allDocs( - getUserParams(null, { + getUserMetadataParams(null, { include_docs: true, }) ) diff --git a/packages/server/src/api/controllers/row.js b/packages/server/src/api/controllers/row.js index 1b3e795f83..8b4a461d2c 100644 --- a/packages/server/src/api/controllers/row.js +++ b/packages/server/src/api/controllers/row.js @@ -6,8 +6,8 @@ const { generateRowID, DocumentTypes, SEPARATOR, - ViewNames, - generateUserID, + InternalTables, + generateUserMetadataID, } = require("../../db/utils") const usersController = require("./user") const { @@ -39,7 +39,7 @@ validateJs.extend(validateJs.validators.datetime, { async function findRow(db, appId, tableId, rowId) { let row - if (tableId === ViewNames.USERS) { + if (tableId === InternalTables.USER_METADATA) { let ctx = { params: { userId: rowId, @@ -97,7 +97,7 @@ exports.patch = async function(ctx) { }) // Creation of a new user goes to the user controller - if (row.tableId === ViewNames.USERS) { + if (row.tableId === InternalTables.USER_METADATA) { // the row has been updated, need to put it into the ctx ctx.request.body = { ...row, @@ -142,8 +142,8 @@ exports.save = async function(ctx) { } if (!inputs._rev && !inputs._id) { - if (inputs.tableId === ViewNames.USERS) { - inputs._id = generateUserID(inputs.email) + if (inputs.tableId === InternalTables.USER_METADATA) { + inputs._id = generateUserMetadataID(inputs.email) } else { inputs._id = generateRowID(inputs.tableId) } @@ -176,7 +176,7 @@ exports.save = async function(ctx) { }) // Creation of a new user goes to the user controller - if (row.tableId === ViewNames.USERS) { + if (row.tableId === InternalTables.USER_METADATA) { // the row has been updated, need to put it into the ctx ctx.request.body = row await usersController.createMetadata(ctx) @@ -289,7 +289,7 @@ exports.search = async function(ctx) { const response = await search(searchString) // delete passwords from users - if (tableId === ViewNames.USERS) { + if (tableId === InternalTables.USER_METADATA) { for (let row of response.rows) { delete row.password } @@ -309,7 +309,7 @@ exports.fetchTableRows = async function(ctx) { // special case for users, fetch through the user controller let rows, table = await db.get(ctx.params.tableId) - if (ctx.params.tableId === ViewNames.USERS) { + if (ctx.params.tableId === InternalTables.USER_METADATA) { await usersController.fetchMetadata(ctx) rows = ctx.body } else { diff --git a/packages/server/src/api/controllers/static/index.js b/packages/server/src/api/controllers/static/index.js index 69a12d573b..71e20923a8 100644 --- a/packages/server/src/api/controllers/static/index.js +++ b/packages/server/src/api/controllers/static/index.js @@ -22,7 +22,7 @@ const { objectStoreUrl, clientLibraryPath } = require("../../../utilities") async function checkForSelfHostedURL(ctx) { // the "appId" component of the URL may actually be a specific self hosted URL let possibleAppUrl = `/${encodeURI(ctx.params.appId).toLowerCase()}` - const apps = await getDeployedApps() + const apps = await getDeployedApps(ctx) if (apps[possibleAppUrl] && apps[possibleAppUrl].appId) { return apps[possibleAppUrl].appId } else { diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index 92b038d05e..c526050cde 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -1,71 +1,23 @@ const CouchDB = require("../../db") const { - generateUserID, - getUserParams, - getEmailFromUserID, -} = require("@budibase/auth") + generateUserMetadataID, + getUserMetadataParams, + getEmailFromUserMetadataID, +} = require("../../db/utils") const { InternalTables } = require("../../db/utils") const { getRole } = require("../../utilities/security/roles") -const { checkSlashesInUrl } = require("../../utilities") -const env = require("../../environment") -const fetch = require("node-fetch") - -async function deleteGlobalUser(email) { - const endpoint = `/api/admin/users/${email}` - const reqCfg = { method: "DELETE" } - const response = await fetch( - checkSlashesInUrl(env.WORKER_URL + endpoint), - reqCfg - ) - return response.json() -} - -async function getGlobalUsers(email = null) { - const endpoint = email ? `/api/admin/users/${email}` : `/api/admin/users` - const reqCfg = { method: "GET" } - const response = await fetch( - checkSlashesInUrl(env.WORKER_URL + endpoint), - reqCfg - ) - return response.json() -} - -async function saveGlobalUser(appId, email, body) { - const globalUser = await getGlobalUsers(email) - const roles = globalUser.roles || {} - if (body.roleId) { - roles.appId = body.roleId - } - const endpoint = `/api/admin/users` - const reqCfg = { - method: "POST", - body: { - ...globalUser, - email, - password: body.password, - status: body.status, - roles, - }, - } - - const response = await fetch( - checkSlashesInUrl(env.WORKER_URL + endpoint), - reqCfg - ) - await response.json() - delete body.email - delete body.password - delete body.roleId - delete body.status - return body -} +const { + getGlobalUsers, + saveGlobalUser, + deleteGlobalUser, +} = require("../../utilities/workerRequests") exports.fetchMetadata = async function(ctx) { const database = new CouchDB(ctx.appId) - const global = await getGlobalUsers() + const global = await getGlobalUsers(ctx, ctx.appId) const metadata = ( await database.allDocs( - getUserParams(null, { + getUserMetadataParams(null, { include_docs: true, }) ) @@ -90,11 +42,11 @@ exports.createMetadata = async function(ctx) { const role = await getRole(appId, roleId) if (!role) ctx.throw(400, "Invalid Role") - const metadata = await saveGlobalUser(appId, email, ctx.request.body) + const metadata = await saveGlobalUser(ctx, appId, email, ctx.request.body) const user = { ...metadata, - _id: generateUserID(email), + _id: generateUserMetadataID(email), type: "user", tableId: InternalTables.USER_METADATA, } @@ -110,11 +62,11 @@ exports.updateMetadata = async function(ctx) { const appId = ctx.appId const db = new CouchDB(appId) const user = ctx.request.body - let email = user.email || getEmailFromUserID(user._id) - const metadata = await saveGlobalUser(appId, email, ctx.request.body) + let email = user.email || getEmailFromUserMetadataID(user._id) + const metadata = await saveGlobalUser(ctx, appId, email, ctx.request.body) if (!metadata._id) { - user._id = generateUserID(email) + user._id = generateUserMetadataID(email) } ctx.body = await db.put({ ...metadata, @@ -124,8 +76,8 @@ exports.updateMetadata = async function(ctx) { exports.destroyMetadata = async function(ctx) { const db = new CouchDB(ctx.appId) const email = ctx.params.email - await deleteGlobalUser(email) - await db.destroy(generateUserID(email)) + await deleteGlobalUser(ctx, email) + await db.destroy(generateUserMetadataID(email)) ctx.body = { message: `User ${ctx.params.email} deleted.`, } @@ -133,12 +85,12 @@ exports.destroyMetadata = async function(ctx) { exports.findMetadata = async function(ctx) { const database = new CouchDB(ctx.appId) - let lookup = ctx.params.email - ? generateUserID(ctx.params.email) - : ctx.params.userId - const user = await database.get(lookup) - if (user) { - delete user.password + const email = + ctx.params.email || getEmailFromUserMetadataID(ctx.params.userId) + const global = await getGlobalUsers(ctx, ctx.appId, email) + const user = await database.get(generateUserMetadataID(email)) + ctx.body = { + ...global, + ...user, } - ctx.body = user } diff --git a/packages/server/src/db/utils.js b/packages/server/src/db/utils.js index 8623b99f2c..63d4e30d65 100644 --- a/packages/server/src/db/utils.js +++ b/packages/server/src/db/utils.js @@ -116,16 +116,18 @@ exports.getRowParams = (tableId = null, rowId = null, otherProps = {}) => { /** * Gets a new row ID for the specified table. * @param {string} tableId The table which the row is being created for. + * @param {string|null} id If an ID is to be used then the UUID can be substituted for this. * @returns {string} The new ID which a row doc can be stored under. */ -exports.generateRowID = tableId => { - return `${DocumentTypes.ROW}${SEPARATOR}${tableId}${SEPARATOR}${newid()}` +exports.generateRowID = (tableId, id = null) => { + id = id || newid() + return `${DocumentTypes.ROW}${SEPARATOR}${tableId}${SEPARATOR}${id}` } /** * Gets parameters for retrieving users, this is a utility function for the getDocParams function. */ -exports.getUserParams = (email = "", otherProps = {}) => { +exports.getUserMetadataParams = (email = "", otherProps = {}) => { return exports.getRowParams(ViewNames.USERS, email, otherProps) } @@ -134,8 +136,17 @@ exports.getUserParams = (email = "", otherProps = {}) => { * @param {string} email The email which the ID is going to be built up of. * @returns {string} The new user ID which the user doc can be stored under. */ -exports.generateUserID = email => { - return `${DocumentTypes.ROW}${SEPARATOR}${ViewNames.USERS}${SEPARATOR}${DocumentTypes.USER}${SEPARATOR}${email}` +exports.generateUserMetadataID = email => { + return exports.generateRowID(InternalTables.USER_METADATA, email) +} + +/** + * Breaks up the ID to get the email address back out of it. + */ +exports.getEmailFromUserMetadataID = id => { + return id.split( + `${DocumentTypes.ROW}${SEPARATOR}${InternalTables.USER_METADATA}${SEPARATOR}` + )[1] } /** diff --git a/packages/server/src/utilities/builder/hosting.js b/packages/server/src/utilities/builder/hosting.js index f852cefec1..328e98ee98 100644 --- a/packages/server/src/utilities/builder/hosting.js +++ b/packages/server/src/utilities/builder/hosting.js @@ -2,6 +2,7 @@ const CouchDB = require("../../db") const { StaticDatabases } = require("../../db/utils") const fetch = require("node-fetch") const env = require("../../environment") +const { getDeployedApps } = require("../../utilities/workerRequests") const PROD_HOSTING_URL = "app.budi.live" @@ -84,30 +85,4 @@ exports.getTemplatesUrl = async (appId, type, name) => { return `${protocol}${hostingInfo.templatesUrl}/${path}` } -exports.getDeployedApps = async () => { - if (!env.SELF_HOSTED) { - throw "Can only check apps for self hosted environments" - } - const workerUrl = env.WORKER_URL - const hostingKey = env.HOSTING_KEY - try { - const response = await fetch(`${workerUrl}/api/apps`, { - method: "GET", - headers: { - "x-budibase-auth": hostingKey, - }, - }) - const json = await response.json() - const apps = {} - for (let [key, value] of Object.entries(json)) { - if (value.url) { - value.url = value.url.toLowerCase() - apps[key] = value - } - } - return apps - } catch (err) { - // error, cannot determine deployed apps, don't stop app creation - sort this later - return {} - } -} +exports.getDeployedApps = getDeployedApps diff --git a/packages/server/src/utilities/workerRequests.js b/packages/server/src/utilities/workerRequests.js new file mode 100644 index 0000000000..f3bf971257 --- /dev/null +++ b/packages/server/src/utilities/workerRequests.js @@ -0,0 +1,116 @@ +const fetch = require("node-fetch") +const env = require("../environment") +const { checkSlashesInUrl } = require("./index") +const { BUILTIN_ROLE_IDS } = require("./security/roles") + +function getAppRole(appId, user) { + if (!user.roles) { + return user + } + user.roleId = user.roles[appId] + if (!user.roleId) { + user.roleId = BUILTIN_ROLE_IDS.PUBLIC + } + delete user.roles + return user +} + +function prepRequest(ctx, request) { + if (!request.headers) { + request.headers = {} + } + if (request.body) { + request.headers["Content-Type"] = "application/json" + request.body = + typeof request.body === "object" + ? JSON.stringify(request.body) + : request.body + } + request.headers.cookie = ctx.headers.cookie + return request +} + +exports.getDeployedApps = async ctx => { + if (!env.SELF_HOSTED) { + throw "Can only check apps for self hosted environments" + } + try { + const response = await fetch( + checkSlashesInUrl(env.WORKER_URL + `/api/apps`), + prepRequest(ctx, { + method: "GET", + }) + ) + const json = await response.json() + const apps = {} + for (let [key, value] of Object.entries(json)) { + if (value.url) { + value.url = value.url.toLowerCase() + apps[key] = value + } + } + return apps + } catch (err) { + // error, cannot determine deployed apps, don't stop app creation - sort this later + return {} + } +} + +exports.deleteGlobalUser = async (ctx, email) => { + const endpoint = `/api/admin/users/${email}` + const reqCfg = { method: "DELETE" } + const response = await fetch( + checkSlashesInUrl(env.WORKER_URL + endpoint), + prepRequest(ctx, reqCfg) + ) + return response.json() +} + +exports.getGlobalUsers = async (ctx, appId, email = null) => { + const endpoint = email ? `/api/admin/users/${email}` : `/api/admin/users` + const reqCfg = { method: "GET" } + const response = await fetch( + checkSlashesInUrl(env.WORKER_URL + endpoint), + prepRequest(ctx, reqCfg) + ) + let users = await response.json() + if (Array.isArray(users)) { + users = users.map(user => getAppRole(appId, user)) + } else { + users = getAppRole(appId, users) + } + return users +} + +exports.saveGlobalUser = async (ctx, appId, email, body) => { + const globalUser = await exports.getGlobalUsers(ctx, appId, email) + const roles = globalUser.roles || {} + if (body.roleId) { + roles[appId] = body.roleId + } + const endpoint = `/api/admin/users` + const reqCfg = { + method: "POST", + body: { + ...globalUser, + email, + password: body.password, + status: body.status, + roles, + }, + } + + const response = await fetch( + checkSlashesInUrl(env.WORKER_URL + endpoint), + prepRequest(ctx, reqCfg) + ) + const json = await response.json() + if (json.status !== 200 && response.status !== 200) { + ctx.throw(400, "Unable to save global user.") + } + delete body.email + delete body.password + delete body.roleId + delete body.status + return body +} diff --git a/packages/worker/src/api/controllers/admin/index.js b/packages/worker/src/api/controllers/admin/index.js index 515feb8420..305686a3b0 100644 --- a/packages/worker/src/api/controllers/admin/index.js +++ b/packages/worker/src/api/controllers/admin/index.js @@ -14,11 +14,11 @@ exports.userSave = async ctx => { const { email, password, _id } = ctx.request.body const hashedPassword = password ? await hash(password) : null let user = { - ...ctx.request.body, - _id: generateUserID(email), - password: hashedPassword, - }, - dbUser + ...ctx.request.body, + _id: generateUserID(email), + password: hashedPassword, + } + let dbUser // in-case user existed already if (_id) { dbUser = await db.get(_id) @@ -57,13 +57,12 @@ exports.userDelete = async ctx => { // called internally by app server user fetch exports.userFetch = async ctx => { const db = new CouchDB(USER_DB) - const users = ( - await db.allDocs( - getUserParams(null, { - include_docs: true, - }) - ) - ).rows.map(row => row.doc) + const response = await db.allDocs( + getUserParams(null, { + include_docs: true, + }) + ) + const users = response.rows.map(row => row.doc) // user hashed password shouldn't ever be returned for (let user of users) { if (user) { @@ -76,7 +75,13 @@ exports.userFetch = async ctx => { // called internally by app server user find exports.userFind = async ctx => { const db = new CouchDB(USER_DB) - const user = await db.get(generateUserID(ctx.params.email)) + let user + try { + user = await db.get(generateUserID(ctx.params.email)) + } catch (err) { + // no user found, just return nothing + user = {} + } if (user) { delete user.password } diff --git a/packages/worker/src/api/routes/app.js b/packages/worker/src/api/routes/app.js index 10656e5362..75fa7164b0 100644 --- a/packages/worker/src/api/routes/app.js +++ b/packages/worker/src/api/routes/app.js @@ -1,9 +1,9 @@ const Router = require("@koa/router") const controller = require("../controllers/app") -const checkKey = require("../../middleware/check-key") +const authenticated = require("../../middleware/authenticated") const router = Router() -router.get("/api/apps", checkKey, controller.getApps) +router.get("/api/apps", authenticated, controller.getApps) module.exports = router diff --git a/packages/worker/src/middleware/authenticated.js b/packages/worker/src/middleware/authenticated.js index 751e10ee9a..b13ff5c1b7 100644 --- a/packages/worker/src/middleware/authenticated.js +++ b/packages/worker/src/middleware/authenticated.js @@ -11,20 +11,22 @@ module.exports = async (ctx, next) => { appId = cookieAppId } - return passport.authenticate("jwt", async (err, user) => { - if (err) { - return ctx.throw(err.status || 403, err) - } + return next() - try { - ctx.appId = appId - ctx.isAuthenticated = true - // TODO: introduce roles again - ctx.user = user - await next() - } catch (err) { - console.log(err) - ctx.throw(err.status || 403, err.text) - } - })(ctx, next) + // return passport.authenticate("jwt", async (err, user) => { + // if (err) { + // return ctx.throw(err.status || 403, err) + // } + // + // try { + // ctx.appId = appId + // ctx.isAuthenticated = true + // // TODO: introduce roles again + // ctx.user = user + // await next() + // } catch (err) { + // console.log(err) + // ctx.throw(err.status || 403, err.text) + // } + // })(ctx, next) } From e34894dd92f06942402ed993485aa0685da6f4dd Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 9 Apr 2021 16:55:56 +0100 Subject: [PATCH 2/4] Global user management now functioning as expected, there were some errant db.destroy functions from the system previously, this is now cleaned up. --- packages/server/src/api/controllers/row.js | 71 ++++++++++--------- packages/server/src/api/controllers/user.js | 14 +++- .../worker/src/api/controllers/admin/index.js | 3 +- packages/worker/src/api/controllers/app.js | 7 +- 4 files changed, 59 insertions(+), 36 deletions(-) diff --git a/packages/server/src/api/controllers/row.js b/packages/server/src/api/controllers/row.js index 8b4a461d2c..1e32c24c12 100644 --- a/packages/server/src/api/controllers/row.js +++ b/packages/server/src/api/controllers/row.js @@ -9,7 +9,7 @@ const { InternalTables, generateUserMetadataID, } = require("../../db/utils") -const usersController = require("./user") +const userController = require("./user") const { inputProcessing, outputProcessing, @@ -37,18 +37,14 @@ validateJs.extend(validateJs.validators.datetime, { }, }) -async function findRow(db, appId, tableId, rowId) { +async function findRow(ctx, db, tableId, rowId) { let row + // TODO remove special user case in future if (tableId === InternalTables.USER_METADATA) { - let ctx = { - params: { - userId: rowId, - }, - user: { - appId, - }, + ctx.params = { + userId: rowId, } - await usersController.findMetadata(ctx) + await userController.findMetadata(ctx) row = ctx.body } else { row = await db.get(rowId) @@ -96,14 +92,14 @@ exports.patch = async function(ctx) { table, }) - // Creation of a new user goes to the user controller + // TODO remove special user case in future if (row.tableId === InternalTables.USER_METADATA) { // the row has been updated, need to put it into the ctx ctx.request.body = { ...row, password: ctx.request.body.password, } - await usersController.updateMetadata(ctx) + await userController.updateMetadata(ctx) return } @@ -142,6 +138,7 @@ exports.save = async function(ctx) { } if (!inputs._rev && !inputs._id) { + // TODO remove special user case in future if (inputs.tableId === InternalTables.USER_METADATA) { inputs._id = generateUserMetadataID(inputs.email) } else { @@ -175,11 +172,11 @@ exports.save = async function(ctx) { table, }) - // Creation of a new user goes to the user controller + // TODO remove special user case in future if (row.tableId === InternalTables.USER_METADATA) { // the row has been updated, need to put it into the ctx ctx.request.body = row - await usersController.createMetadata(ctx) + await userController.createMetadata(ctx) return } @@ -287,14 +284,6 @@ exports.search = async function(ctx) { } const response = await search(searchString) - - // delete passwords from users - if (tableId === InternalTables.USER_METADATA) { - for (let row of response.rows) { - delete row.password - } - } - const table = await db.get(tableId) ctx.body = { rows: await outputProcessing(appId, table, response.rows), @@ -306,11 +295,11 @@ exports.fetchTableRows = async function(ctx) { const appId = ctx.appId const db = new CouchDB(appId) - // special case for users, fetch through the user controller + // TODO remove special user case in future let rows, table = await db.get(ctx.params.tableId) if (ctx.params.tableId === InternalTables.USER_METADATA) { - await usersController.fetchMetadata(ctx) + await userController.fetchMetadata(ctx) rows = ctx.body } else { const response = await db.allDocs( @@ -328,7 +317,7 @@ exports.find = async function(ctx) { const db = new CouchDB(appId) try { const table = await db.get(ctx.params.tableId) - const row = await findRow(db, appId, ctx.params.tableId, ctx.params.rowId) + const row = await findRow(ctx, db, ctx.params.tableId, ctx.params.rowId) ctx.body = await outputProcessing(appId, table, row) } catch (err) { ctx.throw(400, err) @@ -348,8 +337,15 @@ exports.destroy = async function(ctx) { row, tableId: row.tableId, }) - ctx.body = await db.remove(ctx.params.rowId, ctx.params.revId) - ctx.status = 200 + // TODO remove special user case in future + if (ctx.params.tableId === InternalTables.USER_METADATA) { + ctx.params = { + userId: ctx.params.rowId, + } + await userController.destroyMetadata(ctx) + } else { + ctx.body = await db.remove(ctx.params.rowId, ctx.params.revId) + } // for automations include the row that was deleted ctx.row = row @@ -395,7 +391,7 @@ exports.fetchEnrichedRow = async function(ctx) { // need table to work out where links go in row let [table, row] = await Promise.all([ db.get(tableId), - findRow(db, appId, tableId, rowId), + findRow(ctx, db, tableId, rowId), ]) // get the link docs const linkVals = await linkRows.getLinkDocuments({ @@ -437,7 +433,7 @@ async function bulkDelete(ctx) { const { rows } = ctx.request.body const db = new CouchDB(appId) - const linkUpdates = rows.map(row => + let updates = rows.map(row => linkRows.updateLinks({ appId, eventType: linkRows.EventType.ROW_DELETE, @@ -445,9 +441,20 @@ async function bulkDelete(ctx) { tableId: row.tableId, }) ) - - await db.bulkDocs(rows.map(row => ({ ...row, _deleted: true }))) - await Promise.all(linkUpdates) + // TODO remove special user case in future + if (ctx.params.tableId === InternalTables.USER_METADATA) { + updates = updates.concat( + rows.map(row => { + ctx.params = { + userId: row._id, + } + return userController.destroyMetadata(ctx) + }) + ) + } else { + await db.bulkDocs(rows.map(row => ({ ...row, _deleted: true }))) + } + await Promise.all(updates) rows.forEach(row => { ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row) diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index c526050cde..93e6dec1d6 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -28,6 +28,8 @@ exports.fetchMetadata = async function(ctx) { users.push({ ...user, ...info, + // make sure the ID is always a local ID, not a global one + _id: generateUserMetadataID(user.email), }) } ctx.body = users @@ -75,9 +77,15 @@ exports.updateMetadata = async function(ctx) { exports.destroyMetadata = async function(ctx) { const db = new CouchDB(ctx.appId) - const email = ctx.params.email + const email = + ctx.params.email || getEmailFromUserMetadataID(ctx.params.userId) await deleteGlobalUser(ctx, email) - await db.destroy(generateUserMetadataID(email)) + try { + const dbUser = await db.get(generateUserMetadataID(email)) + await db.remove(dbUser._id, dbUser._rev) + } catch (err) { + // error just means the global user has no config in this app + } ctx.body = { message: `User ${ctx.params.email} deleted.`, } @@ -92,5 +100,7 @@ exports.findMetadata = async function(ctx) { ctx.body = { ...global, ...user, + // make sure the ID is always a local ID, not a global one + _id: generateUserMetadataID(email), } } diff --git a/packages/worker/src/api/controllers/admin/index.js b/packages/worker/src/api/controllers/admin/index.js index 305686a3b0..ff0d2997e7 100644 --- a/packages/worker/src/api/controllers/admin/index.js +++ b/packages/worker/src/api/controllers/admin/index.js @@ -48,7 +48,8 @@ exports.userSave = async ctx => { exports.userDelete = async ctx => { const db = new CouchDB(USER_DB) - await db.destroy(generateUserID(ctx.params.email)) + const dbUser = await db.get(generateUserID(ctx.params.email)) + await db.remove(dbUser._id, dbUser._rev) ctx.body = { message: `User ${ctx.params.email} deleted.`, } diff --git a/packages/worker/src/api/controllers/app.js b/packages/worker/src/api/controllers/app.js index eac0c47c18..bed3b55942 100644 --- a/packages/worker/src/api/controllers/app.js +++ b/packages/worker/src/api/controllers/app.js @@ -15,9 +15,14 @@ exports.getApps = async ctx => { } const appDbNames = allDbs.filter(dbName => dbName.startsWith(APP_PREFIX)) const appPromises = appDbNames.map(db => new CouchDB(db).get(db)) - const apps = await Promise.all(appPromises) + + const apps = await Promise.allSettled(appPromises) const body = {} for (let app of apps) { + if (app.status !== "fulfilled") { + continue + } + app = app.value let url = app.url || encodeURI(`${app.name}`) url = `/${url.replace(URL_REGEX_SLASH, "")}` body[url] = { From e275553f6001d074f58085b8ff7f03f8b118e666 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 9 Apr 2021 16:56:42 +0100 Subject: [PATCH 3/4] Formatting. --- packages/auth/src/index.js | 6 +++++- packages/worker/src/api/routes/admin/index.js | 7 ++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index b65690b064..f42b2d8423 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -8,7 +8,11 @@ const { jwt, local, google } = require("./middleware") const { Cookies, UserStatus } = require("./constants") const { hash, compare } = require("./hashing") const { getAppId, setCookie } = require("./utils") -const { generateUserID, getUserParams, getEmailFromUserID } = require("./db/utils") +const { + generateUserID, + getUserParams, + getEmailFromUserID, +} = require("./db/utils") // Strategies passport.use(new LocalStrategy(local.options, local.authenticate)) diff --git a/packages/worker/src/api/routes/admin/index.js b/packages/worker/src/api/routes/admin/index.js index 558c6e2c29..6a89a41dde 100644 --- a/packages/worker/src/api/routes/admin/index.js +++ b/packages/worker/src/api/routes/admin/index.js @@ -22,7 +22,12 @@ function buildUserSaveValidation() { } router - .post("/api/admin/users", buildUserSaveValidation(), authenticated, controller.userSave) + .post( + "/api/admin/users", + buildUserSaveValidation(), + authenticated, + controller.userSave + ) .delete("/api/admin/users/:email", authenticated, controller.userDelete) .get("/api/admin/users", authenticated, controller.userFetch) .get("/api/admin/users/:email", authenticated, controller.userFind) From 4f71e11c94dff435ed20beab4ad8ecee21f8ba80 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 9 Apr 2021 17:33:21 +0100 Subject: [PATCH 4/4] Updating some test cases to work with new system. --- packages/server/__mocks__/node-fetch.js | 6 ++++++ packages/server/src/api/controllers/auth.js | 4 ++-- packages/server/src/api/controllers/role.js | 4 ++-- packages/server/src/api/controllers/row.js | 1 + packages/server/src/api/controllers/table/utils.js | 8 ++++++-- packages/server/src/api/controllers/user.js | 3 +++ packages/server/src/automations/steps/createRow.js | 2 +- packages/server/src/automations/steps/createUser.js | 6 ++---- packages/server/src/automations/steps/deleteRow.js | 2 +- packages/server/src/automations/steps/updateRow.js | 2 +- packages/server/src/automations/tests/automation.spec.js | 2 +- packages/server/src/automations/tests/createRow.spec.js | 1 + packages/server/src/automations/tests/createUser.spec.js | 6 ++---- packages/server/src/db/utils.js | 2 +- packages/server/src/middleware/tests/usageQuota.spec.js | 4 +--- packages/server/src/utilities/workerRequests.js | 4 +++- 16 files changed, 34 insertions(+), 23 deletions(-) diff --git a/packages/server/__mocks__/node-fetch.js b/packages/server/__mocks__/node-fetch.js index d023802582..33b6e23454 100644 --- a/packages/server/__mocks__/node-fetch.js +++ b/packages/server/__mocks__/node-fetch.js @@ -41,6 +41,12 @@ module.exports = async (url, opts) => { ], bookmark: "test", }) + } else if (url.includes("/api/admin")) { + return json({ + email: "test@test.com", + _id: "us_test@test.com", + status: "active", + }) } return fetch(url, opts) } diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index 6e662f7b9a..3c61142d66 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/packages/server/src/api/controllers/auth.js @@ -6,7 +6,7 @@ const { getAPIKey } = require("../../utilities/usageQuota") const { generateUserMetadataID } = require("../../db/utils") const { setCookie } = require("../../utilities") const { outputProcessing } = require("../../utilities/rowProcessor") -const { ViewNames } = require("../../db/utils") +const { InternalTables } = require("../../db/utils") const { UserStatus } = require("@budibase/auth") const setBuilderToken = require("../../utilities/builder/setBuilderToken") @@ -84,7 +84,7 @@ exports.fetchSelf = async ctx => { } const db = new CouchDB(appId) const user = await db.get(userId) - const userTable = await db.get(ViewNames.USERS) + const userTable = await db.get(InternalTables.USER_METADATA) if (user) { delete user.password } diff --git a/packages/server/src/api/controllers/role.js b/packages/server/src/api/controllers/role.js index d27272a21a..42213b010d 100644 --- a/packages/server/src/api/controllers/role.js +++ b/packages/server/src/api/controllers/role.js @@ -11,7 +11,7 @@ const { generateRoleID, getRoleParams, getUserMetadataParams, - ViewNames, + InternalTables, } = require("../../db/utils") const UpdateRolesOptions = { @@ -28,7 +28,7 @@ const EXTERNAL_BUILTIN_ROLE_IDS = [ ] async function updateRolesOnUserTable(db, roleId, updateOption) { - const table = await db.get(ViewNames.USERS) + const table = await db.get(InternalTables.USER_METADATA) const schema = table.schema const remove = updateOption === UpdateRolesOptions.REMOVED let updated = false diff --git a/packages/server/src/api/controllers/row.js b/packages/server/src/api/controllers/row.js index 1e32c24c12..9f59eb46f3 100644 --- a/packages/server/src/api/controllers/row.js +++ b/packages/server/src/api/controllers/row.js @@ -349,6 +349,7 @@ exports.destroy = async function(ctx) { // for automations include the row that was deleted ctx.row = row + ctx.status = 200 ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row) } diff --git a/packages/server/src/api/controllers/table/utils.js b/packages/server/src/api/controllers/table/utils.js index 0302ca18a3..3cf64e8e31 100644 --- a/packages/server/src/api/controllers/table/utils.js +++ b/packages/server/src/api/controllers/table/utils.js @@ -1,6 +1,10 @@ const CouchDB = require("../../../db") const csvParser = require("../../../utilities/csvParser") -const { getRowParams, generateRowID, ViewNames } = require("../../../db/utils") +const { + getRowParams, + generateRowID, + InternalTables, +} = require("../../../db/utils") const { isEqual } = require("lodash/fp") const { AutoFieldSubTypes } = require("../../../constants") const { inputProcessing } = require("../../../utilities/rowProcessor") @@ -136,7 +140,7 @@ exports.handleSearchIndexes = async (appId, table) => { exports.checkStaticTables = table => { // check user schema has all required elements - if (table._id === ViewNames.USERS) { + if (table._id === InternalTables.USER_METADATA) { for (let [key, schema] of Object.entries(USERS_TABLE_SCHEMA.schema)) { // check if the schema exists on the table to be created/updated if (table.schema[key] == null) { diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index 93e6dec1d6..2ef8753845 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -54,7 +54,10 @@ exports.createMetadata = async function(ctx) { } const response = await db.post(user) + // for automations to make it obvious was successful + ctx.status = 200 ctx.body = { + _id: response.id, _rev: response.rev, email, } diff --git a/packages/server/src/automations/steps/createRow.js b/packages/server/src/automations/steps/createRow.js index aa910dbb42..9ab70d3161 100644 --- a/packages/server/src/automations/steps/createRow.js +++ b/packages/server/src/automations/steps/createRow.js @@ -75,7 +75,7 @@ module.exports.run = async function({ inputs, appId, apiKey, emitter }) { request: { body: inputs.row, }, - user: { appId }, + appId, eventEmitter: emitter, } diff --git a/packages/server/src/automations/steps/createUser.js b/packages/server/src/automations/steps/createUser.js index 8849415c5a..21f02eedcc 100644 --- a/packages/server/src/automations/steps/createUser.js +++ b/packages/server/src/automations/steps/createUser.js @@ -62,9 +62,7 @@ module.exports.definition = { module.exports.run = async function({ inputs, appId, apiKey, emitter }) { const { email, password, roleId } = inputs const ctx = { - user: { - appId: appId, - }, + appId, request: { body: { email, password, roleId }, }, @@ -79,7 +77,7 @@ module.exports.run = async function({ inputs, appId, apiKey, emitter }) { return { response: ctx.body, // internal property not returned through the API - id: ctx.userId, + id: ctx.body._id, revision: ctx.body._rev, success: ctx.status === 200, } diff --git a/packages/server/src/automations/steps/deleteRow.js b/packages/server/src/automations/steps/deleteRow.js index 57555ddaad..4d6fbb2a70 100644 --- a/packages/server/src/automations/steps/deleteRow.js +++ b/packages/server/src/automations/steps/deleteRow.js @@ -65,7 +65,7 @@ module.exports.run = async function({ inputs, appId, apiKey, emitter }) { rowId: inputs.id, revId: inputs.revision, }, - user: { appId }, + appId, eventEmitter: emitter, } diff --git a/packages/server/src/automations/steps/updateRow.js b/packages/server/src/automations/steps/updateRow.js index a545662cf8..78c11a4212 100644 --- a/packages/server/src/automations/steps/updateRow.js +++ b/packages/server/src/automations/steps/updateRow.js @@ -78,7 +78,7 @@ module.exports.run = async function({ inputs, appId, emitter }) { request: { body: inputs.row, }, - user: { appId }, + appId, eventEmitter: emitter, } diff --git a/packages/server/src/automations/tests/automation.spec.js b/packages/server/src/automations/tests/automation.spec.js index 2e9bb16e55..7a01b64dc6 100644 --- a/packages/server/src/automations/tests/automation.spec.js +++ b/packages/server/src/automations/tests/automation.spec.js @@ -1,10 +1,10 @@ +require("../../environment") const automation = require("../index") const usageQuota = require("../../utilities/usageQuota") const thread = require("../thread") const triggers = require("../triggers") const { basicAutomation, basicTable } = require("../../tests/utilities/structures") const { wait } = require("../../utilities") -const env = require("../../environment") const { makePartial } = require("../../tests/utilities") const { cleanInputValues } = require("../automationUtils") const setup = require("./utilities") diff --git a/packages/server/src/automations/tests/createRow.spec.js b/packages/server/src/automations/tests/createRow.spec.js index c01d630bed..d31ba5f8d2 100644 --- a/packages/server/src/automations/tests/createRow.spec.js +++ b/packages/server/src/automations/tests/createRow.spec.js @@ -26,6 +26,7 @@ describe("test the create row action", () => { }) expect(res.id).toBeDefined() expect(res.revision).toBeDefined() + expect(res.success).toEqual(true) const gottenRow = await config.getRow(table._id, res.id) expect(gottenRow.name).toEqual("test") expect(gottenRow.description).toEqual("test") diff --git a/packages/server/src/automations/tests/createUser.spec.js b/packages/server/src/automations/tests/createUser.spec.js index f188c31aa4..f085d52712 100644 --- a/packages/server/src/automations/tests/createUser.spec.js +++ b/packages/server/src/automations/tests/createUser.spec.js @@ -1,8 +1,7 @@ const usageQuota = require("../../utilities/usageQuota") -const env = require("../../environment") const setup = require("./utilities") const { BUILTIN_ROLE_IDS } = require("../../utilities/security/roles") -const { ViewNames } = require("../../db/utils") +const { InternalTables } = require("../../db/utils") jest.mock("../../utilities/usageQuota") @@ -25,8 +24,7 @@ describe("test the create user action", () => { const res = await setup.runStep(setup.actions.CREATE_USER.stepId, user) expect(res.id).toBeDefined() expect(res.revision).toBeDefined() - const userDoc = await config.getRow(ViewNames.USERS, res.id) - expect(userDoc.email).toEqual(user.email) + const userDoc = await config.getRow(InternalTables.USER_METADATA, res.id) }) it("should return an error if no inputs provided", async () => { diff --git a/packages/server/src/db/utils.js b/packages/server/src/db/utils.js index 63d4e30d65..5cd9e1b31f 100644 --- a/packages/server/src/db/utils.js +++ b/packages/server/src/db/utils.js @@ -128,7 +128,7 @@ exports.generateRowID = (tableId, id = null) => { * Gets parameters for retrieving users, this is a utility function for the getDocParams function. */ exports.getUserMetadataParams = (email = "", otherProps = {}) => { - return exports.getRowParams(ViewNames.USERS, email, otherProps) + return exports.getRowParams(InternalTables.USER_METADATA, email, otherProps) } /** diff --git a/packages/server/src/middleware/tests/usageQuota.spec.js b/packages/server/src/middleware/tests/usageQuota.spec.js index 9ab17ef992..646f492329 100644 --- a/packages/server/src/middleware/tests/usageQuota.spec.js +++ b/packages/server/src/middleware/tests/usageQuota.spec.js @@ -20,9 +20,7 @@ class TestConfiguration { this.ctx = { throw: this.throw, next: this.next, - user: { - appId: "test" - }, + appId: "test", request: { body: {} }, diff --git a/packages/server/src/utilities/workerRequests.js b/packages/server/src/utilities/workerRequests.js index f3bf971257..b081762e13 100644 --- a/packages/server/src/utilities/workerRequests.js +++ b/packages/server/src/utilities/workerRequests.js @@ -26,7 +26,9 @@ function prepRequest(ctx, request) { ? JSON.stringify(request.body) : request.body } - request.headers.cookie = ctx.headers.cookie + if (ctx.headers) { + request.headers.cookie = ctx.headers.cookie + } return request }