Fixing issue with roles not being added correctly to global users and cleaning up roles when an app is deleted.

packages/server/src/api/controllers/application.js

@@ -27,7 +27,7 @@ const { cloneDeep } = require("lodash/fp")
 const { processObject } = require("@budibase/string-templates")
 const { getAllApps } = require("../../utilities")
 const { USERS_TABLE_SCHEMA } = require("../../constants")
-const { getDeployedApps } = require("../../utilities/workerRequests")
+const { getDeployedApps, removeAppFromUserRoles } = require("../../utilities/workerRequests")
 const { clientLibraryPath } = require("../../utilities")
 const { getAllLocks } = require("../../utilities/redis")
 
@@ -245,6 +245,8 @@ exports.delete = async function (ctx) {
   if (!env.isTest()) {
     await deleteApp(ctx.params.appId)
   }
+  // make sure the app/role doesn't stick around after the app has been deleted
+  await removeAppFromUserRoles(ctx.params.appId)
 
   ctx.status = 200
   ctx.body = result

packages/server/src/api/routes/tests/utilities/index.js

@@ -13,6 +13,7 @@ jest.mock("../../../../utilities/workerRequests", () => ({
       _id: "us_uuid1",
     }
   }),
+  removeAppFromUserRoles: jest.fn(),
 }))
 
 exports.delay = ms => new Promise(resolve => setTimeout(resolve, ms))

packages/server/src/utilities/workerRequests.js

@@ -57,11 +57,10 @@ exports.sendSmtpEmail = async (to, from, subject, contents) => {
     })
   )
 
-  const json = await response.json()
+  if (response.status !== 200) {
-  if (json.status !== 200 && response.status !== 200) {
     throw "Unable to send email."
   }
-  return json
+  return response.json()
 }
 
 exports.getDeployedApps = async ctx => {
@@ -124,10 +123,10 @@ exports.getGlobalSelf = async (ctx, appId = null) => {
     checkSlashesInUrl(env.WORKER_URL + endpoint),
     request(ctx, { method: "GET" })
   )
-  let json = await response.json()
+  if (response.status !== 200) {
-  if (json.status !== 200 && response.status !== 200) {
     ctx.throw(400, "Unable to get self globally.")
   }
+  let json = await response.json()
   if (appId) {
     json = getAppRole(appId, json)
   }
@@ -151,7 +150,7 @@ exports.addAppRoleToUser = async (ctx, appId, roleId, userId = null) => {
     ...body,
     roles: {
       ...user.roles,
-      [appId]: roleId,
+      [getDeployedAppID(appId)]: roleId,
     },
   }
   const response = await fetch(
@@ -161,9 +160,26 @@ exports.addAppRoleToUser = async (ctx, appId, roleId, userId = null) => {
       body,
     })
   )
-  const json = await response.json()
+  if (response.status !== 200) {
-  if (json.status !== 200 && response.status !== 200) {
     ctx.throw(400, "Unable to save self globally.")
   }
-  return json
+  return response.json()
 }
+
+exports.removeAppFromUserRoles = async appId => {
+  const deployedAppId = getDeployedAppID(appId)
+  const response = await fetch(
+    checkSlashesInUrl(env.WORKER_URL + `/api/admin/roles/${deployedAppId}`),
+    request(null, {
+      method: "DELETE",
+      headers: {
+        "x-budibase-api-key": env.INTERNAL_API_KEY,
+      }
+    })
+  )
+  if (response.status !== 200) {
+    throw "Unable to remove app role"
+  }
+  return response.json()
+}
+

packages/worker/src/api/controllers/admin/users.js

@@ -11,6 +11,16 @@ const { sendEmail } = require("../../../utilities/email")
 
 const GLOBAL_DB = StaticDatabases.GLOBAL.name
 
+async function allUsers() {
+  const db = new CouchDB(GLOBAL_DB)
+  const response = await db.allDocs(
+    getGlobalUserParams(null, {
+      include_docs: true,
+    })
+  )
+  return response.rows.map(row => row.doc)
+}
+
 exports.save = async ctx => {
   const db = new CouchDB(GLOBAL_DB)
   const { email, password, _id } = ctx.request.body
@@ -105,6 +115,23 @@ exports.destroy = async ctx => {
   }
 }
 
+exports.removeAppRole = async ctx => {
+  const { appId } = ctx.params
+  const db = new CouchDB(GLOBAL_DB)
+  const users = await allUsers()
+  const bulk = []
+  for (let user of users) {
+    if (user.roles[appId]) {
+      delete user.roles[appId]
+      bulk.push(user)
+    }
+  }
+  await db.bulkDocs(bulk)
+  ctx.body = {
+    message: "App role removed from all users",
+  }
+}
+
 exports.getSelf = async ctx => {
   ctx.params = {
     id: ctx.user._id,
@@ -134,13 +161,7 @@ exports.updateSelf = async ctx => {
 
 // called internally by app server user fetch
 exports.fetch = async ctx => {
-  const db = new CouchDB(GLOBAL_DB)
+  const users = await allUsers()
-  const response = await db.allDocs(
-    getGlobalUserParams(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) {

packages/worker/src/api/routes/admin/users.js

@@ -67,6 +67,7 @@ router
     controller.save
   )
   .get("/api/admin/users", adminOnly, controller.fetch)
+  .delete("/api/admin/roles/:appId", adminOnly, controller.removeAppRole)
   .delete("/api/admin/users/:id", adminOnly, controller.destroy)
   .get("/api/admin/roles/:appId")
   .post(