diff --git a/lerna.json b/lerna.json index 57e3a7b34e..b845465de5 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.21.6", + "version": "2.21.8", "npmClient": "yarn", "packages": [ "packages/*", 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..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 @@ -139,7 +144,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}
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" })} - /> -
{ - 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 diff --git a/packages/pro b/packages/pro index 4e66a0f704..c4c98ae70f 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 4e66a0f7042652763c238b10367310b168905f87 +Subproject commit c4c98ae70f2e936009250893898ecf11f4ddf2c3 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..414518d763 100644 --- a/packages/worker/src/api/routes/global/tests/groups.spec.ts +++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts @@ -104,17 +104,79 @@ describe("/api/global/groups", () => { 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("scim", () => { + async function createScimGroup() { + mocks.licenses.useScimIntegration() + await config.setSCIMConfig(true) - expect(events.group.deleted).toBeCalledTimes(1) + 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", () => { + 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) }) }) @@ -147,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, @@ -257,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/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/groups.ts b/packages/worker/src/tests/api/groups.ts index 0b9081cc92..5153c19db0 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) @@ -44,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 }) => { @@ -61,4 +65,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) + } } 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) {