[full-ci] Implement permission concept for handling user abilities (#…

…8431) * Indroduce CASL for handling permissions * Add CASL to the default test plugins * Use the ability composable in the projects view * Mock resize observer globally * Remove permissionManager * Fetch permissions, update ACLs * Add permission check for logo mixins * Adjust remaining permissions, remove role checks * Improve naming on ability subject and action types * Fix unit tests * Add unit tests for getAbilities * Refactor getAbilities method to improve readability * Add missing type * Fix typo * Adjust ability injection variable name * Add support for contraints * Bump ocis commit id for Drone * Add changelog items * Adjust ability injection variable name * Add permission check for groups in default admin-settings route
owncloud · Feb 17, 2023 · af53a16 · af53a16
1 parent 07c20e5
commit af53a16
Show file tree

Hide file tree

Showing 44 changed files with 412 additions and 440 deletions.
diff --git a/.drone.env b/.drone.env
@@ -1,3 +1,3 @@
 # The version of OCIS to use in pipelines that test against OCIS
-OCIS_COMMITID=3e3d3f9de2919368f5849061e9c3ea6894c6457c
+OCIS_COMMITID=359509dcfe0bd8bf23c71c7aee5fdd27faa7675a
 OCIS_BRANCH=master
diff --git a/changelog/unreleased/change-remove-permission-manager b/changelog/unreleased/change-remove-permission-manager
@@ -0,0 +1,5 @@
+Change: Remove permission manager
+
+BREAKING CHANGE for developers: The `PermissionManager` has been removed. Permission management is now being handled by `CASL`. For more details on how it works please see the linked PR down below.
+
+https://github.com/owncloud/web/pull/8431
diff --git a/packages/web-app-admin-settings/src/components/General/AppearanceSection.vue b/packages/web-app-admin-settings/src/components/General/AppearanceSection.vue
@@ -5,6 +5,7 @@
       <div class="oc-flex oc-flex-middle">
         <h3 v-text="$gettext('Logo')" />
         <oc-button
+          v-if="menuItems.length"
           :id="`logo-context-btn`"
           v-oc-tooltip="$gettext('Show context menu')"
           :aria-label="$gettext('Show context menu')"
