diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index 5db80a216b..24ff11d152 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -6,6 +6,7 @@ const { addDbPrefix, removeDbPrefix, getRedisOptions } = require("./utils") const CLUSTERED = false // for testing just generate the client once +let CONNECTED = false let CLIENT = env.isTest() ? new Redis(getRedisOptions()) : null /** @@ -20,9 +21,8 @@ function init() { return resolve(CLIENT) } // if a connection existed, close it and re-create it - if (CLIENT) { - CLIENT.disconnect() - CLIENT = null + if (CLIENT && CONNECTED) { + return CLIENT } const { opts, host, port } = getRedisOptions(CLUSTERED) if (CLUSTERED) { @@ -32,12 +32,15 @@ function init() { } CLIENT.on("end", err => { reject(err) + CONNECTED = false }) CLIENT.on("error", err => { reject(err) + CONNECTED = false }) CLIENT.on("connect", () => { resolve(CLIENT) + CONNECTED = true }) }) } diff --git a/packages/auth/src/redis/utils.js b/packages/auth/src/redis/utils.js index efdd2aa48d..23702353d8 100644 --- a/packages/auth/src/redis/utils.js +++ b/packages/auth/src/redis/utils.js @@ -10,6 +10,7 @@ exports.Databases = { PW_RESETS: "pwReset", INVITATIONS: "invitation", DEV_LOCKS: "devLocks", + DEBOUNCE: "debounce", } exports.getRedisOptions = (clustered = false) => { diff --git a/packages/server/src/api/controllers/application.js b/packages/server/src/api/controllers/application.js index 386c0f1d7a..69b4d8ea54 100644 --- a/packages/server/src/api/controllers/application.js +++ b/packages/server/src/api/controllers/application.js @@ -214,6 +214,9 @@ exports.update = async function (ctx) { const data = ctx.request.body const newData = { ...application, ...data, url } + if (ctx.request.body._rev !== application._rev) { + newData._rev = application._rev + } // the locked by property is attached by server but generated from // Redis, shouldn't ever store it diff --git a/packages/server/src/api/routes/tests/application.spec.js b/packages/server/src/api/routes/tests/application.spec.js index 9783079124..692a5b8e84 100644 --- a/packages/server/src/api/routes/tests/application.spec.js +++ b/packages/server/src/api/routes/tests/application.spec.js @@ -6,7 +6,12 @@ jest.mock("../../../utilities/redis", () => ({ getAllLocks: () => { return [] }, + doesUserHaveLock: () => { + return true + }, updateLock: jest.fn(), + setDebounce: jest.fn(), + checkDebounce: jest.fn(), })) describe("/applications", () => { @@ -104,4 +109,27 @@ describe("/applications", () => { expect(res.body.rev).toBeDefined() }) }) + + describe("edited at", () => { + it("middleware should set edited at", async () => { + const headers = config.defaultHeaders() + headers["referer"] = `/${config.getAppId()}/test` + const res = await request + .put(`/api/applications/${config.getAppId()}`) + .send({ + name: "UPDATED_NAME", + }) + .set(headers) + .expect('Content-Type', /json/) + .expect(200) + expect(res.body.rev).toBeDefined() + // retrieve the app to check it + const getRes = await request + .get(`/api/applications/${config.getAppId()}/appPackage`) + .set(headers) + .expect('Content-Type', /json/) + .expect(200) + expect(getRes.body.application.updatedAt).toBeDefined() + }) + }) }) diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js index b22fe245d5..78cd9c1db1 100644 --- a/packages/server/src/middleware/authorized.js +++ b/packages/server/src/middleware/authorized.js @@ -4,8 +4,7 @@ const { doesHaveResourcePermission, doesHaveBasePermission, } = require("@budibase/auth/permissions") -const { APP_DEV_PREFIX } = require("../db/utils") -const { doesUserHaveLock, updateLock } = require("../utilities/redis") +const builderMiddleware = require("./builder") function hasResource(ctx) { return ctx.resourceId != null @@ -15,25 +14,7 @@ const WEBHOOK_ENDPOINTS = new RegExp( ["webhooks/trigger", "webhooks/schema"].join("|") ) -async function checkDevAppLocks(ctx) { - const appId = ctx.appId - // if any public usage, don't proceed - if (!ctx.user._id && !ctx.user.userId) { - return - } - - // not a development app, don't need to do anything - if (!appId || !appId.startsWith(APP_DEV_PREFIX)) { - return - } - if (!(await doesUserHaveLock(appId, ctx.user))) { - ctx.throw(403, "User does not hold app lock.") - } - - // they do have lock, update it - await updateLock(appId, ctx.user) -} module.exports = (permType, permLevel = null) => async (ctx, next) => { // webhooks don't need authentication, each webhook unique @@ -45,13 +26,9 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { return ctx.throw(403, "No user info found") } - const builderCall = permType === PermissionTypes.BUILDER - const referer = ctx.headers["referer"] - const editingApp = referer ? referer.includes(ctx.appId) : false - // this makes sure that builder calls abide by dev locks - if (builderCall && editingApp) { - await checkDevAppLocks(ctx) - } + // check general builder stuff, this middleware is a good way + // to find API endpoints which are builder focused + await builderMiddleware(ctx, permType) const isAuthed = ctx.isAuthenticated const { basePermissions, permissions } = await getUserPermissions( @@ -62,9 +39,10 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { // builders for now have permission to do anything // TODO: in future should consider separating permissions with an require("@budibase/auth").isClient check let isBuilder = ctx.user && ctx.user.builder && ctx.user.builder.global + const isBuilderApi = permType === PermissionTypes.BUILDER if (isBuilder) { return next() - } else if (builderCall && !isBuilder) { + } else if (isBuilderApi && !isBuilder) { return ctx.throw(403, "Not Authorized") } diff --git a/packages/server/src/middleware/builder.js b/packages/server/src/middleware/builder.js new file mode 100644 index 0000000000..ff03e208b0 --- /dev/null +++ b/packages/server/src/middleware/builder.js @@ -0,0 +1,75 @@ +const { APP_DEV_PREFIX } = require("../db/utils") +const { + doesUserHaveLock, + updateLock, + checkDebounce, + setDebounce, +} = require("../utilities/redis") +const CouchDB = require("../db") +const { DocumentTypes } = require("../db/utils") +const { PermissionTypes } = require("@budibase/auth/permissions") + +const DEBOUNCE_TIME_SEC = 20 + +/************************************************** * + * This middleware has been broken out of the * + * "authorized" middleware as it had nothing to do * + * with authorization, but requires the perms * + * imparted by it. This middleware shouldn't * + * be called directly, it should always be called * + * through the authorized middleware * + ****************************************************/ + +async function checkDevAppLocks(ctx) { + const appId = ctx.appId + + // if any public usage, don't proceed + if (!ctx.user._id && !ctx.user.userId) { + return + } + + // not a development app, don't need to do anything + if (!appId || !appId.startsWith(APP_DEV_PREFIX)) { + return + } + if (!(await doesUserHaveLock(appId, ctx.user))) { + ctx.throw(403, "User does not hold app lock.") + } + + // they do have lock, update it + await updateLock(appId, ctx.user) +} + +async function updateAppUpdatedAt(ctx) { + const appId = ctx.appId + // if debouncing skip this update + // get methods also aren't updating + if (await checkDebounce(appId) || ctx.method === "GET") { + return + } + const db = new CouchDB(appId) + const metadata = await db.get(DocumentTypes.APP_METADATA) + metadata.updatedAt = new Date().toISOString() + await db.put(metadata) + // set a new debounce record with a short TTL + await setDebounce(appId, DEBOUNCE_TIME_SEC) +} + +module.exports = async (ctx, permType) => { + const appId = ctx.appId + // this only functions within an app context + if (!appId) { + return + } + const isBuilderApi = permType === PermissionTypes.BUILDER + const referer = ctx.headers["referer"] + const editingApp = referer ? referer.includes(appId) : false + // check this is a builder call and editing + if (!isBuilderApi || !editingApp) { + return + } + // check locks + await checkDevAppLocks(ctx) + // set updated at time on app + await updateAppUpdatedAt(ctx) +} \ No newline at end of file diff --git a/packages/server/src/middleware/tests/authorized.spec.js b/packages/server/src/middleware/tests/authorized.spec.js index d51ce4cc4d..50e1b1dcf2 100644 --- a/packages/server/src/middleware/tests/authorized.spec.js +++ b/packages/server/src/middleware/tests/authorized.spec.js @@ -78,9 +78,6 @@ describe("Authorization middleware", () => { }) describe("external web hook call", () => { - let ctx = {} - let middleware - beforeEach(() => { config = new TestConfiguration() config.setEnvironment(true) diff --git a/packages/server/src/utilities/redis.js b/packages/server/src/utilities/redis.js index d1fd3de469..18f967e053 100644 --- a/packages/server/src/utilities/redis.js +++ b/packages/server/src/utilities/redis.js @@ -2,13 +2,13 @@ const { Client, utils } = require("@budibase/auth/redis") const { getGlobalIDFromUserMetadataID } = require("../db/utils") const APP_DEV_LOCK_SECONDS = 600 -const DB_NAME = utils.Databases.DEV_LOCKS -let devAppClient +let devAppClient, debounceClient // we init this as we want to keep the connection open all the time // reduces the performance hit exports.init = async () => { - devAppClient = await new Client(DB_NAME).init() + devAppClient = await new Client(utils.Databases.DEV_LOCKS).init() + debounceClient = await new Client(utils.Databases).init() } exports.doesUserHaveLock = async (devAppId, user) => { @@ -52,3 +52,11 @@ exports.clearLock = async (devAppId, user) => { } await devAppClient.delete(devAppId) } + +exports.checkDebounce = async id => { + return debounceClient.get(id) +} + +exports.setDebounce = async (id, seconds) => { + await debounceClient.store(id, "debouncing", seconds) +}