From b9ce140d954f77859ac500245ca5b4a282292bf8 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 6 Dec 2022 17:20:26 +0000 Subject: [PATCH 1/2] Fix for #7732 - as well as some tests for it, make sure that it is working as expected. --- packages/backend-core/src/db/utils.ts | 2 +- .../src/api/controllers/public/users.ts | 29 +++++++++++--- .../api/routes/public/tests/compare.spec.js | 24 ++---------- .../src/api/routes/public/tests/users.spec.js | 38 +++++++++++++++++++ .../src/api/routes/public/tests/utils.ts | 28 ++++++++++++++ .../server/src/utilities/workerRequests.ts | 4 +- 6 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 packages/server/src/api/routes/public/tests/users.spec.js create mode 100644 packages/server/src/api/routes/public/tests/utils.ts diff --git a/packages/backend-core/src/db/utils.ts b/packages/backend-core/src/db/utils.ts index 590c3eeef8..870744ffea 100644 --- a/packages/backend-core/src/db/utils.ts +++ b/packages/backend-core/src/db/utils.ts @@ -181,7 +181,7 @@ export function generateUserMetadataID(globalId: string) { /** * Breaks up the ID to get the global ID. */ -export function getGlobalIDFromUserMetadataID(id: string) { +export function getGlobalIDFromUserMetadataID(id?: string) { const prefix = `${DocumentType.ROW}${SEPARATOR}${InternalTable.USER_METADATA}${SEPARATOR}` if (!id || !id.includes(prefix)) { return id diff --git a/packages/server/src/api/controllers/public/users.ts b/packages/server/src/api/controllers/public/users.ts index 129d2c883f..4140d3c978 100644 --- a/packages/server/src/api/controllers/public/users.ts +++ b/packages/server/src/api/controllers/public/users.ts @@ -5,9 +5,18 @@ import { saveGlobalUser, } from "../../../utilities/workerRequests" import { publicApiUserFix } from "../../../utilities/users" +import { db as dbCore } from "@budibase/backend-core" import { search as stringSearch } from "./utils" +import { BBContext, User } from "@budibase/types" -function getUser(ctx: any, userId?: string) { +function isLoggedInUser(ctx: BBContext, user: User) { + const loggedInId = ctx.user?._id + const globalUserId = dbCore.getGlobalIDFromUserMetadataID(loggedInId) + // check both just incase + return globalUserId === user._id || loggedInId === user._id +} + +function getUser(ctx: BBContext, userId?: string) { if (userId) { ctx.params = { userId } } else if (!ctx.params?.userId) { @@ -16,37 +25,45 @@ function getUser(ctx: any, userId?: string) { return readGlobalUser(ctx) } -export async function search(ctx: any, next: any) { +export async function search(ctx: BBContext, next: any) { const { name } = ctx.request.body const users = await allGlobalUsers(ctx) ctx.body = stringSearch(users, name, "email") await next() } -export async function create(ctx: any, next: any) { +export async function create(ctx: BBContext, next: any) { const response = await saveGlobalUser(publicApiUserFix(ctx)) ctx.body = await getUser(ctx, response._id) await next() } -export async function read(ctx: any, next: any) { +export async function read(ctx: BBContext, next: any) { ctx.body = await readGlobalUser(ctx) await next() } -export async function update(ctx: any, next: any) { +export async function update(ctx: BBContext, next: any) { const user = await readGlobalUser(ctx) ctx.request.body = { ...ctx.request.body, _rev: user._rev, } + // disallow updating your own role - always overwrite with DB roles + if (isLoggedInUser(ctx, user)) { + ctx.request.body.roles = user.roles + } const response = await saveGlobalUser(publicApiUserFix(ctx)) ctx.body = await getUser(ctx, response._id) await next() } -export async function destroy(ctx: any, next: any) { +export async function destroy(ctx: BBContext, next: any) { const user = await getUser(ctx) + // disallow deleting yourself + if (isLoggedInUser(ctx, user)) { + ctx.throw(405, "Cannot delete user using its own API key.") + } await deleteGlobalUser(ctx) ctx.body = user await next() diff --git a/packages/server/src/api/routes/public/tests/compare.spec.js b/packages/server/src/api/routes/public/tests/compare.spec.js index 860907b69d..eaf0fb2049 100644 --- a/packages/server/src/api/routes/public/tests/compare.spec.js +++ b/packages/server/src/api/routes/public/tests/compare.spec.js @@ -1,41 +1,23 @@ const jestOpenAPI = require("jest-openapi").default const generateSchema = require("../../../../../specs/generate") const setup = require("../../tests/utilities") -const { checkSlashesInUrl } = require("../../../../utilities") +const { generateMakeRequest } = require("./utils") const yamlPath = generateSchema() jestOpenAPI(yamlPath) -let request = setup.getRequest() let config = setup.getConfig() -let apiKey, table, app +let apiKey, table, app, makeRequest beforeAll(async () => { app = await config.init() table = await config.updateTable() apiKey = await config.generateApiKey() + makeRequest = generateMakeRequest(apiKey, setup) }) afterAll(setup.afterAll) -async function makeRequest(method, endpoint, body, appId = config.getAppId()) { - const extraHeaders = { - "x-budibase-api-key": apiKey, - } - if (appId) { - extraHeaders["x-budibase-app-id"] = appId - } - const req = request - [method](checkSlashesInUrl(`/api/public/v1/${endpoint}`)) - .set(config.defaultHeaders(extraHeaders)) - if (body) { - req.send(body) - } - const res = await req - expect(res.body).toBeDefined() - return res -} - describe("check the applications endpoints", () => { it("should allow retrieving applications through search", async () => { const res = await makeRequest("post", "/applications/search") diff --git a/packages/server/src/api/routes/public/tests/users.spec.js b/packages/server/src/api/routes/public/tests/users.spec.js new file mode 100644 index 0000000000..82f63cb847 --- /dev/null +++ b/packages/server/src/api/routes/public/tests/users.spec.js @@ -0,0 +1,38 @@ +const setup = require("../../tests/utilities") +const { generateMakeRequest } = require("./utils") + +const workerRequests = require("../../../../utilities/workerRequests") + +let config = setup.getConfig() +let apiKey, globalUser, makeRequest + +beforeAll(async () => { + await config.init() + globalUser = await config.globalUser() + apiKey = await config.generateApiKey(globalUser._id) + makeRequest = generateMakeRequest(apiKey, setup) + workerRequests.readGlobalUser.mockReturnValue(globalUser) +}) + +afterAll(setup.afterAll) + +describe("check user endpoints", () => { + it("should not allow a user to update their own roles", async () => { + const res = await makeRequest("put", `/users/${globalUser._id}`, { + ...globalUser, + roles: { + "app_1": "ADMIN", + } + }) + expect(workerRequests.saveGlobalUser.mock.lastCall[0].body.data.roles["app_1"]).toBeUndefined() + expect(res.status).toBe(200) + expect(res.body.data.roles["app_1"]).toBeUndefined() + }) + + it("should not allow a user to delete themselves", async () => { + const res = await makeRequest("delete", `/users/${globalUser._id}`) + expect(res.status).toBe(405) + expect(workerRequests.deleteGlobalUser.mock.lastCall).toBeUndefined() + }) +}) + diff --git a/packages/server/src/api/routes/public/tests/utils.ts b/packages/server/src/api/routes/public/tests/utils.ts new file mode 100644 index 0000000000..ad468332e6 --- /dev/null +++ b/packages/server/src/api/routes/public/tests/utils.ts @@ -0,0 +1,28 @@ +import { checkSlashesInUrl } from "../../../../utilities" + +export function generateMakeRequest(apiKey: string, setup: any) { + const request = setup.getRequest() + const config = setup.getConfig() + return async ( + method: string, + endpoint: string, + body?: any, + intAppId: string = config.getAppId() + ) => { + const extraHeaders: any = { + "x-budibase-api-key": apiKey, + } + if (intAppId) { + extraHeaders["x-budibase-app-id"] = intAppId + } + const req = request[method]( + checkSlashesInUrl(`/api/public/v1/${endpoint}`) + ).set(config.defaultHeaders(extraHeaders)) + if (body) { + req.send(body) + } + const res = await req + expect(res.body).toBeDefined() + return res + } +} diff --git a/packages/server/src/utilities/workerRequests.ts b/packages/server/src/utilities/workerRequests.ts index d1fd467025..b3a61ddd68 100644 --- a/packages/server/src/utilities/workerRequests.ts +++ b/packages/server/src/utilities/workerRequests.ts @@ -3,7 +3,7 @@ import env from "../environment" import { checkSlashesInUrl } from "./index" import { db as dbCore, constants, tenancy } from "@budibase/backend-core" import { updateAppRole } from "./global" -import { BBContext, Automation } from "@budibase/types" +import { BBContext, User } from "@budibase/types" export function request(ctx?: BBContext, request?: any) { if (!request.headers) { @@ -138,7 +138,7 @@ export async function deleteGlobalUser(ctx: BBContext) { return checkResponse(response, "delete user", { ctx }) } -export async function readGlobalUser(ctx: BBContext) { +export async function readGlobalUser(ctx: BBContext): Promise { const response = await fetch( checkSlashesInUrl( env.WORKER_URL + `/api/global/users/${ctx.params.userId}` From 4300e5c7332bf504621e0689841618d337c98674 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 6 Dec 2022 18:23:55 +0000 Subject: [PATCH 2/2] Fix for CI build failure. --- packages/backend-core/src/db/utils.ts | 2 +- packages/server/src/api/controllers/public/users.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/db/utils.ts b/packages/backend-core/src/db/utils.ts index 870744ffea..590c3eeef8 100644 --- a/packages/backend-core/src/db/utils.ts +++ b/packages/backend-core/src/db/utils.ts @@ -181,7 +181,7 @@ export function generateUserMetadataID(globalId: string) { /** * Breaks up the ID to get the global ID. */ -export function getGlobalIDFromUserMetadataID(id?: string) { +export function getGlobalIDFromUserMetadataID(id: string) { const prefix = `${DocumentType.ROW}${SEPARATOR}${InternalTable.USER_METADATA}${SEPARATOR}` if (!id || !id.includes(prefix)) { return id diff --git a/packages/server/src/api/controllers/public/users.ts b/packages/server/src/api/controllers/public/users.ts index 4140d3c978..88dc82ffd2 100644 --- a/packages/server/src/api/controllers/public/users.ts +++ b/packages/server/src/api/controllers/public/users.ts @@ -11,7 +11,7 @@ import { BBContext, User } from "@budibase/types" function isLoggedInUser(ctx: BBContext, user: User) { const loggedInId = ctx.user?._id - const globalUserId = dbCore.getGlobalIDFromUserMetadataID(loggedInId) + const globalUserId = dbCore.getGlobalIDFromUserMetadataID(loggedInId!) // check both just incase return globalUserId === user._id || loggedInId === user._id }