From d23af4dec1ef80a9b29896a59ee4330d3f3fb4b1 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 4 Aug 2022 17:14:52 +0100 Subject: [PATCH 1/2] Allow builders (not just admins) to delete apps. --- packages/worker/src/api/routes/global/roles.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/worker/src/api/routes/global/roles.js b/packages/worker/src/api/routes/global/roles.js index 4e27b7d54b..dff4c6a068 100644 --- a/packages/worker/src/api/routes/global/roles.js +++ b/packages/worker/src/api/routes/global/roles.js @@ -1,12 +1,12 @@ const Router = require("@koa/router") const controller = require("../../controllers/global/roles") -const { adminOnly } = require("@budibase/backend-core/auth") +const builderOrAdmin = require("../../../middleware/builderOrAdmin") const router = Router() router - .get("/api/global/roles", adminOnly, controller.fetch) - .get("/api/global/roles/:appId", adminOnly, controller.find) - .delete("/api/global/roles/:appId", adminOnly, controller.removeAppRole) + .get("/api/global/roles", builderOrAdmin, controller.fetch) + .get("/api/global/roles/:appId", builderOrAdmin, controller.find) + .delete("/api/global/roles/:appId", builderOrAdmin, controller.removeAppRole) module.exports = router From f3418c4107158aa9f52092e70f9afeb74f383f76 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 4 Aug 2022 19:03:50 +0100 Subject: [PATCH 2/2] Some more logging, moving middlewares to backend-core. --- packages/backend-core/src/auth.js | 4 +++ .../src/middleware/authenticated.js | 2 +- .../src/middleware/builderOnly.js | 0 .../src/middleware/builderOrAdmin.js | 0 packages/backend-core/src/middleware/index.js | 4 +++ .../backend-core/src/security/sessions.js | 25 ++++++++++------- .../worker/src/api/routes/global/roles.js | 2 +- packages/worker/src/api/routes/global/self.js | 2 +- .../worker/src/api/routes/global/users.js | 2 +- .../worker/src/api/routes/validation/users.ts | 2 +- packages/worker/src/middleware/adminOnly.js | 9 ------ .../worker/src/middleware/joi-validator.js | 28 ------------------- 12 files changed, 28 insertions(+), 52 deletions(-) rename packages/{worker => backend-core}/src/middleware/builderOnly.js (100%) rename packages/{worker => backend-core}/src/middleware/builderOrAdmin.js (100%) delete mode 100644 packages/worker/src/middleware/adminOnly.js delete mode 100644 packages/worker/src/middleware/joi-validator.js diff --git a/packages/backend-core/src/auth.js b/packages/backend-core/src/auth.js index 9ae29a3cbd..d39b8426fb 100644 --- a/packages/backend-core/src/auth.js +++ b/packages/backend-core/src/auth.js @@ -19,6 +19,8 @@ const { csrf, internalApi, adminOnly, + builderOnly, + builderOrAdmin, joiValidator, } = require("./middleware") @@ -176,5 +178,7 @@ module.exports = { updateUserOAuth, ssoCallbackUrl, adminOnly, + builderOnly, + builderOrAdmin, joiValidator, } diff --git a/packages/backend-core/src/middleware/authenticated.js b/packages/backend-core/src/middleware/authenticated.js index d86af773c3..674c16aa55 100644 --- a/packages/backend-core/src/middleware/authenticated.js +++ b/packages/backend-core/src/middleware/authenticated.js @@ -81,7 +81,7 @@ module.exports = ( const session = await getSession(userId, sessionId) if (!session) { - error = "No session found" + error = `Session not found - ${userId} - ${sessionId}` } else { try { if (opts && opts.populateUser) { diff --git a/packages/worker/src/middleware/builderOnly.js b/packages/backend-core/src/middleware/builderOnly.js similarity index 100% rename from packages/worker/src/middleware/builderOnly.js rename to packages/backend-core/src/middleware/builderOnly.js diff --git a/packages/worker/src/middleware/builderOrAdmin.js b/packages/backend-core/src/middleware/builderOrAdmin.js similarity index 100% rename from packages/worker/src/middleware/builderOrAdmin.js rename to packages/backend-core/src/middleware/builderOrAdmin.js diff --git a/packages/backend-core/src/middleware/index.js b/packages/backend-core/src/middleware/index.js index 9d94bf5763..7e7b8a2931 100644 --- a/packages/backend-core/src/middleware/index.js +++ b/packages/backend-core/src/middleware/index.js @@ -10,6 +10,8 @@ const internalApi = require("./internalApi") const datasourceGoogle = require("./passport/datasource/google") const csrf = require("./csrf") const adminOnly = require("./adminOnly") +const builderOrAdmin = require("./builderOrAdmin") +const builderOnly = require("./builderOnly") const joiValidator = require("./joi-validator") module.exports = { google, @@ -27,5 +29,7 @@ module.exports = { }, csrf, adminOnly, + builderOnly, + builderOrAdmin, joiValidator, } diff --git a/packages/backend-core/src/security/sessions.js b/packages/backend-core/src/security/sessions.js index 2ac6eefb24..a3be0a1a58 100644 --- a/packages/backend-core/src/security/sessions.js +++ b/packages/backend-core/src/security/sessions.js @@ -1,6 +1,7 @@ const redis = require("../redis/init") const { v4: uuidv4 } = require("uuid") const { logWarn } = require("../logging") +const env = require("../environment") // a week in seconds const EXPIRY_SECONDS = 86400 * 7 @@ -34,17 +35,21 @@ async function invalidateSessions(userId, sessionIds = null) { })) } - const client = await redis.getSessionClient() - const promises = [] - for (let session of sessions) { - promises.push(client.delete(session.key)) + if (sessions && sessions.length > 0) { + const client = await redis.getSessionClient() + const promises = [] + for (let session of sessions) { + promises.push(client.delete(session.key)) + } + if (!env.isTest()) { + logWarn( + `Invalidating sessions for ${userId} - ${sessions + .map(session => session.key) + .join(", ")}` + ) + } + await Promise.all(promises) } - logWarn( - `Invalidating sessions for ${userId} - ${sessions - .map(session => session.key) - .join(", ")}` - ) - await Promise.all(promises) } catch (err) { console.error(`Error invalidating sessions: ${err}`) } diff --git a/packages/worker/src/api/routes/global/roles.js b/packages/worker/src/api/routes/global/roles.js index dff4c6a068..d99e0e5b56 100644 --- a/packages/worker/src/api/routes/global/roles.js +++ b/packages/worker/src/api/routes/global/roles.js @@ -1,6 +1,6 @@ const Router = require("@koa/router") const controller = require("../../controllers/global/roles") -const builderOrAdmin = require("../../../middleware/builderOrAdmin") +const { builderOrAdmin } = require("@budibase/backend-core/auth") const router = Router() diff --git a/packages/worker/src/api/routes/global/self.js b/packages/worker/src/api/routes/global/self.js index e1af7c2146..1683a94f37 100644 --- a/packages/worker/src/api/routes/global/self.js +++ b/packages/worker/src/api/routes/global/self.js @@ -1,6 +1,6 @@ const Router = require("@koa/router") const controller = require("../../controllers/global/self") -const builderOnly = require("../../../middleware/builderOnly") +const { builderOnly } = require("@budibase/backend-core/auth") const { users } = require("../validation") const router = Router() diff --git a/packages/worker/src/api/routes/global/users.js b/packages/worker/src/api/routes/global/users.js index 0fc479df39..e0a221a795 100644 --- a/packages/worker/src/api/routes/global/users.js +++ b/packages/worker/src/api/routes/global/users.js @@ -6,7 +6,7 @@ const Joi = require("joi") const cloudRestricted = require("../../../middleware/cloudRestricted") const { users } = require("../validation") const selfController = require("../../controllers/global/self") -const builderOrAdmin = require("../../../middleware/builderOrAdmin") +const { builderOrAdmin } = require("@budibase/backend-core/auth") const router = Router() diff --git a/packages/worker/src/api/routes/validation/users.ts b/packages/worker/src/api/routes/validation/users.ts index e7ad4cca18..d84ae94ee6 100644 --- a/packages/worker/src/api/routes/validation/users.ts +++ b/packages/worker/src/api/routes/validation/users.ts @@ -1,4 +1,4 @@ -import joiValidator from "../../../middleware/joi-validator" +const { joiValidator } = require("@budibase/backend-core/auth") import Joi from "joi" let schema: any = { diff --git a/packages/worker/src/middleware/adminOnly.js b/packages/worker/src/middleware/adminOnly.js deleted file mode 100644 index 4bfdf83848..0000000000 --- a/packages/worker/src/middleware/adminOnly.js +++ /dev/null @@ -1,9 +0,0 @@ -module.exports = async (ctx, next) => { - if ( - !ctx.internal && - (!ctx.user || !ctx.user.admin || !ctx.user.admin.global) - ) { - ctx.throw(403, "Admin user only endpoint.") - } - return next() -} diff --git a/packages/worker/src/middleware/joi-validator.js b/packages/worker/src/middleware/joi-validator.js deleted file mode 100644 index 1686b0e727..0000000000 --- a/packages/worker/src/middleware/joi-validator.js +++ /dev/null @@ -1,28 +0,0 @@ -function validate(schema, property) { - // Return a Koa middleware function - return (ctx, next) => { - if (!schema) { - return next() - } - let params = null - if (ctx[property] != null) { - params = ctx[property] - } else if (ctx.request[property] != null) { - params = ctx.request[property] - } - const { error } = schema.validate(params) - if (error) { - ctx.throw(400, `Invalid ${property} - ${error.message}`) - return - } - return next() - } -} - -module.exports.body = schema => { - return validate(schema, "body") -} - -module.exports.params = schema => { - return validate(schema, "params") -}