From bedc8e5cce8afdba3abc88a64d7aed0158ffcf5c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 12 Jun 2023 18:39:30 +0100 Subject: [PATCH 1/2] Fix for custom roles that have not been published causing users to be unable to access an app completely. They should instead be treated as public users as their role isn't valid. --- packages/backend-core/src/security/roles.ts | 9 ++++++++- packages/server/src/api/controllers/role.ts | 10 +++++----- packages/server/src/middleware/currentapp.ts | 6 +++--- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index bdf7a38726..66c44503ba 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -140,9 +140,13 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { * Gets the role object, this is mainly useful for two purposes, to check if the level exists and * to check if the role inherits any others. * @param {string|null} roleId The level ID to lookup. + * @param {object|null} opts options for the function, like whether to halt errors, instead return public. * @returns {Promise} The role object, which may contain an "inherits" property. */ -export async function getRole(roleId?: string): Promise { +export async function getRole( + roleId?: string, + opts?: { defaultPublic?: boolean } +): Promise { if (!roleId) { return undefined } @@ -161,6 +165,9 @@ export async function getRole(roleId?: string): Promise { // finalise the ID role._id = getExternalRoleID(role._id) } catch (err) { + if (opts?.defaultPublic) { + return cloneDeep(BUILTIN_ROLES.PUBLIC) + } // only throw an error if there is no role at all if (Object.keys(role).length === 0) { throw err diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 77c8f8842b..419643cdc7 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -4,7 +4,7 @@ import { getUserMetadataParams, InternalTables, } from "../../db/utils" -import { BBContext, Database } from "@budibase/types" +import { UserCtx, Database } from "@budibase/types" const UpdateRolesOptions = { CREATED: "created", @@ -38,15 +38,15 @@ async function updateRolesOnUserTable( } } -export async function fetch(ctx: BBContext) { +export async function fetch(ctx: UserCtx) { ctx.body = await roles.getAllRoles() } -export async function find(ctx: BBContext) { +export async function find(ctx: UserCtx) { ctx.body = await roles.getRole(ctx.params.roleId) } -export async function save(ctx: BBContext) { +export async function save(ctx: UserCtx) { const db = context.getAppDB() let { _id, name, inherits, permissionId } = ctx.request.body let isCreate = false @@ -72,7 +72,7 @@ export async function save(ctx: BBContext) { ctx.message = `Role '${role.name}' created successfully.` } -export async function destroy(ctx: BBContext) { +export async function destroy(ctx: UserCtx) { const db = context.getAppDB() const roleId = ctx.params.roleId const role = await db.get(roleId) diff --git a/packages/server/src/middleware/currentapp.ts b/packages/server/src/middleware/currentapp.ts index efafc59e21..e63e18463d 100644 --- a/packages/server/src/middleware/currentapp.ts +++ b/packages/server/src/middleware/currentapp.ts @@ -96,15 +96,15 @@ export default async (ctx: UserCtx, next: any) => { const userId = ctx.user ? generateUserMetadataID(ctx.user._id!) : undefined - ctx.user = { + let role = (ctx.user = { ...ctx.user!, // override userID with metadata one _id: userId, userId, globalId, roleId, - role: await roles.getRole(roleId), - } + role: await roles.getRole(roleId, { defaultPublic: true }), + }) } return next() From 1bdf55e9660a32f5e39a8802cd70eab2169c0826 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 13 Jun 2023 14:45:33 +0100 Subject: [PATCH 2/2] Some type updates and fixes for test case. --- packages/backend-core/src/security/roles.ts | 2 +- packages/server/src/api/controllers/routing.ts | 6 +++--- packages/server/src/middleware/currentapp.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index 66c44503ba..e8a3c76c0a 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -165,7 +165,7 @@ export async function getRole( // finalise the ID role._id = getExternalRoleID(role._id) } catch (err) { - if (opts?.defaultPublic) { + if (!isBuiltin(roleId) && opts?.defaultPublic) { return cloneDeep(BUILTIN_ROLES.PUBLIC) } // only throw an error if there is no role at all diff --git a/packages/server/src/api/controllers/routing.ts b/packages/server/src/api/controllers/routing.ts index 05ab35aea2..1bfd289637 100644 --- a/packages/server/src/api/controllers/routing.ts +++ b/packages/server/src/api/controllers/routing.ts @@ -1,6 +1,6 @@ import { getRoutingInfo } from "../../utilities/routing" import { roles } from "@budibase/backend-core" -import { BBContext } from "@budibase/types" +import { UserCtx } from "@budibase/types" const URL_SEPARATOR = "/" @@ -56,11 +56,11 @@ async function getRoutingStructure() { return { routes: routing.json } } -export async function fetch(ctx: BBContext) { +export async function fetch(ctx: UserCtx) { ctx.body = await getRoutingStructure() } -export async function clientFetch(ctx: BBContext) { +export async function clientFetch(ctx: UserCtx) { const routing = await getRoutingStructure() let roleId = ctx.user?.role?._id const roleIds = (await roles.getUserRoleHierarchy(roleId, { diff --git a/packages/server/src/middleware/currentapp.ts b/packages/server/src/middleware/currentapp.ts index e63e18463d..6879a103bc 100644 --- a/packages/server/src/middleware/currentapp.ts +++ b/packages/server/src/middleware/currentapp.ts @@ -96,7 +96,7 @@ export default async (ctx: UserCtx, next: any) => { const userId = ctx.user ? generateUserMetadataID(ctx.user._id!) : undefined - let role = (ctx.user = { + ctx.user = { ...ctx.user!, // override userID with metadata one _id: userId, @@ -104,7 +104,7 @@ export default async (ctx: UserCtx, next: any) => { globalId, roleId, role: await roles.getRole(roleId, { defaultPublic: true }), - }) + } } return next()