Fix for app sync, base it on group roles, not just user roles - stops app sync from pulling in group users which do not actually have access to the app.

This commit is contained in:
mike12345567 2023-03-21 13:55:28 +00:00
parent e54173ee4c
commit 1c73b92595
3 changed files with 43 additions and 29 deletions

View File

@ -44,7 +44,6 @@ import {
Layout, Layout,
Screen, Screen,
MigrationType, MigrationType,
BBContext,
Database, Database,
UserCtx, UserCtx,
} from "@budibase/types" } from "@budibase/types"
@ -74,14 +73,14 @@ async function getScreens() {
).rows.map((row: any) => row.doc) ).rows.map((row: any) => row.doc)
} }
function getUserRoleId(ctx: BBContext) { function getUserRoleId(ctx: UserCtx) {
return !ctx.user?.role || !ctx.user.role._id return !ctx.user?.role || !ctx.user.role._id
? roles.BUILTIN_ROLE_IDS.PUBLIC ? roles.BUILTIN_ROLE_IDS.PUBLIC
: ctx.user.role._id : ctx.user.role._id
} }
function checkAppUrl( function checkAppUrl(
ctx: BBContext, ctx: UserCtx,
apps: App[], apps: App[],
url: string, url: string,
currentAppId?: string currentAppId?: string
@ -95,7 +94,7 @@ function checkAppUrl(
} }
function checkAppName( function checkAppName(
ctx: BBContext, ctx: UserCtx,
apps: App[], apps: App[],
name: string, name: string,
currentAppId?: string currentAppId?: string
@ -160,7 +159,7 @@ async function addDefaultTables(db: Database) {
await db.bulkDocs([...defaultDbDocs]) await db.bulkDocs([...defaultDbDocs])
} }
export async function fetch(ctx: BBContext) { export async function fetch(ctx: UserCtx) {
const dev = ctx.query && ctx.query.status === AppStatus.DEV const dev = ctx.query && ctx.query.status === AppStatus.DEV
const all = ctx.query && ctx.query.status === AppStatus.ALL const all = ctx.query && ctx.query.status === AppStatus.ALL
const apps = (await dbCore.getAllApps({ dev, all })) as App[] const apps = (await dbCore.getAllApps({ dev, all })) as App[]
@ -185,7 +184,7 @@ export async function fetch(ctx: BBContext) {
ctx.body = await checkAppMetadata(apps) ctx.body = await checkAppMetadata(apps)
} }
export async function fetchAppDefinition(ctx: BBContext) { export async function fetchAppDefinition(ctx: UserCtx) {
const layouts = await getLayouts() const layouts = await getLayouts()
const userRoleId = getUserRoleId(ctx) const userRoleId = getUserRoleId(ctx)
const accessController = new roles.AccessController() const accessController = new roles.AccessController()
@ -231,7 +230,7 @@ export async function fetchAppPackage(ctx: UserCtx) {
} }
} }
async function performAppCreate(ctx: BBContext) { async function performAppCreate(ctx: UserCtx) {
const apps = (await dbCore.getAllApps({ dev: true })) as App[] const apps = (await dbCore.getAllApps({ dev: true })) as App[]
const name = ctx.request.body.name, const name = ctx.request.body.name,
possibleUrl = ctx.request.body.url possibleUrl = ctx.request.body.url
@ -360,7 +359,7 @@ async function creationEvents(request: any, app: App) {
} }
} }
async function appPostCreate(ctx: BBContext, app: App) { async function appPostCreate(ctx: UserCtx, app: App) {
const tenantId = tenancy.getTenantId() const tenantId = tenancy.getTenantId()
await migrations.backPopulateMigrations({ await migrations.backPopulateMigrations({
type: MigrationType.APP, type: MigrationType.APP,
@ -391,7 +390,7 @@ async function appPostCreate(ctx: BBContext, app: App) {
} }
} }
export async function create(ctx: BBContext) { export async function create(ctx: UserCtx) {
const newApplication = await quotas.addApp(() => performAppCreate(ctx)) const newApplication = await quotas.addApp(() => performAppCreate(ctx))
await appPostCreate(ctx, newApplication) await appPostCreate(ctx, newApplication)
await cache.bustCache(cache.CacheKey.CHECKLIST) await cache.bustCache(cache.CacheKey.CHECKLIST)
@ -401,7 +400,7 @@ export async function create(ctx: BBContext) {
// This endpoint currently operates as a PATCH rather than a PUT // This endpoint currently operates as a PATCH rather than a PUT
// Thus name and url fields are handled only if present // Thus name and url fields are handled only if present
export async function update(ctx: BBContext) { export async function update(ctx: UserCtx) {
const apps = (await dbCore.getAllApps({ dev: true })) as App[] const apps = (await dbCore.getAllApps({ dev: true })) as App[]
// validation // validation
const name = ctx.request.body.name, const name = ctx.request.body.name,
@ -421,7 +420,7 @@ export async function update(ctx: BBContext) {
ctx.body = app ctx.body = app
} }
export async function updateClient(ctx: BBContext) { export async function updateClient(ctx: UserCtx) {
// Get current app version // Get current app version
const db = context.getAppDB() const db = context.getAppDB()
const application = await db.get(DocumentType.APP_METADATA) const application = await db.get(DocumentType.APP_METADATA)
@ -445,7 +444,7 @@ export async function updateClient(ctx: BBContext) {
ctx.body = app ctx.body = app
} }
export async function revertClient(ctx: BBContext) { export async function revertClient(ctx: UserCtx) {
// Check app can be reverted // Check app can be reverted
const db = context.getAppDB() const db = context.getAppDB()
const application = await db.get(DocumentType.APP_METADATA) const application = await db.get(DocumentType.APP_METADATA)
@ -471,7 +470,7 @@ export async function revertClient(ctx: BBContext) {
ctx.body = app ctx.body = app
} }
const unpublishApp = async (ctx: any) => { async function unpublishApp(ctx: UserCtx) {
let appId = ctx.params.appId let appId = ctx.params.appId
appId = dbCore.getProdAppID(appId) appId = dbCore.getProdAppID(appId)
@ -487,7 +486,7 @@ const unpublishApp = async (ctx: any) => {
return result return result
} }
async function destroyApp(ctx: BBContext) { async function destroyApp(ctx: UserCtx) {
let appId = ctx.params.appId let appId = ctx.params.appId
appId = dbCore.getProdAppID(appId) appId = dbCore.getProdAppID(appId)
const devAppId = dbCore.getDevAppID(appId) const devAppId = dbCore.getDevAppID(appId)
@ -515,12 +514,12 @@ async function destroyApp(ctx: BBContext) {
return result return result
} }
async function preDestroyApp(ctx: BBContext) { async function preDestroyApp(ctx: UserCtx) {
const { rows } = await getUniqueRows([ctx.params.appId]) const { rows } = await getUniqueRows([ctx.params.appId])
ctx.rowCount = rows.length ctx.rowCount = rows.length
} }
async function postDestroyApp(ctx: BBContext) { async function postDestroyApp(ctx: UserCtx) {
const rowCount = ctx.rowCount const rowCount = ctx.rowCount
await groups.cleanupApp(ctx.params.appId) await groups.cleanupApp(ctx.params.appId)
if (rowCount) { if (rowCount) {
@ -528,7 +527,7 @@ async function postDestroyApp(ctx: BBContext) {
} }
} }
export async function destroy(ctx: BBContext) { export async function destroy(ctx: UserCtx) {
await preDestroyApp(ctx) await preDestroyApp(ctx)
const result = await destroyApp(ctx) const result = await destroyApp(ctx)
await postDestroyApp(ctx) await postDestroyApp(ctx)
@ -536,7 +535,7 @@ export async function destroy(ctx: BBContext) {
ctx.body = result ctx.body = result
} }
export const unpublish = async (ctx: BBContext) => { export async function unpublish(ctx: UserCtx) {
const prodAppId = dbCore.getProdAppID(ctx.params.appId) const prodAppId = dbCore.getProdAppID(ctx.params.appId)
const dbExists = await dbCore.dbExists(prodAppId) const dbExists = await dbCore.dbExists(prodAppId)
@ -551,7 +550,7 @@ export const unpublish = async (ctx: BBContext) => {
ctx.status = 204 ctx.status = 204
} }
export async function sync(ctx: BBContext) { export async function sync(ctx: UserCtx) {
const appId = ctx.params.appId const appId = ctx.params.appId
try { try {
ctx.body = await sdk.applications.syncApp(appId) ctx.body = await sdk.applications.syncApp(appId)

View File

@ -9,7 +9,10 @@ import { isEqual } from "lodash"
export function combineMetadataAndUser(user: any, metadata: any) { export function combineMetadataAndUser(user: any, metadata: any) {
// skip users with no access // skip users with no access
if (user.roleId === rolesCore.BUILTIN_ROLE_IDS.PUBLIC) { if (
user.roleId == null ||
user.roleId === rolesCore.BUILTIN_ROLE_IDS.PUBLIC
) {
return null return null
} }
delete user._rev delete user._rev

View File

@ -8,7 +8,7 @@ import {
} from "@budibase/backend-core" } from "@budibase/backend-core"
import env from "../environment" import env from "../environment"
import { groups } from "@budibase/pro" import { groups } from "@budibase/pro"
import { BBContext, ContextUser, User } from "@budibase/types" import { UserCtx, ContextUser, User, UserGroup } from "@budibase/types"
export function updateAppRole( export function updateAppRole(
user: ContextUser, user: ContextUser,
@ -43,33 +43,40 @@ export function updateAppRole(
async function checkGroupRoles( async function checkGroupRoles(
user: ContextUser, user: ContextUser,
{ appId }: { appId?: string } = {} opts: { appId?: string; groups?: UserGroup[] } = {}
) { ) {
if (user.roleId && user.roleId !== roles.BUILTIN_ROLE_IDS.PUBLIC) { if (user.roleId && user.roleId !== roles.BUILTIN_ROLE_IDS.PUBLIC) {
return user return user
} }
if (appId) { if (opts.appId) {
user.roleId = await groups.getGroupRoleId(user as User, appId) user.roleId = await groups.getGroupRoleId(user as User, opts.appId, {
groups: opts.groups,
})
}
// final fallback, simply couldn't find a role - user must be public
if (!user.roleId) {
user.roleId = roles.BUILTIN_ROLE_IDS.PUBLIC
} }
return user return user
} }
async function processUser( async function processUser(
user: ContextUser, user: ContextUser,
{ appId }: { appId?: string } = {} opts: { appId?: string; groups?: UserGroup[] } = {}
) { ) {
if (user) { if (user) {
delete user.password delete user.password
} }
user = await updateAppRole(user, { appId }) const appId = opts.appId || context.getAppId()
user = updateAppRole(user, { appId })
if (!user.roleId && user?.userGroups?.length) { if (!user.roleId && user?.userGroups?.length) {
user = await checkGroupRoles(user, { appId }) user = await checkGroupRoles(user, { appId, groups: opts?.groups })
} }
return user return user
} }
export async function getCachedSelf(ctx: BBContext, appId: string) { export async function getCachedSelf(ctx: UserCtx, appId: string) {
// this has to be tenant aware, can't depend on the context to find it out // this has to be tenant aware, can't depend on the context to find it out
// running some middlewares before the tenancy causes context to break // running some middlewares before the tenancy causes context to break
const user = await cache.user.getUser(ctx.user?._id!) const user = await cache.user.getUser(ctx.user?._id!)
@ -90,6 +97,7 @@ export async function getGlobalUser(userId: string) {
export async function getGlobalUsers(users?: ContextUser[]) { export async function getGlobalUsers(users?: ContextUser[]) {
const appId = context.getAppId() const appId = context.getAppId()
const db = tenancy.getGlobalDB() const db = tenancy.getGlobalDB()
const allGroups = await groups.fetch()
let globalUsers let globalUsers
if (users) { if (users) {
const globalIds = users.map(user => const globalIds = users.map(user =>
@ -118,7 +126,11 @@ export async function getGlobalUsers(users?: ContextUser[]) {
return globalUsers return globalUsers
} }
return globalUsers.map(user => updateAppRole(user)) // pass in the groups, meaning we don't actually need to retrieve them for
// each user individually
return Promise.all(
globalUsers.map(user => processUser(user, { groups: allGroups }))
)
} }
export async function getGlobalUsersFromMetadata(users: ContextUser[]) { export async function getGlobalUsersFromMetadata(users: ContextUser[]) {