From 799b3fc138735aec5d6c60d787bdd0d281c10271 Mon Sep 17 00:00:00 2001
From: mike12345567 <me@michaeldrury.co.uk>
Date: Thu, 13 May 2021 12:16:09 +0100
Subject: [PATCH] Updating locks to store the whole global user as well as
 implementing the locks on dev apps fetch.

---
 .../server/src/api/controllers/application.js | 25 ++++++++++-----
 packages/server/src/api/controllers/dev.js    |  3 +-
 packages/server/src/middleware/authorized.js  | 10 +++---
 packages/server/src/utilities/redis.js        | 31 ++++++++++++++++---
 4 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/packages/server/src/api/controllers/application.js b/packages/server/src/api/controllers/application.js
index bcf8a0ec7f..906256208c 100644
--- a/packages/server/src/api/controllers/application.js
+++ b/packages/server/src/api/controllers/application.js
@@ -35,6 +35,7 @@ const { getAllApps } = require("../../utilities")
 const { USERS_TABLE_SCHEMA } = require("../../constants")
 const { getDeployedApps } = require("../../utilities/workerRequests")
 const { clientLibraryPath } = require("../../utilities")
+const { getAllLocks } = require("../../utilities/redis")
 
 const URL_REGEX_SLASH = /\/|\\/g
 
@@ -122,15 +123,23 @@ async function createInstance(template) {
 exports.fetch = async function (ctx) {
   let apps = await getAllApps()
 
-  const appStatus = ctx.query.status
-  if (appStatus) {
-    apps = apps.filter(app => {
-      if (appStatus === AppStatus.DEV) {
-        return app._id.startsWith(DocumentTypes.APP_DEV)
-      }
+  const isDev = ctx.query.status === AppStatus.DEV
+  apps = apps.filter(app => {
+    if (isDev) {
+      return app._id.startsWith(DocumentTypes.APP_DEV)
+    }
+    return !app._id.startsWith(DocumentTypes.APP_DEV)
+  })
 
-      return !app._id.startsWith(DocumentTypes.APP_DEV)
-    })
+  // get the locks for all the dev apps
+  if (isDev) {
+    const locks = await getAllLocks()
+    for (let app of apps) {
+      const lock = locks.find(lock => lock.appId === app._id)
+      if (lock) {
+        app.lockedBy = lock.user
+      }
+    }
   }
 
   ctx.body = apps
diff --git a/packages/server/src/api/controllers/dev.js b/packages/server/src/api/controllers/dev.js
index ab1d1490f6..e6686d4bde 100644
--- a/packages/server/src/api/controllers/dev.js
+++ b/packages/server/src/api/controllers/dev.js
@@ -37,9 +37,8 @@ exports.redirectDelete = async ctx => {
 
 exports.removeLock = async ctx => {
   const { appId } = ctx.params
-  const globalUserId = getGlobalIDFromUserMetadataID(ctx.user._id)
   try {
-    await clearLock(appId, globalUserId)
+    await clearLock(appId, ctx.user)
   } catch (err) {
     ctx.throw(400, "Unable to remove lock.")
   }
diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js
index f3b170dc1c..99163fc321 100644
--- a/packages/server/src/middleware/authorized.js
+++ b/packages/server/src/middleware/authorized.js
@@ -4,7 +4,7 @@ const {
   doesHaveResourcePermission,
   doesHaveBasePermission,
 } = require("../utilities/security/permissions")
-const { APP_DEV_PREFIX, getGlobalIDFromUserMetadataID } = require("../db/utils")
+const { APP_DEV_PREFIX } = require("../db/utils")
 const { doesUserHaveLock, updateLock } = require("../utilities/redis")
 
 function hasResource(ctx) {
@@ -19,17 +19,15 @@ async function checkDevAppLocks(ctx) {
   const appId = ctx.appId
 
   // not a development app, don't need to do anything
-  if (!appId.startsWith(APP_DEV_PREFIX)) {
+  if (!appId || !appId.startsWith(APP_DEV_PREFIX)) {
     return
   }
-  // get the user which is currently using the dev app
-  const userId = getGlobalIDFromUserMetadataID(ctx.user._id)
-  if (!(await doesUserHaveLock(appId, userId))) {
+  if (!(await doesUserHaveLock(appId, ctx.user))) {
     ctx.throw(403, "User does not hold app lock.")
   }
 
   // they do have lock, update it
-  await updateLock(appId, userId)
+  await updateLock(appId, ctx.user)
 }
 
 module.exports = (permType, permLevel = null) => async (ctx, next) => {
diff --git a/packages/server/src/utilities/redis.js b/packages/server/src/utilities/redis.js
index cc5444ce3c..6664bb2072 100644
--- a/packages/server/src/utilities/redis.js
+++ b/packages/server/src/utilities/redis.js
@@ -1,4 +1,5 @@
 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
@@ -10,20 +11,40 @@ exports.init = async () => {
   devAppClient = await new Client(DB_NAME).init()
 }
 
-exports.doesUserHaveLock = async (devAppId, userId) => {
+exports.doesUserHaveLock = async (devAppId, user) => {
   const value = await devAppClient.get(devAppId)
-  return value == null || value === userId
+  if (!value) {
+    return true
+  }
+  // make sure both IDs are global
+  const expected = getGlobalIDFromUserMetadataID(value._id)
+  const userId = getGlobalIDFromUserMetadataID(user._id)
+  return expected === userId
 }
 
-exports.updateLock = async (devAppId, userId) => {
-  await devAppClient.store(devAppId, userId, APP_DEV_LOCK_SECONDS)
+exports.getAllLocks = async () => {
+  const locks = await devAppClient.scan()
+  return locks.map(lock => ({
+    appId: lock.key,
+    user: lock.value,
+  }))
 }
 
-exports.clearLock = async (devAppId, userId) => {
+exports.updateLock = async (devAppId, user) => {
+  // make sure always global user ID
+  const inputUser = {
+    ...user,
+    _id: getGlobalIDFromUserMetadataID(user._id),
+  }
+  await devAppClient.store(devAppId, inputUser, APP_DEV_LOCK_SECONDS)
+}
+
+exports.clearLock = async (devAppId, user) => {
   const value = await devAppClient.get(devAppId)
   if (!value) {
     return
   }
+  const userId = getGlobalIDFromUserMetadataID(user._id)
   if (value !== userId) {
     throw "User does not hold lock, cannot clear it."
   }