Merge branch 'master' into cheeks-lab-day-binding-eval

2024-03-12 13:27:03 +00:00 · 2024-03-12 13:27:03 +00:00 · 30b7797421
parent fe12a86064 efe07cef5d
commit 30b7797421
10 changed files with 159 additions and 46 deletions
--- a/lerna.json
+++ b/lerna.json
@ -1,5 +1,5 @@
 {
-  "version": "2.21.6",
+  "version": "2.21.8",
  "npmClient": "yarn",
  "packages": [
    "packages/*",
--- 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 @@
        <span slot="control">
          <Icon hoverable name="More" />
        </span>
-        <MenuItem icon="Refresh" on:click={() => editModal.show()}>
+        <MenuItem
+          icon="Refresh"
+          on:click={() => editModal.show()}
+          disabled={!isAdmin}
+        >
          Edit
        </MenuItem>
        <div title={isScimGroup && "Group synced from your AD"}>
          <MenuItem
            icon="Delete"
            on:click={() => deleteModal.show()}
-            disabled={isScimGroup}
+            disabled={readonly}
          >
            Delete
          </MenuItem>
@ -139,7 +144,7 @@
    </div>

    <Layout noPadding gap="S">
-      <GroupUsers {groupId} {readonly} />
+      <GroupUsers {groupId} {readonly} {isScimGroup} />
    </Layout>

    <Layout noPadding gap="S">
--- 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 @@
 </script>

 <div class="header">
-  {#if !readonly}
-    <EditUserPicker {groupId} onUsersUpdated={fetchGroupUsers.getInitialData} />
-  {:else}
+  {#if isScimGroup}
    <ActiveDirectoryInfo text="Users synced from your AD" />
+  {:else if !readonly}
+    <EditUserPicker {groupId} onUsersUpdated={fetchGroupUsers.getInitialData} />
  {/if}

  <div class="controls-right">
--- 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,11 +325,10 @@
      <Layout gap="S" noPadding>
        <div class="tableTitle">
          <Heading size="S">Groups</Heading>
-          {#if internalGroups?.length}
+          {#if internalGroups?.length && isAdmin}
            <div bind:this={popoverAnchor}>
              <Button on:click={popover.show()} secondary>Add to group</Button>
            </div>
-          {/if}
            <Popover align="right" bind:this={popover} anchor={popoverAnchor}>
              <UserGroupPicker
                labelKey="name"
@ -339,6 +341,7 @@
                extractIconProps={item => ({ group: item, size: "S" })}
              />
            </Popover>
+          {/if}
        </div>
        <Table
          schema={groupSchema}
--- 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
--- a/packages/pro
+++ b/packages/pro
@ -1 +1 @@
-Subproject commit 4e66a0f7042652763c238b10367310b168905f87
+Subproject commit c4c98ae70f2e936009250893898ecf11f4ddf2c3
--- a/packages/worker/src/api/routes/global/tests/groups.spec.ts
+++ b/packages/worker/src/api/routes/global/tests/groups.spec.ts
@ -104,19 +104,81 @@ describe("/api/global/groups", () => {
      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", () => {
    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
-        )
+      await config.api.groups.deleteGroup(oldGroup.body._id, oldGroup.body._rev)

      expect(events.group.deleted).toBeCalledTimes(1)
    })
  })
-  })

  describe("find users", () => {
    describe("without users", () => {
@ -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!, {
+        await config.api.groups.updateGroupUsers(
+          group._id!,
+          {
            add: [builder._id!],
            remove: [],
-        })
+          },
+          { expect: 403 }
+        )
      })
    })
  })
--- 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 })
+      })
    })
  })

--- 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)
+  }
 }
--- 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) {