From fe4fcad77cb5beddef83fe0edcf300dc0d544741 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 4 Jun 2021 12:13:29 +0100 Subject: [PATCH] Fixing issue with builder not always having the correct roles to view an app - global builders are now admins in all apps. --- packages/auth/src/security/roles.js | 1 - packages/server/src/api/controllers/auth.js | 2 +- .../server/src/api/controllers/routing.js | 4 --- packages/server/src/api/controllers/user.js | 5 --- .../src/api/routes/tests/routing.spec.js | 4 +-- .../routes/tests/utilities/TestFunctions.js | 17 ++++++--- packages/server/src/middleware/currentapp.js | 2 +- .../src/tests/utilities/TestConfiguration.js | 6 ++-- .../server/src/utilities/workerRequests.js | 36 ++++++++----------- 9 files changed, 31 insertions(+), 46 deletions(-) diff --git a/packages/auth/src/security/roles.js b/packages/auth/src/security/roles.js index d652c25b00..53e1b90d73 100644 --- a/packages/auth/src/security/roles.js +++ b/packages/auth/src/security/roles.js @@ -13,7 +13,6 @@ const BUILTIN_IDS = { POWER: "POWER", BASIC: "BASIC", PUBLIC: "PUBLIC", - BUILDER: "BUILDER", } // exclude internal roles like builder diff --git a/packages/server/src/api/controllers/auth.js b/packages/server/src/api/controllers/auth.js index 92d731cfbb..da863f5493 100644 --- a/packages/server/src/api/controllers/auth.js +++ b/packages/server/src/api/controllers/auth.js @@ -5,7 +5,7 @@ const { getFullUser } = require("../../utilities/users") exports.fetchSelf = async ctx => { const appId = ctx.appId - const { userId } = ctx.user + let userId = ctx.user.userId || ctx.user._id /* istanbul ignore next */ if (!userId) { ctx.body = {} diff --git a/packages/server/src/api/controllers/routing.js b/packages/server/src/api/controllers/routing.js index 1bbb521eab..d281b92fe2 100644 --- a/packages/server/src/api/controllers/routing.js +++ b/packages/server/src/api/controllers/routing.js @@ -63,10 +63,6 @@ exports.fetch = async ctx => { exports.clientFetch = async ctx => { const routing = await getRoutingStructure(ctx.appId) let roleId = ctx.user.role._id - // builder is a special case, always return the full routing structure - if (roleId === BUILTIN_ROLE_IDS.BUILDER) { - roleId = BUILTIN_ROLE_IDS.ADMIN - } const roleIds = await getUserRoleHierarchy(ctx.appId, roleId) for (let topLevel of Object.values(routing.routes)) { for (let subpathKey of Object.keys(topLevel.subpaths)) { diff --git a/packages/server/src/api/controllers/user.js b/packages/server/src/api/controllers/user.js index 73ba56943a..fc207c479f 100644 --- a/packages/server/src/api/controllers/user.js +++ b/packages/server/src/api/controllers/user.js @@ -4,7 +4,6 @@ const { getUserMetadataParams, } = require("../../db/utils") const { InternalTables } = require("../../db/utils") -const { BUILTIN_ROLE_IDS } = require("@budibase/auth/roles") const { getGlobalUsers, addAppRoleToUser, @@ -47,10 +46,6 @@ exports.fetchMetadata = async function (ctx) { exports.updateSelfMetadata = async function (ctx) { // overwrite the ID with current users ctx.request.body._id = ctx.user._id - if (ctx.user.builder && ctx.user.builder.global) { - // specific case, update self role in global user - await addAppRoleToUser(ctx, ctx.appId, BUILTIN_ROLE_IDS.ADMIN) - } // make sure no stale rev delete ctx.request.body._rev await exports.updateMetadata(ctx) diff --git a/packages/server/src/api/routes/tests/routing.spec.js b/packages/server/src/api/routes/tests/routing.spec.js index 622552c77f..38cd62ae76 100644 --- a/packages/server/src/api/routes/tests/routing.spec.js +++ b/packages/server/src/api/routes/tests/routing.spec.js @@ -28,9 +28,7 @@ describe("/routing", () => { it("returns the correct routing for basic user", async () => { workerRequests.getGlobalUsers.mockImplementationOnce((ctx, appId) => { return { - roles: { - [appId]: BUILTIN_ROLE_IDS.BASIC, - } + roleId: BUILTIN_ROLE_IDS.BASIC, } }) const res = await request diff --git a/packages/server/src/api/routes/tests/utilities/TestFunctions.js b/packages/server/src/api/routes/tests/utilities/TestFunctions.js index c49e44c949..dfd77eec7a 100644 --- a/packages/server/src/api/routes/tests/utilities/TestFunctions.js +++ b/packages/server/src/api/routes/tests/utilities/TestFunctions.js @@ -2,6 +2,7 @@ const rowController = require("../../../controllers/row") const appController = require("../../../controllers/application") const CouchDB = require("../../../../db") const { AppStatus } = require("../../../../db/utils") +const { BUILTIN_ROLE_IDS } = require("@budibase/auth/roles") function Request(appId, params) { this.appId = appId @@ -77,11 +78,17 @@ exports.checkPermissionsEndpoint = async ({ .set(passHeader) .expect(200) - user = await config.createUser("fail@budibase.com", password, failRole) - const failHeader = await config.login("fail@budibase.com", password, { - roleId: failRole, - userId: user.globalId, - }) + let failHeader + if (failRole === BUILTIN_ROLE_IDS.PUBLIC) { + failHeader = config.publicHeaders() + } else { + user = await config.createUser("fail@budibase.com", password, failRole) + failHeader = await config.login("fail@budibase.com", password, { + roleId: failRole, + userId: user.globalId, + builder: false, + }) + } await exports .createRequest(config.request, method, url, body) diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index ae83da8ba6..e47c9894fa 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/packages/server/src/middleware/currentapp.js @@ -33,7 +33,7 @@ module.exports = async (ctx, next) => { updateCookie = true appId = requestAppId // retrieving global user gets the right role - roleId = globalUser.roleId + roleId = globalUser.roleId || BUILTIN_ROLE_IDS.PUBLIC } else if (appCookie != null) { appId = appCookie.appId roleId = appCookie.roleId || BUILTIN_ROLE_IDS.PUBLIC diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 60e503c128..d110903ab6 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -101,7 +101,7 @@ class TestConfiguration { userId: GLOBAL_USER_ID, } const app = { - roleId: BUILTIN_ROLE_IDS.BUILDER, + roleId: BUILTIN_ROLE_IDS.ADMIN, appId: this.appId, } const authToken = jwt.sign(auth, env.JWT_SECRET) @@ -306,11 +306,10 @@ class TestConfiguration { return await this._req(config, null, controllers.layout.save) } - async createUser(roleId = BUILTIN_ROLE_IDS.POWER) { + async createUser() { const globalId = `us_${Math.random()}` const resp = await this.globalUser( globalId, - roleId === BUILTIN_ROLE_IDS.BUILDER ) return { ...resp, @@ -319,7 +318,6 @@ class TestConfiguration { } async login(email, password, { roleId, userId, builder } = {}) { - roleId = !roleId ? BUILTIN_ROLE_IDS.BUILDER : roleId userId = !userId ? `us_uuid1` : userId if (!this.request) { throw "Server has not been opened, cannot login." diff --git a/packages/server/src/utilities/workerRequests.js b/packages/server/src/utilities/workerRequests.js index 99d9a1c3e2..59ab2c296c 100644 --- a/packages/server/src/utilities/workerRequests.js +++ b/packages/server/src/utilities/workerRequests.js @@ -9,19 +9,26 @@ function getAppRole(appId, user) { if (!user.roles) { return user } - // always use the deployed app - user.roleId = user.roles[getDeployedAppID(appId)] - if (!user.roleId) { - user.roleId = BUILTIN_ROLE_IDS.PUBLIC + if (user.builder && user.builder.global) { + user.roleId = BUILTIN_ROLE_IDS.ADMIN + } else { + // always use the deployed app + user.roleId = user.roles[getDeployedAppID(appId)] + if (!user.roleId) { + user.roleId = BUILTIN_ROLE_IDS.PUBLIC + } } delete user.roles return user } -function request(ctx, request) { +function request(ctx, request, noApiKey) { if (!request.headers) { request.headers = {} } + if (!noApiKey) { + request.headers["x-budibase-api-key"] = env.INTERNAL_API_KEY + } if (request.body && Object.keys(request.body).length > 0) { request.headers["Content-Type"] = "application/json" request.body = @@ -44,9 +51,6 @@ exports.sendSmtpEmail = async (to, from, subject, contents) => { checkSlashesInUrl(env.WORKER_URL + `/api/admin/email/send`), request(null, { method: "POST", - headers: { - "x-budibase-api-key": env.INTERNAL_API_KEY, - }, body: { email: to, from, @@ -86,16 +90,6 @@ exports.getDeployedApps = async ctx => { } } -exports.deleteGlobalUser = async (ctx, globalId) => { - const endpoint = `/api/admin/users/${globalId}` - const reqCfg = { method: "DELETE" } - const response = await fetch( - checkSlashesInUrl(env.WORKER_URL + endpoint), - request(ctx, reqCfg) - ) - return response.json() -} - exports.getGlobalUsers = async (ctx, appId = null, globalId = null) => { const endpoint = globalId ? `/api/admin/users/${globalId}` @@ -121,7 +115,8 @@ exports.getGlobalSelf = async (ctx, appId = null) => { const endpoint = `/api/admin/users/self` const response = await fetch( checkSlashesInUrl(env.WORKER_URL + endpoint), - request(ctx, { method: "GET" }) + // we don't want to use API key when getting self + request(ctx, { method: "GET" }, true) ) if (response.status !== 200) { ctx.throw(400, "Unable to get self globally.") @@ -172,9 +167,6 @@ exports.removeAppFromUserRoles = async appId => { 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) {