From 91847504c8bd4e79f278f5d5287c5c6f5deda088 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 18 Jul 2023 18:10:15 +0100 Subject: [PATCH] Adding test cases for admin/builder checking middlewares. --- .../backend-core/src/middleware/adminOnly.ts | 4 +- .../src/middleware/tests/builder.spec.ts | 141 ++++++++++++++++++ packages/backend-core/src/users.ts | 2 +- .../tests/core/utilities/structures/users.ts | 19 +++ packages/server/src/middleware/currentapp.ts | 1 - packages/types/src/documents/global/user.ts | 11 +- 6 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 packages/backend-core/src/middleware/tests/builder.spec.ts diff --git a/packages/backend-core/src/middleware/adminOnly.ts b/packages/backend-core/src/middleware/adminOnly.ts index dbe1e3a501..dc2fe9064e 100644 --- a/packages/backend-core/src/middleware/adminOnly.ts +++ b/packages/backend-core/src/middleware/adminOnly.ts @@ -1,6 +1,6 @@ -import { BBContext } from "@budibase/types" +import { UserCtx } from "@budibase/types" -export default async (ctx: BBContext, next: any) => { +export default async (ctx: UserCtx, next: any) => { if ( !ctx.internal && (!ctx.user || !ctx.user.admin || !ctx.user.admin.global) diff --git a/packages/backend-core/src/middleware/tests/builder.spec.ts b/packages/backend-core/src/middleware/tests/builder.spec.ts new file mode 100644 index 0000000000..68c72ffe8a --- /dev/null +++ b/packages/backend-core/src/middleware/tests/builder.spec.ts @@ -0,0 +1,141 @@ +import adminOnly from "../adminOnly" +import builderOnly from "../builderOnly" +import builderOrAdmin from "../builderOrAdmin" +import { structures } from "../../../tests" +import { ContextUser } from "@budibase/types" +import { doInAppContext } from "../../context" + +const appId = "app_aaa" +const basicUser = structures.users.user() +const adminUser = structures.users.adminUser() +const adminOnlyUser = structures.users.adminOnlyUser() +const builderUser = structures.users.builderUser() +const appBuilderUser = structures.users.appBuilderUser(appId) + +function buildUserCtx(user: ContextUser) { + return { + internal: false, + user, + throw: jest.fn(), + } as any +} + +function passed(throwFn: jest.Func, nextFn: jest.Func) { + expect(throwFn).not.toBeCalled() + expect(nextFn).toBeCalled() +} + +function threw(throwFn: jest.Func) { + // cant check next, the throw function doesn't actually throw - so it still continues + expect(throwFn).toBeCalled() +} + +describe("adminOnly middleware", () => { + it("should allow admin user", () => { + const ctx = buildUserCtx(adminUser), + next = jest.fn() + adminOnly(ctx, next) + passed(ctx.throw, next) + }) + + it("should not allow basic user", () => { + const ctx = buildUserCtx(basicUser), + next = jest.fn() + adminOnly(ctx, next) + threw(ctx.throw) + }) + + it("should not allow builder user", () => { + const ctx = buildUserCtx(builderUser), + next = jest.fn() + adminOnly(ctx, next) + threw(ctx.throw) + }) +}) + +describe("builderOnly middleware", () => { + it("should allow builder user", () => { + const ctx = buildUserCtx(builderUser), + next = jest.fn() + builderOnly(ctx, next) + passed(ctx.throw, next) + }) + + it("should allow app builder user", () => { + const ctx = buildUserCtx(appBuilderUser), + next = jest.fn() + doInAppContext(appId, () => { + builderOnly(ctx, next) + }) + passed(ctx.throw, next) + }) + + it("should allow admin and builder user", () => { + const ctx = buildUserCtx(adminUser), + next = jest.fn() + builderOnly(ctx, next) + passed(ctx.throw, next) + }) + + it("should not allow admin user", () => { + const ctx = buildUserCtx(adminOnlyUser), + next = jest.fn() + builderOnly(ctx, next) + threw(ctx.throw) + }) + + it("should not allow app builder user to different app", () => { + const ctx = buildUserCtx(appBuilderUser), + next = jest.fn() + doInAppContext("app_bbb", () => { + builderOnly(ctx, next) + }) + threw(ctx.throw) + }) + + it("should not allow basic user", () => { + const ctx = buildUserCtx(basicUser), + next = jest.fn() + builderOnly(ctx, next) + threw(ctx.throw) + }) +}) + +describe("builderOrAdmin middleware", () => { + it("should allow builder user", () => { + const ctx = buildUserCtx(builderUser), + next = jest.fn() + builderOrAdmin(ctx, next) + passed(ctx.throw, next) + }) + + it("should allow builder and admin user", () => { + const ctx = buildUserCtx(adminUser), + next = jest.fn() + builderOrAdmin(ctx, next) + passed(ctx.throw, next) + }) + + it("should allow admin user", () => { + const ctx = buildUserCtx(adminOnlyUser), + next = jest.fn() + builderOrAdmin(ctx, next) + passed(ctx.throw, next) + }) + + it("should allow app builder user", () => { + const ctx = buildUserCtx(appBuilderUser), + next = jest.fn() + doInAppContext(appId, () => { + builderOrAdmin(ctx, next) + }) + passed(ctx.throw, next) + }) + + it("should not allow basic user", () => { + const ctx = buildUserCtx(basicUser), + next = jest.fn() + builderOrAdmin(ctx, next) + threw(ctx.throw) + }) +}) diff --git a/packages/backend-core/src/users.ts b/packages/backend-core/src/users.ts index 2e6ede3cf3..71cd599f87 100644 --- a/packages/backend-core/src/users.ts +++ b/packages/backend-core/src/users.ts @@ -258,7 +258,7 @@ export async function getUserCount() { export function isBuilder(user: User | ContextUser, appId?: string) { if (user.builder?.global) { return true - } else if (appId && user.builder?.apps?.includes(appId)) { + } else if (appId && user.builder?.apps?.includes(getProdAppID(appId))) { return true } return false diff --git a/packages/backend-core/tests/core/utilities/structures/users.ts b/packages/backend-core/tests/core/utilities/structures/users.ts index 7a6b4f0d80..0a4f2e8b54 100644 --- a/packages/backend-core/tests/core/utilities/structures/users.ts +++ b/packages/backend-core/tests/core/utilities/structures/users.ts @@ -1,5 +1,6 @@ import { AdminUser, + AdminOnlyUser, BuilderUser, SSOAuthDetails, SSOUser, @@ -21,6 +22,15 @@ export const adminUser = (userProps?: any): AdminUser => { } } +export const adminOnlyUser = (userProps?: any): AdminOnlyUser => { + return { + ...user(userProps), + admin: { + global: true, + }, + } +} + export const builderUser = (userProps?: any): BuilderUser => { return { ...user(userProps), @@ -30,6 +40,15 @@ export const builderUser = (userProps?: any): BuilderUser => { } } +export const appBuilderUser = (appId: string, userProps?: any): BuilderUser => { + return { + ...user(userProps), + builder: { + apps: [appId], + }, + } +} + export function ssoUser( opts: { user?: any; details?: SSOAuthDetails } = {} ): SSOUser { diff --git a/packages/server/src/middleware/currentapp.ts b/packages/server/src/middleware/currentapp.ts index 10ee13a67a..1f580d80e4 100644 --- a/packages/server/src/middleware/currentapp.ts +++ b/packages/server/src/middleware/currentapp.ts @@ -11,7 +11,6 @@ import { getCachedSelf } from "../utilities/global" import env from "../environment" import { isWebhookEndpoint } from "./utils" import { UserCtx, ContextUser } from "@budibase/types" -import { removePortalUserPermissions } from "@budibase/backend-core/src/users" export default async (ctx: UserCtx, next: any) => { // try to get the appID from the request diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index e9e3ccc662..2ce714801d 100644 --- a/packages/types/src/documents/global/user.ts +++ b/packages/types/src/documents/global/user.ts @@ -42,7 +42,7 @@ export interface User extends Document { forceResetPassword?: boolean roles: UserRoles builder?: { - global: boolean + global?: boolean apps?: string[] } admin?: { @@ -70,7 +70,8 @@ export interface UserRoles { export interface BuilderUser extends User { builder: { - global: boolean + global?: boolean + apps?: string[] } } @@ -83,6 +84,12 @@ export interface AdminUser extends User { } } +export interface AdminOnlyUser extends User { + admin: { + global: boolean + } +} + export function isUser(user: object): user is User { return !!(user as User).roles }