@@ -59,10 +60,9 @@ export default defineComponent({
     const store = useStore()
     const instance = getCurrentInstance().proxy as any
 
-    const menuItems = computed(() => [
-      ...instance.$_uploadLogo_items,
-      ...instance.$_resetLogo_items
-    ])
+    const menuItems = computed(() =>
+      [...instance.$_uploadLogo_items, ...instance.$_resetLogo_items].filter((i) => i.isEnabled())
+    )
     const logo = computed(() => store.getters.configuration.currentTheme.logo.topbar)
     const menuSections = computed(() => [
       {

diff --git a/packages/web-app-admin-settings/src/index.ts b/packages/web-app-admin-settings/src/index.ts
@@ -3,6 +3,8 @@ import General from './views/General.vue'
 import Users from './views/Users.vue'
 import Groups from './views/Groups.vue'
 import Spaces from './views/Spaces.vue'
+import { Ability } from 'web-pkg'
+
 // just a dummy function to trick gettext tools
 function $gettext(msg) {
   return msg
@@ -15,14 +17,23 @@ const appInfo = {
   isFileEditor: false
 }
 
-const routes = ({ $permissionManager }) => [
+const routes = ({ $ability }: { $ability: Ability }) => [
   {
     path: '/',
     redirect: () => {
-      if ($permissionManager.hasSystemManagement()) {
+      if ($ability.can('read-all', 'Setting')) {
         return { name: 'admin-settings-general' }
       }
-      return { name: 'admin-settings-spaces' }
+      if ($ability.can('read-all', 'Account')) {
+        return { name: 'admin-settings-users' }
+      }
+      if ($ability.can('read-all', 'Group')) {
+        return { name: 'admin-settings-groups' }
+      }
+      if ($ability.can('read-all', 'Space')) {
+        return { name: 'admin-settings-spaces' }
+      }
+      throw Error('Insufficient permissions')
     }
   },
   {
@@ -63,15 +74,15 @@ const routes = ({ $permissionManager }) => [
   }
 ]
 
-const navItems = ({ $permissionManager }) => [
+const navItems = ({ $ability }: { $ability: Ability }) => [
   {
     name: $gettext('General'),
     icon: 'settings-4',
     route: {
       path: `/${appInfo.id}/general?`
     },
     enabled: () => {
-      return $permissionManager.hasSystemManagement()
+      return $ability.can('read-all', 'Setting')
     }
   },
   {
@@ -81,7 +92,7 @@ const navItems = ({ $permissionManager }) => [
       path: `/${appInfo.id}/users?`
     },
     enabled: () => {
-      return $permissionManager.hasUserManagement()
+      return $ability.can('read-all', 'Account')
     }
   },
   {
@@ -91,7 +102,7 @@ const navItems = ({ $permissionManager }) => [
       path: `/${appInfo.id}/groups?`
     },
     enabled: () => {
-      return $permissionManager.hasUserManagement()
+      return $ability.can('read-all', 'Group')
     }
   },
   {
@@ -101,7 +112,7 @@ const navItems = ({ $permissionManager }) => [
       path: `/${appInfo.id}/spaces?`
     },
     enabled: () => {
-      return $permissionManager.hasSpaceManagement()
+      return $ability.can('read-all', 'Space')
     }
   }
 ]

diff --git a/packages/web-app-admin-settings/src/mixins/general/resetLogo.ts b/packages/web-app-admin-settings/src/mixins/general/resetLogo.ts
@@ -11,6 +11,9 @@ export default {
           label: () => {
             return this.$gettext('Reset logo')
           },
+          isEnabled: () => {
+            return this.$can('update-all', 'Logo')
+          },
           handler: this.$_resetLogo_trigger,
           componentType: 'button',
           class: 'oc-general-actions-reset-logo-trigger'

diff --git a/packages/web-app-admin-settings/src/mixins/general/uploadLogo.ts b/packages/web-app-admin-settings/src/mixins/general/uploadLogo.ts
@@ -12,6 +12,9 @@ export default {
           label: () => {
             return this.$gettext('Upload logo')
           },
+          isEnabled: () => {
+            return this.$can('update-all', 'Logo')
+          },
           handler: this.$_uploadLogo_trigger,
           componentType: 'button',
           class: 'oc-general-actions-upload-logo-trigger'

diff --git a/packages/web-app-admin-settings/tests/unit/index.spec.ts b/packages/web-app-admin-settings/tests/unit/index.spec.ts
@@ -1,46 +1,46 @@
 import index from '../../src/index'
+import { Ability } from 'web-pkg'
+import { mock } from 'jest-mock-extended'
+
+const getAbilityMock = (hasPermission) => mock<Ability>({ can: () => hasPermission })
 
 describe('admin settings index', () => {
   describe('navItems', () => {
     describe('general', () => {
       it.each([true, false])('should be enabled according to the permissions', (enabled) => {
-        const $permissionManager = { hasSystemManagement: () => enabled }
         expect(
           index
-            .navItems({ $permissionManager })
+            .navItems({ $ability: getAbilityMock(enabled) })
             .find((n) => n.name === 'General')
             .enabled()
         ).toBe(enabled)
       })
     })
     describe('user management', () => {
       it.each([true, false])('should be enabled according to the permissions', (enabled) => {
-        const $permissionManager = { hasUserManagement: () => enabled }
         expect(
           index
-            .navItems({ $permissionManager })
+            .navItems({ $ability: getAbilityMock(enabled) })
             .find((n) => n.name === 'Users')
             .enabled()
         ).toBe(enabled)
       })
     })
     describe('group management', () => {
       it.each([true, false])('should be enabled according to the permissions', (enabled) => {
-        const $permissionManager = { hasUserManagement: () => enabled }
         expect(
           index
-            .navItems({ $permissionManager })
+            .navItems({ $ability: getAbilityMock(enabled) })
             .find((n) => n.name === 'Groups')
             .enabled()
         ).toBe(enabled)
       })
     })
     describe('space management', () => {
       it.each([true, false])('should be enabled according to the permissions', (enabled) => {
-        const $permissionManager = { hasSpaceManagement: () => enabled }
         expect(
           index
-            .navItems({ $permissionManager })
+            .navItems({ $ability: getAbilityMock(enabled) })
             .find((n) => n.name === 'Spaces')
             .enabled()
         ).toBe(enabled)
@@ -50,23 +50,56 @@ describe('admin settings index', () => {
   describe('routes', () => {
     describe('default-route "/"', () => {
       it('should redirect to general if permission given', () => {
-        const $permissionManager = { hasSystemManagement: () => true }
+        const ability = mock<Ability>()
+        ability.can.mockReturnValueOnce(true)
         expect(
           index
-            .routes({ $permissionManager })
+            .routes({ $ability: ability })
             .find((n) => n.path === '/')
             .redirect().name
         ).toEqual('admin-settings-general')
       })
-      it('should redirect to space management if no system management permission given', () => {
-        const $permissionManager = { hasSystemManagement: () => false }
+      it('should redirect to user management if permission given', () => {
+        const ability = mock<Ability>()
+        ability.can.mockReturnValueOnce(false)
+        ability.can.mockReturnValueOnce(true)
+        expect(
+          index
+            .routes({ $ability: ability })
+            .find((n) => n.path === '/')
+            .redirect().name
+        ).toEqual('admin-settings-users')
+      })
+      it('should redirect to group management if permission given', () => {
+        const ability = mock<Ability>()
+        ability.can.mockReturnValueOnce(false)
+        ability.can.mockReturnValueOnce(false)
+        ability.can.mockReturnValueOnce(true)
         expect(
           index
-            .routes({ $permissionManager })
+            .routes({ $ability: ability })
+            .find((n) => n.path === '/')
+            .redirect().name
+        ).toEqual('admin-settings-groups')
+      })
+      it('should redirect to space management if permission given', () => {
+        const ability = mock<Ability>()
+        ability.can.mockReturnValueOnce(false)
+        ability.can.mockReturnValueOnce(false)
+        ability.can.mockReturnValueOnce(false)
+        ability.can.mockReturnValueOnce(true)
+        expect(
+          index
+            .routes({ $ability: ability })
             .find((n) => n.path === '/')
             .redirect().name
         ).toEqual('admin-settings-spaces')
       })
+      it('should throw an error if permissions are insufficient', () => {
+        const ability = mock<Ability>()
+        ability.can.mockReturnValue(false)
+        expect(index.routes({ $ability: ability }).find((n) => n.path === '/').redirect).toThrow()
+      })
     })
   })
 })
diff --git a/packages/web-app-admin-settings/tests/unit/views/Spaces.spec.ts b/packages/web-app-admin-settings/tests/unit/views/Spaces.spec.ts
@@ -12,14 +12,6 @@ import {
 } from 'web-test-helpers'
 import Spaces from '../../../src/views/Spaces.vue'
 
-window.ResizeObserver =
-  window.ResizeObserver ||
-  jest.fn().mockImplementation(() => ({
-    disconnect: jest.fn(),
-    observe: jest.fn(),
-    unobserve: jest.fn()
-  }))
-
 const selectors = {
   loadingSpinnerStub: 'app-loading-spinner-stub',
   spacesListStub: 'spaces-list-stub',

diff --git a/packages/web-app-files/src/views/spaces/Projects.vue b/packages/web-app-files/src/views/spaces/Projects.vue
@@ -73,7 +73,7 @@ import AppLoadingSpinner from 'web-pkg/src/components/AppLoadingSpinner.vue'
 
 import AppBar from '../../components/AppBar/AppBar.vue'
 import CreateSpace from '../../components/AppBar/CreateSpace.vue'
-import { useAccessToken, useStore, useGraphClient } from 'web-pkg/src/composables'
+import { useAbility, useAccessToken, useStore, useGraphClient } from 'web-pkg/src/composables'
 import { loadPreview } from 'web-pkg/src/helpers/preview'
 import { ImageDimension } from 'web-pkg/src/constants'
 import SpaceContextActions from '../../components/Spaces/SpaceContextActions.vue'
@@ -103,6 +103,7 @@ export default defineComponent({
   setup() {
     const store = useStore()
     const { selectedResourcesIds } = useSelectedResources({ store })
+    const { can } = useAbility()
 
     const runtimeSpaces = computed(
       () => store.getters['runtime/spaces/spaces'].filter((s) => isProjectSpaceResource(s)) || []
@@ -135,6 +136,8 @@ export default defineComponent({
       return loadResourcesTask.isRunning || !loadResourcesTask.last
     })
 
+    const hasCreatePermission = computed(() => can('create-all', 'Space'))
+
     return {
       ...useSideBar(),
       ...useScrollTo(),
@@ -147,7 +150,8 @@ export default defineComponent({
       handleSort,
       sortBy,
       sortDir,
-      sortFields
+      sortFields,
+      hasCreatePermission
     }
   },
   data: function () {
@@ -168,9 +172,6 @@ export default defineComponent({
     },
     showSpaceMemberLabel() {
       return this.$gettext('Show members')
-    },
-    hasCreatePermission() {
-      return this.$permissionManager.hasSpaceManagement()
     }
   },
   watch: {

diff --git a/packages/web-app-files/tests/unit/components/AppBar/AppBar.spec.ts b/packages/web-app-files/tests/unit/components/AppBar/AppBar.spec.ts
@@ -39,14 +39,6 @@ const breadCrumbItemWithContextActionAllowed = {
   allowContextActions: true
 }
 
-window.ResizeObserver =
-  window.ResizeObserver ||
-  jest.fn().mockImplementation(() => ({
-    disconnect: jest.fn(),
-    observe: jest.fn(),
-    unobserve: jest.fn()
-  }))
-
 describe('AppBar component', () => {
   describe('renders', () => {
     it('by default no breadcrumbs, no bulkactions, no sharesnavigation but viewoptions and sidebartoggle', () => {

diff --git a/packages/web-app-files/tests/unit/components/Spaces/SpaceContextActions.spec.ts b/packages/web-app-files/tests/unit/components/Spaces/SpaceContextActions.spec.ts
@@ -35,9 +35,7 @@ function getWrapper(space) {
       global: {
         mocks: {
           ...defaultComponentMocks({ currentRoute: mock<RouteLocation>({ path: '/files' }) }),
-          $permissionManager: {
-            canEditSpaceQuota: () => true
-          }
+          $can: () => true
         },
         plugins: [...defaultPlugins(), store]
       }

diff --git a/packages/web-app-files/tests/unit/components/Spaces/SpaceHeader.spec.ts b/packages/web-app-files/tests/unit/components/Spaces/SpaceHeader.spec.ts
@@ -8,16 +8,6 @@ import {
   defaultComponentMocks
 } from 'web-test-helpers'
 
-window.ResizeObserver =
-  window.ResizeObserver ||
-  jest.fn().mockImplementation(() => ({
-    disconnect: jest.fn(),
-    observe: jest.fn(),
-    unobserve: jest.fn()
-  }))
-
-afterEach(() => jest.clearAllMocks())
-
 describe('SpaceHeader', () => {
   it('should add the "squashed"-class when the sidebar is opened', () => {
     const wrapper = getWrapper({ space: buildSpace({ id: 1 }), sideBarOpen: true })
@@ -42,12 +32,7 @@ describe('SpaceHeader', () => {
 })
 
 function getWrapper({ space = {}, sideBarOpen = false }) {
-  const mocks = {
-    ...defaultComponentMocks(),
-    $permissionManager: {
-      canEditSpaceQuota: () => true
-    }
-  }
+  const mocks = defaultComponentMocks()
   const store = createStore(defaultStoreMockOptions)
   return mount(SpaceHeader, {
     props: {
@@ -58,7 +43,8 @@ function getWrapper({ space = {}, sideBarOpen = false }) {
       mocks,
       plugins: [...defaultPlugins(), store],
       stubs: {
-        'quota-modal': true
+        'quota-modal': true,
+        'space-context-actions': true
       }
     }
   })