From 821de55363e771c536a6a85fbaa9efb30801b7df Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 9 Feb 2021 17:24:36 +0000 Subject: [PATCH] Adding basic permissions test which proves a public user can read from a table, but cannot write. --- .../src/api/routes/tests/couchTestUtils.js | 19 ++++++++++++++-- .../src/api/routes/tests/permissions.spec.js | 19 +++++++++++++++- .../server/src/api/routes/tests/row.spec.js | 8 ++----- packages/server/src/middleware/authorized.js | 22 +++++++++++-------- .../server/src/utilities/security/roles.js | 2 +- 5 files changed, 51 insertions(+), 19 deletions(-) diff --git a/packages/server/src/api/routes/tests/couchTestUtils.js b/packages/server/src/api/routes/tests/couchTestUtils.js index 0bc7f90998..d89dce4087 100644 --- a/packages/server/src/api/routes/tests/couchTestUtils.js +++ b/packages/server/src/api/routes/tests/couchTestUtils.js @@ -40,6 +40,17 @@ exports.defaultHeaders = appId => { return headers } +exports.publicHeaders = appId => { + const headers = { + Accept: "application/json", + } + if (appId) { + headers["x-budibase-app-id"] = appId + } + + return headers +} + exports.BASE_TABLE = { name: "TestTable", type: "table", @@ -73,13 +84,17 @@ exports.createTable = async (request, appId, table, removeId = true) => { return res.body } -exports.createRow = async (request, appId, tableId, row = null) => { - row = row || { +exports.makeBasicRow = tableId => { + return { name: "Test Contact", description: "original description", status: "new", tableId: tableId, } +} + +exports.createRow = async (request, appId, tableId, row = null) => { + row = row || exports.makeBasicRow(tableId) const res = await request .post(`/api/${tableId}/rows`) .send(row) diff --git a/packages/server/src/api/routes/tests/permissions.spec.js b/packages/server/src/api/routes/tests/permissions.spec.js index e1322b7b13..8353eb271d 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.js +++ b/packages/server/src/api/routes/tests/permissions.spec.js @@ -5,6 +5,8 @@ const { supertest, defaultHeaders, addPermission, + publicHeaders, + makeBasicRow, } = require("./couchTestUtils") const { BUILTIN_ROLE_IDS } = require("../../../utilities/security/roles") @@ -102,7 +104,22 @@ describe("/permission", () => { describe("check public user allowed", () => { it("should be able to read the row", async () => { - // TODO + const res = await request + .get(`/api/${table._id}/rows`) + .set(publicHeaders(appId)) + .expect("Content-Type", /json/) + .expect(200) + expect(res.body[0]._id).toEqual(row._id) + }) + + it("shouldn't allow writing from a public user", async () => { + const res = await request + .post(`/api/${table._id}/rows`) + .send(makeBasicRow(table._id)) + .set(publicHeaders(appId)) + .expect("Content-Type", /json/) + .expect(403) + expect(res.status).toEqual(403) }) }) }) diff --git a/packages/server/src/api/routes/tests/row.spec.js b/packages/server/src/api/routes/tests/row.spec.js index 47977ed207..c411016b71 100644 --- a/packages/server/src/api/routes/tests/row.spec.js +++ b/packages/server/src/api/routes/tests/row.spec.js @@ -5,6 +5,7 @@ const { defaultHeaders, createLinkedTable, createAttachmentTable, + makeBasicRow, } = require("./couchTestUtils"); const { enrichRows } = require("../../../utilities") const env = require("../../../environment") @@ -30,12 +31,7 @@ describe("/rows", () => { app = await createApplication(request) appId = app.instance._id table = await createTable(request, appId) - row = { - name: "Test Contact", - description: "original description", - status: "new", - tableId: table._id - } + row = makeBasicRow(table._id) }) const createRow = async r => diff --git a/packages/server/src/middleware/authorized.js b/packages/server/src/middleware/authorized.js index b838ef4e29..7eac602f78 100644 --- a/packages/server/src/middleware/authorized.js +++ b/packages/server/src/middleware/authorized.js @@ -43,12 +43,8 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { // don't expose builder endpoints in the cloud if (env.CLOUD && permType === PermissionTypes.BUILDER) return - if (!ctx.auth.authenticated) { - ctx.throw(403, "Session not authenticated") - } - if (!ctx.user) { - ctx.throw(403, "User not found") + ctx.throw(403, "No user info found") } const role = ctx.user.role @@ -56,11 +52,15 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { ctx.appId, role._id ) - if (ADMIN_ROLES.indexOf(role._id) !== -1) { - return next() - } + const isAdmin = ADMIN_ROLES.indexOf(role._id) !== -1 + const isAuthed = ctx.auth.authenticated - if (permType === PermissionTypes.BUILDER) { + // this may need to change in the future, right now only admins + // can have access to builder features, this is hard coded into + // our rules + if (isAdmin && isAuthed) { + return next() + } else if (permType === PermissionTypes.BUILDER) { ctx.throw(403, "Not Authorized") } @@ -71,6 +71,10 @@ module.exports = (permType, permLevel = null) => async (ctx, next) => { return next() } + if (!isAuthed) { + ctx.throw(403, "Session not authenticated") + } + if (!doesHaveBasePermission(permType, permLevel, basePermissions)) { ctx.throw(403, "User does not have permission") } diff --git a/packages/server/src/utilities/security/roles.js b/packages/server/src/utilities/security/roles.js index afc7fd217f..447c049e1d 100644 --- a/packages/server/src/utilities/security/roles.js +++ b/packages/server/src/utilities/security/roles.js @@ -77,7 +77,7 @@ exports.getRole = async (appId, roleId) => { } try { const db = new CouchDB(appId) - const dbRole = await db.get(roleId) + const dbRole = await db.get(exports.getDBRoleID(roleId)) role = Object.assign(role, dbRole) // finalise the ID role._id = exports.getExternalRoleID(role._id)