diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index e2bd6c40e5..e12bf3655d 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/packages/server/src/api/controllers/permission.ts @@ -7,6 +7,10 @@ import { GetResourcePermsResponse, ResourcePermissionInfo, GetDependantResourcesResponse, + AddPermissionResponse, + AddPermissionRequest, + RemovePermissionRequest, + RemovePermissionResponse, } from "@budibase/types" import { getRoleParams } from "../../db/utils" import { @@ -16,9 +20,9 @@ import { import { removeFromArray } from "../../utilities" import sdk from "../../sdk" -const PermissionUpdateType = { - REMOVE: "remove", - ADD: "add", +enum PermissionUpdateType { + REMOVE = "remove", + ADD = "add", } const SUPPORTED_LEVELS = CURRENTLY_SUPPORTED_LEVELS @@ -39,7 +43,7 @@ async function updatePermissionOnRole( resourceId, level, }: { roleId: string; resourceId: string; level: PermissionLevel }, - updateType: string + updateType: PermissionUpdateType ) { const allowedAction = await sdk.permissions.resourceActionAllowed({ resourceId, @@ -107,11 +111,15 @@ async function updatePermissionOnRole( } const response = await db.bulkDocs(docUpdates) - return response.map((resp: any) => { + return response.map(resp => { const version = docUpdates.find(role => role._id === resp.id)?.version - resp._id = roles.getExternalRoleID(resp.id, version) - delete resp.id - return resp + const _id = roles.getExternalRoleID(resp.id, version) + return { + _id, + rev: resp.rev, + error: resp.error, + reason: resp.reason, + } }) } @@ -189,13 +197,14 @@ export async function getDependantResources( } } -export async function addPermission(ctx: UserCtx) { - ctx.body = await updatePermissionOnRole(ctx.params, PermissionUpdateType.ADD) +export async function addPermission(ctx: UserCtx) { + const params: AddPermissionRequest = ctx.params + ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.ADD) } -export async function removePermission(ctx: UserCtx) { - ctx.body = await updatePermissionOnRole( - ctx.params, - PermissionUpdateType.REMOVE - ) +export async function removePermission( + ctx: UserCtx +) { + const params: RemovePermissionRequest = ctx.params + ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.REMOVE) } diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 129bc00b44..b9c9ba0359 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/packages/server/src/api/routes/tests/permissions.spec.ts @@ -45,7 +45,7 @@ describe("/permission", () => { table = (await config.createTable()) as typeof table row = await config.createRow() view = await config.api.viewV2.create({ tableId: table._id }) - perms = await config.api.permission.set({ + perms = await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: table._id, level: PermissionLevel.READ, @@ -88,13 +88,13 @@ describe("/permission", () => { }) it("should get resource permissions with multiple roles", async () => { - perms = await config.api.permission.set({ + perms = await config.api.permission.add({ roleId: HIGHER_ROLE_ID, resourceId: table._id, level: PermissionLevel.WRITE, }) const res = await config.api.permission.get(table._id) - expect(res.body).toEqual({ + expect(res).toEqual({ permissions: { read: { permissionType: "EXPLICIT", role: STD_ROLE_ID }, write: { permissionType: "EXPLICIT", role: HIGHER_ROLE_ID }, @@ -117,16 +117,19 @@ describe("/permission", () => { level: PermissionLevel.READ, }) - const response = await config.api.permission.set( + await config.api.permission.add( { roleId: STD_ROLE_ID, resourceId: table._id, level: PermissionLevel.EXECUTE, }, - { expectStatus: 403 } - ) - expect(response.message).toEqual( - "You are not allowed to 'read' the resource type 'datasource'" + { + status: 403, + body: { + message: + "You are not allowed to 'read' the resource type 'datasource'", + }, + } ) }) }) @@ -138,9 +141,9 @@ describe("/permission", () => { resourceId: table._id, level: PermissionLevel.READ, }) - expect(res.body[0]._id).toEqual(STD_ROLE_ID) + expect(res[0]._id).toEqual(STD_ROLE_ID) const permsRes = await config.api.permission.get(table._id) - expect(permsRes.body[STD_ROLE_ID]).toBeUndefined() + expect(permsRes.permissions[STD_ROLE_ID]).toBeUndefined() }) it("throw forbidden if the action is not allowed for the resource", async () => { @@ -156,10 +159,13 @@ describe("/permission", () => { resourceId: table._id, level: PermissionLevel.EXECUTE, }, - { expectStatus: 403 } - ) - expect(response.body.message).toEqual( - "You are not allowed to 'read' the resource type 'datasource'" + { + status: 403, + body: { + message: + "You are not allowed to 'read' the resource type 'datasource'", + }, + } ) }) }) @@ -203,7 +209,7 @@ describe("/permission", () => { }) it("should ignore the view permissions if the flag is not on", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: view.id, level: PermissionLevel.READ, @@ -224,7 +230,7 @@ describe("/permission", () => { it("should use the view permissions if the flag is on", async () => { mocks.licenses.useViewPermissions() - await config.api.permission.set({ + await config.api.permission.add({ roleId: STD_ROLE_ID, resourceId: view.id, level: PermissionLevel.READ, @@ -277,7 +283,7 @@ describe("/permission", () => { const res = await config.api.permission.get(legacyView.name) - expect(res.body).toEqual({ + expect(res).toEqual({ permissions: { read: { permissionType: "BASE", diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index 2e1b1c47b9..7423157678 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -1523,7 +1523,7 @@ describe.each([ }) it("allow public users to fetch when permissions are explicit", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, level: PermissionLevel.READ, resourceId: viewId, @@ -1538,7 +1538,7 @@ describe.each([ }) it("allow public users to fetch when permissions are inherited", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, level: PermissionLevel.READ, resourceId: tableId, @@ -1553,12 +1553,12 @@ describe.each([ }) it("respects inherited permissions, not allowing not public views from public tables", async () => { - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, level: PermissionLevel.READ, resourceId: tableId, }) - await config.api.permission.set({ + await config.api.permission.add({ roleId: roles.BUILTIN_ROLE_IDS.POWER, level: PermissionLevel.READ, resourceId: viewId, diff --git a/packages/server/src/tests/utilities/api/permission.ts b/packages/server/src/tests/utilities/api/permission.ts index ffa89e88f9..986796d9a1 100644 --- a/packages/server/src/tests/utilities/api/permission.ts +++ b/packages/server/src/tests/utilities/api/permission.ts @@ -1,52 +1,39 @@ -import { AnyDocument, PermissionLevel } from "@budibase/types" -import TestConfiguration from "../TestConfiguration" -import { TestAPI } from "./base" +import { + AddPermissionRequest, + AddPermissionResponse, + GetResourcePermsResponse, + RemovePermissionRequest, + RemovePermissionResponse, +} from "@budibase/types" +import { Expectations, TestAPI } from "./base" export class PermissionAPI extends TestAPI { - constructor(config: TestConfiguration) { - super(config) + get = async (resourceId: string, expectations?: Expectations) => { + return await this._get( + `/api/permission/${resourceId}`, + { expectations } + ) } - get = async ( - resourceId: string, - { expectStatus } = { expectStatus: 200 } - ) => { - return this.request - .get(`/api/permission/${resourceId}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - } - - set = async ( - { - roleId, - resourceId, - level, - }: { roleId: string; resourceId: string; level: PermissionLevel }, - { expectStatus } = { expectStatus: 200 } - ): Promise => { - const res = await this.request - .post(`/api/permission/${roleId}/${resourceId}/${level}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return res.body + add = async ( + request: AddPermissionRequest, + expectations?: Expectations + ): Promise => { + const { roleId, resourceId, level } = request + return await this._post( + `/api/permission/${roleId}/${resourceId}/${level}`, + { expectations } + ) } revoke = async ( - { - roleId, - resourceId, - level, - }: { roleId: string; resourceId: string; level: PermissionLevel }, - { expectStatus } = { expectStatus: 200 } + request: RemovePermissionRequest, + expectations?: Expectations ) => { - const res = await this.request - .delete(`/api/permission/${roleId}/${resourceId}/${level}`) - .set(this.config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(expectStatus) - return res + const { roleId, resourceId, level } = request + return await this._delete( + `/api/permission/${roleId}/${resourceId}/${level}`, + { expectations } + ) } } diff --git a/packages/types/src/api/web/app/permission.ts b/packages/types/src/api/web/app/permission.ts index a8ab0e8084..ebe7a8bea3 100644 --- a/packages/types/src/api/web/app/permission.ts +++ b/packages/types/src/api/web/app/permission.ts @@ -1,4 +1,4 @@ -import { PlanType } from "../../../sdk" +import { PermissionLevel, PlanType } from "../../../sdk" export interface ResourcePermissionInfo { role: string @@ -14,3 +14,21 @@ export interface GetResourcePermsResponse { export interface GetDependantResourcesResponse { resourceByType?: Record } + +export interface AddedPermission { + _id: string + rev?: string + error?: string + reason?: string +} + +export type AddPermissionResponse = AddedPermission[] + +export interface AddPermissionRequest { + roleId: string + resourceId: string + level: PermissionLevel +} + +export interface RemovePermissionRequest extends AddPermissionRequest {} +export interface RemovePermissionResponse extends AddPermissionResponse {}