From ad61f2af3b322000d192cd0fc7d183827eda5634 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Mon, 25 Oct 2021 16:59:09 +0100 Subject: [PATCH 1/3] Prevent non builder from accessing dev apps --- .vscode/launch.json | 34 ++++------ packages/server/scripts/jestSetup.js | 2 + .../server/src/api/routes/tests/auth.spec.js | 3 +- .../src/api/routes/tests/permissions.spec.js | 3 + .../server/src/api/routes/tests/query.spec.js | 4 +- .../src/api/routes/tests/routing.spec.js | 32 ++++++--- .../server/src/api/routes/tests/user.spec.js | 2 +- .../routes/tests/utilities/TestFunctions.js | 16 ++--- .../src/api/routes/tests/webhook.spec.js | 5 +- packages/server/src/middleware/currentapp.js | 8 ++- .../src/tests/utilities/TestConfiguration.js | 66 +++++++++++++------ .../server/src/tests/utilities/controllers.js | 1 + .../src/utilities/queue/inMemoryQueue.js | 7 ++ .../worker/src/api/routes/tests/auth.spec.js | 2 +- 14 files changed, 118 insertions(+), 67 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 1587bfc537..34951b6310 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -4,39 +4,27 @@ // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ - { - "type": "node", - "request": "launch", - "name": "Launch Program", - "program": "${workspaceFolder}/app.js", - "skipFiles": [ - "/**" - ] - }, - { - "type": "node", - "request": "launch", - "name": "Debug External", - "program": "${workspaceFolder}/packages/cli/bin/budi", - "args": [], - "cwd":"C:/code/my-apps", - "console": "externalTerminal" - }, { "name": "Budibase Server", "type": "node", "request": "launch", - "runtimeArgs": ["--nolazy", "-r", "ts-node/register/transpile-only"], - "args": ["${workspaceFolder}/packages/server/src/index.ts"], + "runtimeArgs": [ + "--nolazy", + "-r", + "ts-node/register/transpile-only" + ], + "args": [ + "${workspaceFolder}/packages/server/src/index.ts" + ], "cwd": "${workspaceFolder}/packages/server" - }, - { + }, + { "name": "Budibase Worker", "type": "node", "request": "launch", "program": "${workspaceFolder}/packages/worker/src/index.js", "cwd": "${workspaceFolder}/packages/worker" - } + } ], "compounds": [ { diff --git a/packages/server/scripts/jestSetup.js b/packages/server/scripts/jestSetup.js index 1f3551bf5f..6999d92315 100644 --- a/packages/server/scripts/jestSetup.js +++ b/packages/server/scripts/jestSetup.js @@ -8,3 +8,5 @@ env._set("CLIENT_ID", "test-client-id") env._set("BUDIBASE_DIR", tmpdir("budibase-unittests")) env._set("LOG_LEVEL", "silent") env._set("PORT", 0) + +global.console.log = jest.fn() // console.log are ignored in tests diff --git a/packages/server/src/api/routes/tests/auth.spec.js b/packages/server/src/api/routes/tests/auth.spec.js index e8abc2abe8..c50780a8d5 100644 --- a/packages/server/src/api/routes/tests/auth.spec.js +++ b/packages/server/src/api/routes/tests/auth.spec.js @@ -13,8 +13,7 @@ describe("/authenticate", () => { describe("fetch self", () => { it("should be able to fetch self", async () => { - await config.createUser("test@test.com", "p4ssw0rd") - const headers = await config.login("test@test.com", "p4ssw0rd", { userId: "us_uuid1" }) + const headers = await config.login() const res = await request .get(`/api/self`) .set(headers) diff --git a/packages/server/src/api/routes/tests/permissions.spec.js b/packages/server/src/api/routes/tests/permissions.spec.js index ce9f24a572..0304e551cc 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.js +++ b/packages/server/src/api/routes/tests/permissions.spec.js @@ -89,6 +89,9 @@ describe("/permission", () => { describe("check public user allowed", () => { it("should be able to read the row", async () => { + // replicate changes before checking permissions + await config.deploy() + const res = await request .get(`/api/${table._id}/rows`) .set(config.publicHeaders()) diff --git a/packages/server/src/api/routes/tests/query.spec.js b/packages/server/src/api/routes/tests/query.spec.js index 716817509b..8dc153f28e 100644 --- a/packages/server/src/api/routes/tests/query.spec.js +++ b/packages/server/src/api/routes/tests/query.spec.js @@ -94,9 +94,9 @@ describe("/queries", () => { const query = await config.createQuery() const res = await request .get(`/api/queries/${query._id}`) - .set(await config.roleHeaders({})) - .expect("Content-Type", /json/) + .set(await config.defaultHeaders()) .expect(200) + .expect("Content-Type", /json/) expect(res.body.fields).toBeUndefined() expect(res.body.parameters).toBeUndefined() expect(res.body.schema).toBeUndefined() diff --git a/packages/server/src/api/routes/tests/routing.spec.js b/packages/server/src/api/routes/tests/routing.spec.js index 81f56a939d..079fe0dedc 100644 --- a/packages/server/src/api/routes/tests/routing.spec.js +++ b/packages/server/src/api/routes/tests/routing.spec.js @@ -21,16 +21,31 @@ describe("/routing", () => { screen2.routing.roleId = BUILTIN_ROLE_IDS.POWER screen2.routing.route = route screen2 = await config.createScreen(screen2) + await config.deploy() }) describe("fetch", () => { + it("prevents a public user from accessing development app", async () => { + await request + .get(`/api/routing/client`) + .set(config.publicHeaders({ prodApp: false })) + .expect(302) + }) + + it("prevents a non builder from accessing development app", async () => { + await request + .get(`/api/routing/client`) + .set(await config.roleHeaders({ + roleId: BUILTIN_ROLE_IDS.BASIC, + prodApp: false + })) + .expect(302) + }) it("returns the correct routing for basic user", async () => { const res = await request .get(`/api/routing/client`) .set(await config.roleHeaders({ - email: "basic@test.com", - roleId: BUILTIN_ROLE_IDS.BASIC, - builder: false + roleId: BUILTIN_ROLE_IDS.BASIC })) .expect("Content-Type", /json/) .expect(200) @@ -49,9 +64,7 @@ describe("/routing", () => { const res = await request .get(`/api/routing/client`) .set(await config.roleHeaders({ - email: "basic@test.com", - roleId: BUILTIN_ROLE_IDS.POWER, - builder: false, + roleId: BUILTIN_ROLE_IDS.POWER })) .expect("Content-Type", /json/) .expect(200) @@ -71,9 +84,12 @@ describe("/routing", () => { it("should fetch all routes for builder", async () => { const res = await request .get(`/api/routing`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) + .set(await config.roleHeaders({ + roleId: BUILTIN_ROLE_IDS.POWER, + builder: true, + })) .expect(200) + .expect("Content-Type", /json/) expect(res.body.routes).toBeDefined() expect(res.body.routes[route].subpaths[route]).toBeDefined() const subpath = res.body.routes[route].subpaths[route] diff --git a/packages/server/src/api/routes/tests/user.spec.js b/packages/server/src/api/routes/tests/user.spec.js index 48117fbe4b..a9e98f2555 100644 --- a/packages/server/src/api/routes/tests/user.spec.js +++ b/packages/server/src/api/routes/tests/user.spec.js @@ -38,7 +38,7 @@ describe("/users", () => { }) it("should apply authorization to endpoint", async () => { - await config.createUser("brenda@brenda.com", "brendas_password") + await config.createUser() await checkPermissionsEndpoint({ config, request, diff --git a/packages/server/src/api/routes/tests/utilities/TestFunctions.js b/packages/server/src/api/routes/tests/utilities/TestFunctions.js index 87190c3f76..857e902b66 100644 --- a/packages/server/src/api/routes/tests/utilities/TestFunctions.js +++ b/packages/server/src/api/routes/tests/utilities/TestFunctions.js @@ -50,9 +50,10 @@ exports.createRequest = (request, method, url, body) => { } exports.checkBuilderEndpoint = async ({ config, method, url, body }) => { - const headers = await config.login("test@test.com", "test", { + const headers = await config.login({ userId: "us_fail", builder: false, + prodApp: true, }) await exports .createRequest(config.request, method, url, body) @@ -68,11 +69,9 @@ exports.checkPermissionsEndpoint = async ({ passRole, failRole, }) => { - const password = "PASSWORD" - let user = await config.createUser("pass@budibase.com", password, passRole) - const passHeader = await config.login("pass@budibase.com", password, { + const passHeader = await config.login({ roleId: passRole, - userId: user.globalId, + prodApp: true, }) await exports @@ -82,13 +81,12 @@ exports.checkPermissionsEndpoint = async ({ let failHeader if (failRole === BUILTIN_ROLE_IDS.PUBLIC) { - failHeader = config.publicHeaders() + failHeader = config.publicHeaders({ prodApp: true }) } else { - user = await config.createUser("fail@budibase.com", password, failRole) - failHeader = await config.login("fail@budibase.com", password, { + failHeader = await config.login({ roleId: failRole, - userId: user.globalId, builder: false, + prodApp: true, }) } diff --git a/packages/server/src/api/routes/tests/webhook.spec.js b/packages/server/src/api/routes/tests/webhook.spec.js index 75331ea4c7..ed2709a0e6 100644 --- a/packages/server/src/api/routes/tests/webhook.spec.js +++ b/packages/server/src/api/routes/tests/webhook.spec.js @@ -112,8 +112,11 @@ describe("/webhooks", () => { describe("trigger", () => { it("should allow triggering from public", async () => { + // replicate changes before checking webhook + await config.deploy() + const res = await request - .post(`/api/webhooks/trigger/${config.getAppId()}/${webhook._id}`) + .post(`/api/webhooks/trigger/${config.prodAppId}/${webhook._id}`) .expect("Content-Type", /json/) .expect(200) expect(res.body.message).toBeDefined() diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index a39d2eba0a..505bebc694 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/packages/server/src/middleware/currentapp.js @@ -3,7 +3,7 @@ const { getAppId, setCookie, getCookie, clearCookie } = const { Cookies } = require("@budibase/auth").constants const { getRole } = require("@budibase/auth/roles") const { BUILTIN_ROLE_IDS } = require("@budibase/auth/roles") -const { generateUserMetadataID } = require("../db/utils") +const { generateUserMetadataID, isDevAppID } = require("../db/utils") const { dbExists } = require("@budibase/auth/db") const { isUserInAppTenant } = require("@budibase/auth/tenancy") const { getCachedSelf } = require("../utilities/global") @@ -35,6 +35,12 @@ module.exports = async (ctx, next) => { requestAppId = requestAppId || appId } + // deny access to application preview + if (isDevAppID(requestAppId) && (!ctx.user || !ctx.user.builder?.global)) { + clearCookie(ctx, Cookies.CurrentApp) + return ctx.redirect("/") + } + let appId, roleId = BUILTIN_ROLE_IDS.PUBLIC if (!ctx.user) { diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 0eb3851d98..5f1d57467c 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -26,7 +26,6 @@ auth.init(CouchDB) const GLOBAL_USER_ID = "us_uuid1" const EMAIL = "babs@babs.com" -const PASSWORD = "babs_password" class TestConfiguration { constructor(openServer = true) { @@ -67,13 +66,18 @@ class TestConfiguration { return request.body } - async globalUser(id = GLOBAL_USER_ID, builder = true, roles) { + async globalUser({ + id = GLOBAL_USER_ID, + builder = true, + email = EMAIL, + roles, + } = {}) { const db = getGlobalDB(TENANT_ID) let existing try { existing = await db.get(id) } catch (err) { - existing = {} + existing = { email } } const user = { _id: id, @@ -84,6 +88,8 @@ class TestConfiguration { await createASession(id, { sessionId: "sessionid", tenantId: TENANT_ID }) if (builder) { user.builder = { global: true } + } else { + user.builder = { global: false } } const resp = await db.put(user) return { @@ -132,12 +138,14 @@ class TestConfiguration { return headers } - publicHeaders() { + publicHeaders({ prodApp = true } = {}) { + const appId = prodApp ? this.prodAppId : this.appId + const headers = { Accept: "application/json", } - if (this.appId) { - headers[Headers.APP_ID] = this.appId + if (this.prodAppId) { + headers[Headers.APP_ID] = appId } return headers } @@ -146,17 +154,37 @@ class TestConfiguration { email = EMAIL, roleId = BUILTIN_ROLE_IDS.ADMIN, builder = false, - }) { - return this.login(email, PASSWORD, { roleId, builder }) + prodApp = true, + } = {}) { + return this.login({ email, roleId, builder, prodApp }) } async createApp(appName) { + // create dev app this.app = await this._req({ name: appName }, null, controllers.app.create) this.appId = this.app.appId + + // create production app + this.prodApp = await this.deploy() + this.prodAppId = this.prodApp.appId + + this.allApps.push(this.prodApp) this.allApps.push(this.app) + return this.app } + async deploy() { + const deployment = await this._req(null, null, controllers.deploy.deployApp) + const prodAppId = deployment.appId.replace("_dev", "") + const appPackage = await this._req( + null, + { appId: prodAppId }, + controllers.app.fetchAppPackage + ) + return appPackage.application + } + async updateTable(config = null) { config = config || basicTable() this.table = await this._req(config, null, controllers.table.save) @@ -313,9 +341,9 @@ class TestConfiguration { return await this._req(config, null, controllers.layout.save) } - async createUser(id = null) { + async createUser(id = null, email = EMAIL) { const globalId = !id ? `us_${Math.random()}` : `us_${id}` - const resp = await this.globalUser(globalId) + const resp = await this.globalUser({ id: globalId, email }) await userCache.invalidateUser(globalId) return { ...resp, @@ -323,21 +351,21 @@ class TestConfiguration { } } - async login(email, password, { roleId, userId, builder } = {}) { + async login({ roleId, userId, builder, prodApp = false } = {}) { + const appId = prodApp ? this.prodAppId : this.appId + userId = !userId ? `us_uuid1` : userId if (!this.request) { throw "Server has not been opened, cannot login." } // make sure the user exists in the global DB if (roleId !== BUILTIN_ROLE_IDS.PUBLIC) { - const appId = `app${this.getAppId().split("app_dev")[1]}` - await this.globalUser(userId, builder, { - [appId]: roleId, + await this.globalUser({ + userId, + builder, + roles: { [this.prodAppId]: roleId }, }) } - if (!email || !password) { - await this.createUser() - } await createASession(userId, { sessionId: "sessionid", tenantId: TENANT_ID, @@ -350,7 +378,7 @@ class TestConfiguration { } const app = { roleId: roleId, - appId: this.appId, + appId, } const authToken = jwt.sign(auth, env.JWT_SECRET) const appToken = jwt.sign(app, env.JWT_SECRET) @@ -363,7 +391,7 @@ class TestConfiguration { `${Cookies.Auth}=${authToken}`, `${Cookies.CurrentApp}=${appToken}`, ], - [Headers.APP_ID]: this.appId, + [Headers.APP_ID]: appId, } } } diff --git a/packages/server/src/tests/utilities/controllers.js b/packages/server/src/tests/utilities/controllers.js index b07754038f..8da6c97047 100644 --- a/packages/server/src/tests/utilities/controllers.js +++ b/packages/server/src/tests/utilities/controllers.js @@ -12,4 +12,5 @@ module.exports = { screen: require("../../api/controllers/screen"), webhook: require("../../api/controllers/webhook"), layout: require("../../api/controllers/layout"), + deploy: require("../../api/controllers/deploy"), } diff --git a/packages/server/src/utilities/queue/inMemoryQueue.js b/packages/server/src/utilities/queue/inMemoryQueue.js index d4b1d297d8..c57bac978c 100644 --- a/packages/server/src/utilities/queue/inMemoryQueue.js +++ b/packages/server/src/utilities/queue/inMemoryQueue.js @@ -82,6 +82,13 @@ class InMemoryQueue { // TODO: implement for testing console.log(cronJobId) } + + /** + * Implemented for tests + */ + getRepeatableJobs() { + return [] + } } module.exports = InMemoryQueue diff --git a/packages/worker/src/api/routes/tests/auth.spec.js b/packages/worker/src/api/routes/tests/auth.spec.js index dacff30ce3..bb66c547c4 100644 --- a/packages/worker/src/api/routes/tests/auth.spec.js +++ b/packages/worker/src/api/routes/tests/auth.spec.js @@ -24,7 +24,7 @@ describe("/api/global/auth", () => { // initially configure settings await config.saveSmtpConfig() await config.saveSettingsConfig() - await config.createUser("test@test.com") + await config.createUser() const res = await request .post(`/api/global/auth/${TENANT_ID}/reset`) .send({ From 46a886f83ddb4c88399717a35bee3704d24d3bae Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Tue, 26 Oct 2021 09:42:19 +0100 Subject: [PATCH 2/3] Linting --- packages/server/src/middleware/currentapp.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/server/src/middleware/currentapp.js b/packages/server/src/middleware/currentapp.js index 505bebc694..01b9dcc248 100644 --- a/packages/server/src/middleware/currentapp.js +++ b/packages/server/src/middleware/currentapp.js @@ -36,7 +36,10 @@ module.exports = async (ctx, next) => { } // deny access to application preview - if (isDevAppID(requestAppId) && (!ctx.user || !ctx.user.builder?.global)) { + if ( + isDevAppID(requestAppId) && + (!ctx.user || !ctx.user.builder || !ctx.user.builder.global) + ) { clearCookie(ctx, Cookies.CurrentApp) return ctx.redirect("/") } From 67f851b68cb93fa4ac39267d9088a16abe5daf74 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Tue, 26 Oct 2021 16:21:26 +0100 Subject: [PATCH 3/3] Update app id check in tests --- packages/server/src/tests/utilities/TestConfiguration.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/tests/utilities/TestConfiguration.js b/packages/server/src/tests/utilities/TestConfiguration.js index 5f1d57467c..dfdc770389 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.js +++ b/packages/server/src/tests/utilities/TestConfiguration.js @@ -144,7 +144,7 @@ class TestConfiguration { const headers = { Accept: "application/json", } - if (this.prodAppId) { + if (appId) { headers[Headers.APP_ID] = appId } return headers