From 4b38b5263b5028f728d953b0dfa200d96ee2126e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 11 Mar 2024 13:14:02 +0100 Subject: [PATCH 01/15] Allow group members edits to admins --- .../portal/users/users/[userId].svelte | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte index 1d15b13107..eb0305e959 100644 --- a/packages/builder/src/pages/builder/portal/users/users/[userId].svelte +++ b/packages/builder/src/pages/builder/portal/users/users/[userId].svelte @@ -39,9 +39,10 @@ name: { width: "1fr", }, - ...(readonly + ...(!isAdmin ? {} - : { + : // Add + { _id: { displayName: "", width: "auto", @@ -90,7 +91,9 @@ $: internalGroups = $groups?.filter(g => !g?.scimInfo?.isSync) $: isSSO = !!user?.provider - $: readonly = !sdk.users.isAdmin($auth.user) || user?.scimInfo?.isSync + $: isAdmin = sdk.users.isAdmin($auth.user) + $: isScim = user?.scimInfo?.isSync + $: readonly = !isAdmin || isScim $: privileged = sdk.users.isAdminOrGlobalBuilder(user) $: nameLabel = getNameLabel(user) $: filteredGroups = getFilteredGroups(internalGroups, searchTerm) @@ -322,23 +325,23 @@
Groups - {#if internalGroups?.length} + {#if internalGroups?.length && isAdmin}
+ + addGroup(e.detail)} + on:deselect={e => removeGroup(e.detail)} + iconComponent={GroupIcon} + extractIconProps={item => ({ group: item, size: "S" })} + /> + {/if} - - addGroup(e.detail)} - on:deselect={e => removeGroup(e.detail)} - iconComponent={GroupIcon} - extractIconProps={item => ({ group: item, size: "S" })} - /> -
Date: Mon, 11 Mar 2024 15:16:07 +0100 Subject: [PATCH 02/15] Update pro ref --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 4e66a0f704..71b8e60f7c 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 4e66a0f7042652763c238b10367310b168905f87 +Subproject commit 71b8e60f7c4c80e9711569416450ab8f4a7fa7d1 From 4f5eb6110ac8dedd94efad5516d51750adb0c134 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 11 Mar 2024 16:46:33 +0100 Subject: [PATCH 03/15] Update pro ref --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 71b8e60f7c..e565db07f6 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 71b8e60f7c4c80e9711569416450ab8f4a7fa7d1 +Subproject commit e565db07f6c51868087e88dfebde0328493443e6 From c34fa61479f281e4d21d7438d482135bf23caf4a Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 11 Mar 2024 16:16:26 +0000 Subject: [PATCH 04/15] Bump version to 2.21.7 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 57e3a7b34e..f881568106 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.6", + "version": "2.21.7", "npmClient": "yarn", "packages": [ "packages/*", From 6f2f5fd5ce8eb0233dbc24ed71f4cc5ca7495427 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:38:47 +0100 Subject: [PATCH 05/15] Display AD message correctly for builders --- .../src/pages/builder/portal/users/groups/[groupId].svelte | 2 +- .../portal/users/groups/_components/GroupUsers.svelte | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte index d0eb50c8dd..632419e979 100644 --- a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte +++ b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte @@ -139,7 +139,7 @@ - + diff --git a/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte b/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte index 438feecc07..9f6703964a 100644 --- a/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte +++ b/packages/builder/src/pages/builder/portal/users/groups/_components/GroupUsers.svelte @@ -13,6 +13,7 @@ export let groupId export let readonly + export let isScimGroup let emailSearch let fetchGroupUsers @@ -61,10 +62,10 @@
- {#if !readonly} - - {:else} + {#if isScimGroup} + {:else if !readonly} + {/if}
From 75df04fbda4c6f38269fc202cb5fcd89d331196c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:40:16 +0100 Subject: [PATCH 06/15] Fix group edition display for builders --- .../builder/portal/users/groups/[groupId].svelte | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte index 632419e979..5cb88149d5 100644 --- a/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte +++ b/packages/builder/src/pages/builder/portal/users/groups/[groupId].svelte @@ -49,7 +49,8 @@ $: group = $groups.find(x => x._id === groupId) $: isScimGroup = group?.scimInfo?.isSync - $: readonly = !sdk.users.isAdmin($auth.user) || isScimGroup + $: isAdmin = sdk.users.isAdmin($auth.user) + $: readonly = !isAdmin || isScimGroup $: groupApps = $apps .filter(app => groups.actions @@ -123,14 +124,18 @@ - editModal.show()}> + editModal.show()} + disabled={!isAdmin} + > Edit
deleteModal.show()} - disabled={isScimGroup} + disabled={readonly} > Delete From 4b2c16998ca12004731a1e00493dde669f54ea89 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:51:01 +0100 Subject: [PATCH 07/15] Fix SCIM groups edition --- packages/builder/src/stores/portal/groups.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/stores/portal/groups.js b/packages/builder/src/stores/portal/groups.js index 34fe0f9231..2c2a4e14d0 100644 --- a/packages/builder/src/stores/portal/groups.js +++ b/packages/builder/src/stores/portal/groups.js @@ -35,7 +35,7 @@ export function createGroupsStore() { get: getGroup, save: async group => { - const { _scimInfo, ...dataToSave } = group + const { scimInfo: _, ...dataToSave } = group const response = await API.saveGroup(dataToSave) group._id = response._id group._rev = response._rev From e754c660ee39e220bf45bc3855fd1e74fa21d5c8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:56:36 +0100 Subject: [PATCH 08/15] Fix edition of groups with groups --- packages/builder/src/stores/portal/groups.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/stores/portal/groups.js b/packages/builder/src/stores/portal/groups.js index 2c2a4e14d0..1edc8a461c 100644 --- a/packages/builder/src/stores/portal/groups.js +++ b/packages/builder/src/stores/portal/groups.js @@ -35,7 +35,9 @@ export function createGroupsStore() { get: getGroup, save: async group => { - const { scimInfo: _, ...dataToSave } = group + const { ...dataToSave } = group + delete dataToSave.scimInfo + delete dataToSave.userGroups const response = await API.saveGroup(dataToSave) group._id = response._id group._rev = response._rev From b2000c0805aea14ea6def1b2f5eeac1018a2da98 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 08:56:56 +0100 Subject: [PATCH 09/15] Lint test --- .../src/api/routes/global/tests/groups.spec.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index b69c4781c4..dd39185181 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -103,18 +103,15 @@ describe("/api/global/groups", () => { expect(events.group.updated).toBeCalledTimes(1) expect(events.group.permissionsEdited).not.toBeCalled() }) + }) - describe("destroy", () => { - it("should be able to delete a basic group", async () => { - const group = structures.groups.UserGroup() - let oldGroup = await config.api.groups.saveGroup(group) - await config.api.groups.deleteGroup( - oldGroup.body._id, - oldGroup.body._rev - ) + describe("destroy", () => { + it("should be able to delete a basic group", async () => { + const group = structures.groups.UserGroup() + let oldGroup = await config.api.groups.saveGroup(group) + await config.api.groups.deleteGroup(oldGroup.body._id, oldGroup.body._rev) - expect(events.group.deleted).toBeCalledTimes(1) - }) + expect(events.group.deleted).toBeCalledTimes(1) }) }) From cd0004ec3dc31b5ce97cbad134c94aeff8cb205e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 09:46:10 +0100 Subject: [PATCH 10/15] Add scim tests --- packages/pro | 2 +- .../api/routes/global/tests/groups.spec.ts | 65 +++++++++++++++++++ packages/worker/src/tests/api/groups.ts | 13 +++- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/packages/pro b/packages/pro index e565db07f6..7accd0cb0b 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit e565db07f6c51868087e88dfebde0328493443e6 +Subproject commit 7accd0cb0b21258e9085568143dbf885f9f87afe diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index dd39185181..52df4fa9fd 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -103,6 +103,71 @@ describe("/api/global/groups", () => { expect(events.group.updated).toBeCalledTimes(1) expect(events.group.permissionsEdited).not.toBeCalled() }) + + describe("scim", () => { + async function createScimGroup() { + mocks.licenses.useScimIntegration() + await config.setSCIMConfig(true) + + const scimGroup = await config.api.scimGroupsAPI.post({ + body: structures.scim.createGroupRequest({ + displayName: generator.word(), + }), + }) + + const { body: group } = await config.api.groups.find(scimGroup.id) + + expect(group).toBeDefined() + return group + } + + it("update will not allow sending SCIM fields", async () => { + const group = await createScimGroup() + + const updatedGroup: UserGroup = { + ...group, + name: generator.word(), + } + await config.api.groups.saveGroup(updatedGroup, { + expect: { + message: 'Invalid body - "scimInfo" is not allowed', + status: 400, + }, + }) + + expect(events.group.updated).not.toBeCalled() + }) + + it("update will not amend the SCIM fields", async () => { + const group: UserGroup = await createScimGroup() + + const updatedGroup: UserGroup = { + ...group, + name: generator.word(), + scimInfo: undefined, + } + + await config.api.groups.saveGroup(updatedGroup, { + expect: 200, + }) + + expect(events.group.updated).toBeCalledTimes(1) + expect( + ( + await config.api.groups.find(group._id!, { + expect: 200, + }) + ).body + ).toEqual( + expect.objectContaining({ + ...group, + name: updatedGroup.name, + scimInfo: group.scimInfo, + _rev: expect.any(String), + }) + ) + }) + }) }) describe("destroy", () => { diff --git a/packages/worker/src/tests/api/groups.ts b/packages/worker/src/tests/api/groups.ts index 0b9081cc92..cd95773e87 100644 --- a/packages/worker/src/tests/api/groups.ts +++ b/packages/worker/src/tests/api/groups.ts @@ -7,7 +7,10 @@ export class GroupsAPI extends TestAPI { super(config) } - saveGroup = (group: UserGroup, { expect } = { expect: 200 }) => { + saveGroup = ( + group: UserGroup, + { expect }: { expect: number | object } = { expect: 200 } + ) => { return this.request .post(`/api/global/groups`) .send(group) @@ -61,4 +64,12 @@ export class GroupsAPI extends TestAPI { .expect("Content-Type", /json/) .expect(expect) } + + find = (id: string, { expect } = { expect: 200 }) => { + return this.request + .get(`/api/global/groups/${id}`) + .set(this.config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(expect) + } } From 26c98ea084f61d886694dedafd8e84650ad82231 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 09:57:25 +0100 Subject: [PATCH 11/15] Fix tests --- .../src/api/routes/global/tests/groups.spec.ts | 14 +++++++++----- packages/worker/src/tests/api/groups.ts | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index 52df4fa9fd..0715c3a7bb 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -319,12 +319,16 @@ describe("/api/global/groups", () => { }) }) - it("update should return 200", async () => { + it("update should return forbidden", async () => { await config.withUser(builder, async () => { - await config.api.groups.updateGroupUsers(group._id!, { - add: [builder._id!], - remove: [], - }) + await config.api.groups.updateGroupUsers( + group._id!, + { + add: [builder._id!], + remove: [], + }, + { expect: 403 } + ) }) }) }) diff --git a/packages/worker/src/tests/api/groups.ts b/packages/worker/src/tests/api/groups.ts index cd95773e87..5153c19db0 100644 --- a/packages/worker/src/tests/api/groups.ts +++ b/packages/worker/src/tests/api/groups.ts @@ -47,14 +47,15 @@ export class GroupsAPI extends TestAPI { updateGroupUsers = ( id: string, - body: { add: string[]; remove: string[] } + body: { add: string[]; remove: string[] }, + { expect } = { expect: 200 } ) => { return this.request .post(`/api/global/groups/${id}/users`) .send(body) .set(this.config.defaultHeaders()) .expect("Content-Type", /json/) - .expect(200) + .expect(expect) } fetch = ({ expect } = { expect: 200 }) => { From 3efaf01684d520d2f38c9c82cd73cfb41cd583f8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 10:02:39 +0100 Subject: [PATCH 12/15] Fix multiple runs --- packages/worker/src/api/routes/global/tests/groups.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/worker/src/api/routes/global/tests/groups.spec.ts b/packages/worker/src/api/routes/global/tests/groups.spec.ts index 0715c3a7bb..414518d763 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -209,7 +209,7 @@ describe("/api/global/groups", () => { await Promise.all( Array.from({ length: 30 }).map(async (_, i) => { - const email = `user${i}@example.com` + const email = `user${i}+${generator.guid()}@example.com` const user = await config.api.users.saveUser({ ...structures.users.user(), email, From 106a71b6476e5f3cb601f2f88e07fd076464ca7c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 10:15:58 +0100 Subject: [PATCH 13/15] Update pro ref --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 7accd0cb0b..c4c98ae70f 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 7accd0cb0b21258e9085568143dbf885f9f87afe +Subproject commit c4c98ae70f2e936009250893898ecf11f4ddf2c3 From ae83f637b36acaac4a8b0c036cb2a20fd7e71a42 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 12 Mar 2024 09:35:05 +0000 Subject: [PATCH 14/15] Bump version to 2.21.8 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index f881568106..b845465de5 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.7", + "version": "2.21.8", "npmClient": "yarn", "packages": [ "packages/*", From 3f302d300ec19759513fe1ecbcdb82ac6d4be5f1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 12 Mar 2024 11:09:16 +0100 Subject: [PATCH 15/15] Add test, account holder cannot be removed --- .../src/api/routes/global/tests/scim.spec.ts | 20 +++++++++++++++++++ packages/worker/src/tests/api/scim/shared.ts | 8 ++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/scim.spec.ts b/packages/worker/src/api/routes/global/tests/scim.spec.ts index 3d5a884579..ad43f089db 100644 --- a/packages/worker/src/api/routes/global/tests/scim.spec.ts +++ b/packages/worker/src/api/routes/global/tests/scim.spec.ts @@ -2,6 +2,7 @@ import tk from "timekeeper" import _ from "lodash" import { generator, mocks, structures } from "@budibase/backend-core/tests" import { + CloudAccount, ScimCreateUserRequest, ScimGroupResponse, ScimUpdateRequest, @@ -604,6 +605,25 @@ describe("scim", () => { expect(events.user.deleted).toBeCalledTimes(1) }) + + it("an account holder cannot be removed even when synched", async () => { + const account: CloudAccount = { + ...structures.accounts.account(), + budibaseUserId: user.id, + email: user.emails![0].value, + } + mocks.accounts.getAccount.mockResolvedValue(account) + + await deleteScimUser(user.id, { + expect: { + message: "Account holder cannot be deleted", + status: 400, + error: { code: "http" }, + }, + }) + + await config.api.scimUsersAPI.find(user.id, { expect: 200 }) + }) }) }) diff --git a/packages/worker/src/tests/api/scim/shared.ts b/packages/worker/src/tests/api/scim/shared.ts index 1b064b8f41..546a940093 100644 --- a/packages/worker/src/tests/api/scim/shared.ts +++ b/packages/worker/src/tests/api/scim/shared.ts @@ -1,13 +1,17 @@ import TestConfiguration from "../../TestConfiguration" import { TestAPI } from "../base" -const defaultConfig = { +const defaultConfig: RequestSettings = { expect: 200, setHeaders: true, skipContentTypeCheck: false, } -export type RequestSettings = typeof defaultConfig +export type RequestSettings = { + expect: number | object + setHeaders: boolean + skipContentTypeCheck: boolean +} export abstract class ScimTestAPI extends TestAPI { constructor(config: TestConfiguration) {