From 05efe0506109c4c01f60723ae5f8c8670f55a6c7 Mon Sep 17 00:00:00 2001 From: Martin McKeaveney Date: Tue, 9 Mar 2021 11:27:12 +0000 Subject: [PATCH] tests for authorized middleware --- packages/server/src/middleware/authorized.js | 9 +- .../src/middleware/tests/TestConfiguration.js | 265 ++---------------- .../{authorized.js => authenticated.spec.js} | 0 .../src/middleware/tests/authorized.spec.js | 196 +++++++++++++ .../src/middleware/tests/resourceId.spec.js | 0 .../src/middleware/tests/selfhost.spec.js | 2 +- 6 files changed, 227 insertions(+), 245 deletions(-) rename packages/server/src/middleware/tests/{authorized.js => authenticated.spec.js} (100%) create mode 100644 packages/server/src/middleware/tests/authorized.spec.js create mode 100644 packages/server/src/middleware/tests/resourceId.spec.js diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js index 7eac602f78..6d646d46fd 100644 --- a/packages/server/src/middleware/authorized.js +++ b/packages/server/src/middleware/authorized.js @@ -24,6 +24,7 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { if (!env.CLOUD && LOCAL_PASS.test(ctx.request.url)) { return next() } + if (env.CLOUD && ctx.headers["x-api-key"] && ctx.headers["x-instanceid"]) { // api key header passed by external webhook if (await isAPIKeyValid(ctx.headers["x-api-key"])) { @@ -37,14 +38,14 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { return next() } - ctx.throw(403, "API key invalid") + return ctx.throw(403, "API key invalid") } // don't expose builder endpoints in the cloud if (env.CLOUD && permType === PermissionTypes.BUILDER) return if (!ctx.user) { - ctx.throw(403, "No user info found") + return ctx.throw(403, "No user info found") } const role = ctx.user.role @@ -52,7 +53,7 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { ctx.appId, role._id ) - const isAdmin = ADMIN_ROLES.indexOf(role._id) !== -1 + const isAdmin = ADMIN_ROLES.includes(role._id) const isAuthed = ctx.auth.authenticated // this may need to change in the future, right now only admins @@ -61,7 +62,7 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { if (isAdmin && isAuthed) { return next() } else if (permType === PermissionTypes.BUILDER) { - ctx.throw(403, "Not Authorized") + return ctx.throw(403, "Not Authorized") } if ( diff --git a/packages/server/src/middleware/tests/TestConfiguration.js b/packages/server/src/middleware/tests/TestConfiguration.js index 8092130ce3..11d925e8f0 100644 --- a/packages/server/src/middleware/tests/TestConfiguration.js +++ b/packages/server/src/middleware/tests/TestConfiguration.js @@ -1,247 +1,32 @@ -// const { BUILTIN_ROLE_IDS } = require("../../../../utilities/security/roles") -// const jwt = require("jsonwebtoken") -// const env = require("../../../../environment") -// const { -// basicTable, -// basicRow, -// basicRole, -// basicAutomation, -// basicDatasource, -// basicQuery, -// } = require("./structures") -// const controllers = require("./controllers") -// const supertest = require("supertest") +let env = require("../../environment") -// const EMAIL = "babs@babs.com" -// const PASSWORD = "babs_password" +class TestConfiguration { + constructor(middleware) { + // env = config.env || {} + this.middleware = middleware + this.next = jest.fn() + this.throwMock = jest.fn() + } -// class TestConfiguration { -// constructor() { -// env.PORT = 4002 -// this.server = require("../../../../app") -// // we need the request for logging in, involves cookies, hard to fake -// this.request = supertest(this.server) -// this.appId = null -// } + callMiddleware(ctx, next) { + return this.middleware(ctx, next) + } -// getRequest() { -// return this.request -// } + clear() { + jest.clearAllMocks() + } -// getAppId() { -// return this.appId -// } + setEnv(config) { + env = config + } -// async _req(config, params, controlFunc) { -// const request = {} -// // fake cookies, we don't need them -// request.cookies = { set: () => {}, get: () => {} } -// request.config = { jwtSecret: env.JWT_SECRET } -// request.appId = this.appId -// request.user = { appId: this.appId } -// request.request = { -// body: config, -// } -// if (params) { -// request.params = params -// } -// await controlFunc(request) -// return request.body -// } + async init() { + // return this.createApp(appName) + } -// async init(appName = "test_application") { -// return this.createApp(appName) -// } + end() { + // this.server.close() + } +} -// end() { -// this.server.close() -// } - -// defaultHeaders() { -// const builderUser = { -// userId: "BUILDER", -// roleId: BUILTIN_ROLE_IDS.BUILDER, -// } -// const builderToken = jwt.sign(builderUser, env.JWT_SECRET) -// const headers = { -// Accept: "application/json", -// Cookie: [`budibase:builder:local=${builderToken}`], -// } -// if (this.appId) { -// headers["x-budibase-app-id"] = this.appId -// } -// return headers -// } - -// publicHeaders() { -// const headers = { -// Accept: "application/json", -// } -// if (this.appId) { -// headers["x-budibase-app-id"] = this.appId -// } -// return headers -// } - -// async callMiddleware() { -// this.middleware(this.ctx, next) -// return this.app -// } - -// async updateTable(config = null) { -// config = config || basicTable() -// this.table = await this._req(config, null, controllers.table.save) -// return this.table -// } - -// async createTable(config = null) { -// if (config != null && config._id) { -// delete config._id -// } -// return this.updateTable(config) -// } - -// async getTable(tableId = null) { -// tableId = tableId || this.table._id -// return this._req(null, { id: tableId }, controllers.table.find) -// } - -// async createLinkedTable() { -// if (!this.table) { -// throw "Must have created a table first." -// } -// const tableConfig = basicTable() -// tableConfig.primaryDisplay = "name" -// tableConfig.schema.link = { -// type: "link", -// fieldName: "link", -// tableId: this.table._id, -// } -// const linkedTable = await this.createTable(tableConfig) -// this.linkedTable = linkedTable -// return linkedTable -// } - -// async createAttachmentTable() { -// const table = basicTable() -// table.schema.attachment = { -// type: "attachment", -// } -// return this.createTable(table) -// } - -// async createRow(config = null) { -// if (!this.table) { -// throw "Test requires table to be configured." -// } -// config = config || basicRow(this.table._id) -// return this._req(config, { tableId: this.table._id }, controllers.row.save) -// } - -// async createRole(config = null) { -// config = config || basicRole() -// return this._req(config, null, controllers.role.save) -// } - -// async addPermission(roleId, resourceId, level = "read") { -// return this._req( -// null, -// { -// roleId, -// resourceId, -// level, -// }, -// controllers.perms.addPermission -// ) -// } - -// async createView(config) { -// if (!this.table) { -// throw "Test requires table to be configured." -// } -// const view = config || { -// map: "function(doc) { emit(doc[doc.key], doc._id); } ", -// tableId: this.table._id, -// } -// return this._req(view, null, controllers.view.save) -// } - -// async createAutomation(config) { -// config = config || basicAutomation() -// if (config._rev) { -// delete config._rev -// } -// this.automation = ( -// await this._req(config, null, controllers.automation.create) -// ).automation -// return this.automation -// } - -// async getAllAutomations() { -// return this._req(null, null, controllers.automation.fetch) -// } - -// async deleteAutomation(automation = null) { -// automation = automation || this.automation -// if (!automation) { -// return -// } -// return this._req( -// null, -// { id: automation._id, rev: automation._rev }, -// controllers.automation.destroy -// ) -// } - -// async createDatasource(config = null) { -// config = config || basicDatasource() -// this.datasource = await this._req(config, null, controllers.datasource.save) -// return this.datasource -// } - -// async createQuery(config = null) { -// if (!this.datasource && !config) { -// throw "No data source created for query." -// } -// config = config || basicQuery(this.datasource._id) -// return this._req(config, null, controllers.query.save) -// } - -// async createUser( -// email = EMAIL, -// password = PASSWORD, -// roleId = BUILTIN_ROLE_IDS.POWER -// ) { -// return this._req( -// { -// email, -// password, -// roleId, -// }, -// null, -// controllers.user.create -// ) -// } - -// async login(email, password) { -// if (!email || !password) { -// await this.createUser() -// email = EMAIL -// password = PASSWORD -// } -// const result = await this.request -// .post(`/api/authenticate`) -// .set({ -// "x-budibase-app-id": this.appId, -// }) -// .send({ email, password }) - -// // returning necessary request headers -// return { -// Accept: "application/json", -// Cookie: result.headers["set-cookie"], -// } -// } -// } - -// module.exports = TestConfiguration +module.exports = TestConfiguration diff --git a/packages/server/src/middleware/tests/authorized.js b/packages/server/src/middleware/tests/authenticated.spec.js similarity index 100% rename from packages/server/src/middleware/tests/authorized.js rename to packages/server/src/middleware/tests/authenticated.spec.js diff --git a/packages/server/src/middleware/tests/authorized.spec.js b/packages/server/src/middleware/tests/authorized.spec.js new file mode 100644 index 0000000000..d3e5e52d2d --- /dev/null +++ b/packages/server/src/middleware/tests/authorized.spec.js @@ -0,0 +1,196 @@ +const authorizedMiddleware = require("../authorized") +const env = require("../../environment") +const apiKey = require("../../utilities/security/apikey") +const { AuthTypes } = require("../../constants") +const { PermissionTypes, PermissionLevels } = require("../../utilities/security/permissions") +const { Test } = require("supertest") +jest.mock("../../environment") +jest.mock("../../utilities/security/apikey") + +class TestConfiguration { + constructor(role) { + this.middleware = authorizedMiddleware(role) + this.next = jest.fn() + this.throw = jest.fn() + this.ctx = { + headers: {}, + request: { + url: "" + }, + auth: {}, + next: this.next, + throw: this.throw + } + } + + executeMiddleware() { + return this.middleware(this.ctx, this.next) + } + + setUser(user) { + this.ctx.user = user + } + + setMiddlewareRequiredPermission(...perms) { + this.middleware = authorizedMiddleware(...perms) + } + + setResourceId(id) { + this.ctx.resourceId = id + } + + setAuthenticated(isAuthed) { + this.ctx.auth = { authenticated: isAuthed } + } + + setRequestUrl(url) { + this.ctx.request.url = url + } + + setCloudEnv(isCloud) { + env.CLOUD = isCloud + } + + setRequestHeaders(headers) { + this.ctx.headers = headers + } + + afterEach() { + jest.clearAllMocks() + } +} + + +describe("Authorization middleware", () => { + const next = jest.fn() + let config + + afterEach(() => { + config.afterEach() + }) + + beforeEach(() => { + config = new TestConfiguration() + }) + + it("passes the middleware for local webhooks", async () => { + config.setRequestUrl("https://something/webhooks/trigger") + await config.executeMiddleware() + expect(config.next).toHaveBeenCalled() + }) + + describe("external web hook call", () => { + let ctx = {} + let middleware + + beforeEach(() => { + config = new TestConfiguration() + config.setCloudEnv(true) + config.setRequestHeaders({ + "x-api-key": "abc123", + "x-instanceid": "instance123", + }) + }) + + it("passes to next() if api key is valid", async () => { + apiKey.isAPIKeyValid.mockResolvedValueOnce(true) + + await config.executeMiddleware() + + expect(config.next).toHaveBeenCalled() + expect(config.ctx.auth).toEqual({ + authenticated: AuthTypes.EXTERNAL, + apiKey: config.ctx.headers["x-api-key"], + }) + expect(config.ctx.user).toEqual({ + appId: config.ctx.headers["x-instanceid"], + }) + }) + + it("throws if api key is invalid", async () => { + apiKey.isAPIKeyValid.mockResolvedValueOnce(false) + + await config.executeMiddleware() + + expect(config.throw).toHaveBeenCalledWith(403, "API key invalid") + }) + }) + + describe("non-webhook call", () => { + let config + + beforeEach(() => { + config = new TestConfiguration() + config.setCloudEnv(true) + config.setAuthenticated(true) + }) + + it("throws when no user data is present in context", async () => { + await config.executeMiddleware() + + expect(config.throw).toHaveBeenCalledWith(403, "No user info found") + }) + + it("passes on to next() middleware if user is an admin", async () => { + config.setUser({ + role: { + _id: "ADMIN", + } + }) + + await config.executeMiddleware() + + expect(config.next).toHaveBeenCalled() + }) + + it("throws if the user has only builder permissions", async () => { + config.setCloudEnv(false) + config.setMiddlewareRequiredPermission(PermissionTypes.BUILDER) + config.setUser({ + role: { + _id: "" + } + }) + await config.executeMiddleware() + + expect(config.throw).toHaveBeenCalledWith(403, "Not Authorized") + }) + + it("passes on to next() middleware if the user has resource permission", async () => { + config.setResourceId(PermissionTypes.QUERY) + config.setUser({ + role: { + _id: "" + } + }) + config.setMiddlewareRequiredPermission(PermissionTypes.QUERY) + + await config.executeMiddleware() + expect(config.next).toHaveBeenCalled() + }) + + it("throws if the user session is not authenticated after permission checks", async () => { + config.setUser({ + role: { + _id: "" + }, + }) + config.setAuthenticated(false) + + await config.executeMiddleware() + expect(config.throw).toHaveBeenCalledWith(403, "Session not authenticated") + }) + + it("throws if the user does not have base permissions to perform the operation", async () => { + config.setUser({ + role: { + _id: "" + }, + }) + config.setMiddlewareRequiredPermission(PermissionTypes.ADMIN, PermissionLevels.BASIC) + + await config.executeMiddleware() + expect(config.throw).toHaveBeenCalledWith(403, "User does not have permission") + }) + }) +}) diff --git a/packages/server/src/middleware/tests/resourceId.spec.js b/packages/server/src/middleware/tests/resourceId.spec.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/server/src/middleware/tests/selfhost.spec.js b/packages/server/src/middleware/tests/selfhost.spec.js index 9d66f44463..3601df89a2 100644 --- a/packages/server/src/middleware/tests/selfhost.spec.js +++ b/packages/server/src/middleware/tests/selfhost.spec.js @@ -1,6 +1,6 @@ const selfHostMiddleware = require("../selfhost"); const env = require("../../environment") -const hosting = require("../../utilities/builder/hosting") +const hosting = require("../../utilities/builder/hosting"); jest.mock("../../environment") jest.mock("../../utilities/builder/hosting")