diff --git a/packages/server/src/api/controllers/public/users.ts b/packages/server/src/api/controllers/public/users.ts index 129d2c883f..88dc82ffd2 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}`