Merge pull request #13191 from Budibase/budi-7710-user-groups-do-not-fully-support-custom-roles-5

[fix] BUDI-7710: User groups do not fully support custom roles

packages/backend-core/src/cache/user.ts

@@ -6,7 +6,7 @@ import env from "../environment"
 import * as accounts from "../accounts"
 import { UserDB } from "../users"
 import { sdk } from "@budibase/shared-core"
-import { User } from "@budibase/types"
+import { User, UserMetadata } from "@budibase/types"
 
 const EXPIRY_SECONDS = 3600
 
@@ -15,7 +15,7 @@ const EXPIRY_SECONDS = 3600
  */
 async function populateFromDB(userId: string, tenantId: string) {
   const db = tenancy.getTenantDB(tenantId)
-  const user = await db.get<any>(userId)
+  const user = await db.get<UserMetadata>(userId)
   user.budibaseAccess = true
   if (!env.SELF_HOSTED && !env.DISABLE_ACCOUNT_PORTAL) {
     const account = await accounts.getAccount(user.email)

packages/backend-core/src/db/Replication.ts

@@ -1,66 +1,57 @@
+import PouchDB from "pouchdb"
 import { getPouchDB, closePouchDB } from "./couch"
 import { DocumentType } from "../constants"
 
 class Replication {
-  source: any
+  source: PouchDB.Database
-  target: any
+  target: PouchDB.Database
-  replication: any
 
-  /**
+  constructor({ source, target }: { source: string; target: string }) {
-   *
-   * @param source - the DB you want to replicate or rollback to
-   * @param target - the DB you want to replicate to, or rollback from
-   */
-  constructor({ source, target }: any) {
     this.source = getPouchDB(source)
     this.target = getPouchDB(target)
   }
 
-  close() {
+  async close() {
-    return Promise.all([closePouchDB(this.source), closePouchDB(this.target)])
+    await Promise.all([closePouchDB(this.source), closePouchDB(this.target)])
   }
 
-  promisify(operation: any, opts = {}) {
+  replicate(opts: PouchDB.Replication.ReplicateOptions = {}) {
-    return new Promise(resolve => {
+    return new Promise<PouchDB.Replication.ReplicationResult<{}>>(resolve => {
-      operation(this.target, opts)
+      this.source.replicate
-        .on("denied", function (err: any) {
+        .to(this.target, opts)
+        .on("denied", function (err) {
           // a document failed to replicate (e.g. due to permissions)
           throw new Error(`Denied: Document failed to replicate ${err}`)
         })
-        .on("complete", function (info: any) {
+        .on("complete", function (info) {
           return resolve(info)
         })
-        .on("error", function (err: any) {
+        .on("error", function (err) {
           throw new Error(`Replication Error: ${err}`)
         })
     })
   }
 
-  /**
+  appReplicateOpts(
-   * Two way replication operation, intended to be promise based.
+    opts: PouchDB.Replication.ReplicateOptions = {}
-   * @param opts - PouchDB replication options
+  ): PouchDB.Replication.ReplicateOptions {
-   */
+    if (typeof opts.filter === "string") {
-  sync(opts = {}) {
+      return opts
-    this.replication = this.promisify(this.source.sync, opts)
-    return this.replication
     }
 
-  /**
+    const filter = opts.filter
-   * One way replication operation, intended to be promise based.
+    delete opts.filter
-   * @param opts - PouchDB replication options
-   */
-  replicate(opts = {}) {
-    this.replication = this.promisify(this.source.replicate.to, opts)
-    return this.replication
-  }
 
-  appReplicateOpts() {
     return {
-      filter: (doc: any) => {
+      ...opts,
+      filter: (doc: any, params: any) => {
         if (doc._id && doc._id.startsWith(DocumentType.AUTOMATION_LOG)) {
           return false
         }
-        return doc._id !== DocumentType.APP_METADATA
+        if (doc._id === DocumentType.APP_METADATA) {
+          return false
+        }
+        return filter ? filter(doc, params) : true
       },
     }
   }
@@ -75,10 +66,6 @@ class Replication {
     // take the opportunity to remove deleted tombstones
     await this.replicate()
   }
-
-  cancel() {
-    this.replication.cancel()
-  }
 }
 
 export default Replication

packages/backend-core/src/security/roles.ts

@@ -101,10 +101,7 @@ export function getBuiltinRole(roleId: string): Role | undefined {
 /**
  * Works through the inheritance ranks to see how far up the builtin stack this ID is.
  */
-export function builtinRoleToNumber(id?: string) {
+export function builtinRoleToNumber(id: string) {
-  if (!id) {
-    return 0
-  }
   const builtins = getBuiltinRoles()
   const MAX = Object.values(builtins).length + 1
   if (id === BUILTIN_IDS.ADMIN || id === BUILTIN_IDS.BUILDER) {

packages/server/src/api/controllers/role.ts

@@ -106,6 +106,21 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
   )
   role._rev = result.rev
   ctx.body = role
+
+  const devDb = context.getDevAppDB()
+  const prodDb = context.getProdAppDB()
+
+  if (await prodDb.exists()) {
+    const replication = new dbCore.Replication({
+      source: devDb.name,
+      target: prodDb.name,
+    })
+    await replication.replicate({
+      filter: (doc: any, params: any) => {
+        return doc._id && doc._id.startsWith("role_")
+      },
+    })
+  }
 }
 
 export async function destroy(ctx: UserCtx<void, DestroyRoleResponse>) {

packages/server/src/api/controllers/user.ts

@@ -1,6 +1,6 @@
 import { generateUserFlagID, InternalTables } from "../../db/utils"
 import { getFullUser } from "../../utilities/users"
-import { context } from "@budibase/backend-core"
+import { cache, context } from "@budibase/backend-core"
 import {
   ContextUserMetadata,
   Ctx,

packages/server/src/api/routes/tests/application.spec.ts

@@ -16,8 +16,9 @@ import * as setup from "./utilities"
 import { AppStatus } from "../../../db/utils"
 import { events, utils, context } from "@budibase/backend-core"
 import env from "../../../environment"
-import type { App } from "@budibase/types"
+import { type App } from "@budibase/types"
 import tk from "timekeeper"
+import * as uuid from "uuid"
 
 describe("/applications", () => {
   let config = setup.getConfig()
@@ -251,7 +252,7 @@ describe("/applications", () => {
 
   describe("permissions", () => {
     it("should only return apps a user has access to", async () => {
-      const user = await config.createUser({
+      let user = await config.createUser({
         builder: { global: false },
         admin: { global: false },
       })
@@ -260,6 +261,81 @@ describe("/applications", () => {
         const apps = await config.api.application.fetch()
         expect(apps).toHaveLength(0)
       })
+
+      user = await config.globalUser({
+        ...user,
+        builder: {
+          apps: [config.getProdAppId()],
+        },
+      })
+
+      await config.withUser(user, async () => {
+        const apps = await config.api.application.fetch()
+        expect(apps).toHaveLength(1)
+      })
+    })
+
+    it("should only return apps a user has access to through a custom role", async () => {
+      let user = await config.createUser({
+        builder: { global: false },
+        admin: { global: false },
+      })
+
+      await config.withUser(user, async () => {
+        const apps = await config.api.application.fetch()
+        expect(apps).toHaveLength(0)
+      })
+
+      const role = await config.api.roles.save({
+        name: "Test",
+        inherits: "PUBLIC",
+        permissionId: "read_only",
+        version: "name",
+      })
+
+      user = await config.globalUser({
+        ...user,
+        roles: {
+          [config.getProdAppId()]: role.name,
+        },
+      })
+
+      await config.withUser(user, async () => {
+        const apps = await config.api.application.fetch()
+        expect(apps).toHaveLength(1)
+      })
+    })
+
+    it.only("should only return apps a user has access to through a custom role on a group", async () => {
+      let user = await config.createUser({
+        builder: { global: false },
+        admin: { global: false },
+      })
+
+      await config.withUser(user, async () => {
+        const apps = await config.api.application.fetch()
+        expect(apps).toHaveLength(0)
+      })
+
+      const roleName = uuid.v4().replace(/-/g, "")
+      const role = await config.api.roles.save({
+        name: roleName,
+        inherits: "PUBLIC",
+        permissionId: "read_only",
+        version: "name",
+      })
+
+      const group = await config.createGroup(role._id!)
+
+      user = await config.globalUser({
+        ...user,
+        userGroups: [group._id!],
+      })
+
+      await config.withUser(user, async () => {
+        const apps = await config.api.application.fetch()
+        expect(apps).toHaveLength(1)
+      })
     })
   })
 })

packages/server/src/tests/utilities/TestConfiguration.ts

@@ -299,11 +299,11 @@ export default class TestConfiguration {
     }
   }
 
-  withUser(user: User, f: () => Promise<void>) {
+  async withUser(user: User, f: () => Promise<void>) {
     const oldUser = this.user
     this.user = user
     try {
-      return f()
+      return await f()
     } finally {
       this.user = oldUser
     }
@@ -363,6 +363,7 @@ export default class TestConfiguration {
       _id,
       ...existing,
       ...config,
+      _rev: existing._rev,
       email,
       roles,
       tenantId,
@@ -372,11 +373,12 @@ export default class TestConfiguration {
       admin,
     }
     await sessions.createASession(_id, {
-      sessionId: "sessionid",
+      sessionId: this.sessionIdForUser(_id),
       tenantId: this.getTenantId(),
       csrfToken: this.csrfToken,
     })
     const resp = await db.put(user)
+    await cache.user.invalidateUser(_id)
     return {
       _rev: resp.rev,
       ...user,
@@ -384,9 +386,7 @@ export default class TestConfiguration {
   }
 
   async createUser(user: Partial<User> = {}): Promise<User> {
-    const resp = await this.globalUser(user)
+    return await this.globalUser(user)
-    await cache.user.invalidateUser(resp._id!)
-    return resp
   }
 
   async createGroup(roleId: string = roles.BUILTIN_ROLE_IDS.BASIC) {
@@ -416,6 +416,10 @@ export default class TestConfiguration {
     })
   }
 
+  sessionIdForUser(userId: string): string {
+    return `sessionid-${userId}`
+  }
+
   async login({
     roleId,
     userId,
@@ -442,13 +446,13 @@ export default class TestConfiguration {
         })
       }
       await sessions.createASession(userId, {
-        sessionId: "sessionid",
+        sessionId: this.sessionIdForUser(userId),
         tenantId: this.getTenantId(),
       })
       // have to fake this
       const authObj = {
         userId,
-        sessionId: "sessionid",
+        sessionId: this.sessionIdForUser(userId),
         tenantId: this.getTenantId(),
       }
       const authToken = jwt.sign(authObj, coreEnv.JWT_SECRET as Secret)
@@ -470,7 +474,7 @@ export default class TestConfiguration {
     const user = this.getUser()
     const authObj: AuthToken = {
       userId: user._id!,
-      sessionId: "sessionid",
+      sessionId: this.sessionIdForUser(user._id!),
       tenantId,
     }
     const authToken = jwt.sign(authObj, coreEnv.JWT_SECRET as Secret)

packages/server/src/tests/utilities/api/index.ts

@@ -11,6 +11,7 @@ import { BackupAPI } from "./backup"
 import { AttachmentAPI } from "./attachment"
 import { UserAPI } from "./user"
 import { QueryAPI } from "./query"
+import { RoleAPI } from "./role"
 
 export default class API {
   table: TableAPI
@@ -25,6 +26,7 @@ export default class API {
   attachment: AttachmentAPI
   user: UserAPI
   query: QueryAPI
+  roles: RoleAPI
 
   constructor(config: TestConfiguration) {
     this.table = new TableAPI(config)
@@ -39,5 +41,6 @@ export default class API {
     this.attachment = new AttachmentAPI(config)
     this.user = new UserAPI(config)
     this.query = new QueryAPI(config)
+    this.roles = new RoleAPI(config)
   }
 }

packages/server/src/tests/utilities/api/role.ts

@@ -0,0 +1,41 @@
+import {
+  AccessibleRolesResponse,
+  FetchRolesResponse,
+  FindRoleResponse,
+  SaveRoleRequest,
+  SaveRoleResponse,
+} from "@budibase/types"
+import { Expectations, TestAPI } from "./base"
+
+export class RoleAPI extends TestAPI {
+  fetch = async (expectations?: Expectations) => {
+    return await this._get<FetchRolesResponse>(`/api/roles`, {
+      expectations,
+    })
+  }
+
+  find = async (roleId: string, expectations?: Expectations) => {
+    return await this._get<FindRoleResponse>(`/api/roles/${roleId}`, {
+      expectations,
+    })
+  }
+
+  save = async (body: SaveRoleRequest, expectations?: Expectations) => {
+    return await this._post<SaveRoleResponse>(`/api/roles`, {
+      body,
+      expectations,
+    })
+  }
+
+  destroy = async (roleId: string, expectations?: Expectations) => {
+    return await this._delete(`/api/roles/${roleId}`, {
+      expectations,
+    })
+  }
+
+  accesssible = async (expectations?: Expectations) => {
+    return await this._get<AccessibleRolesResponse>(`/api/roles/accessible`, {
+      expectations,
+    })
+  }
+}

packages/types/src/documents/app/role.ts

@@ -5,4 +5,5 @@ export interface Role extends Document {
   inherits?: string
   permissions: { [key: string]: string[] }
   version?: string
+  name: string
 }