Merge pull request #9810 from owncloud/share-and-favorite-permission-…

…checks

feat: add permission checks for creating shares and favorites

.drone.env

@@ -1,3 +1,3 @@
 # The version of OCIS to use in pipelines that test against OCIS
-OCIS_COMMITID=c4d3ec7e5b07abe00ce3d9c322387eba80224ecf
+OCIS_COMMITID=a9366caa0cb97de0c4a68a43f05156cc584bee16
 OCIS_BRANCH=master

changelog/unreleased/enhancement-share-and-favorite-permission-checks

@@ -0,0 +1,6 @@
+Enhancement: Permission checks for shares and favorites
+
+Permission checks for creating shares and favorites have been added.
+
+https://github.com/owncloud/ocis/issues/7497
+https://github.com/owncloud/web/pull/9810

packages/web-app-files/src/components/FilesList/QuickActions.vue

@@ -49,7 +49,7 @@ export default defineComponent({
     const language = useGettext()
 
     const filteredActions = computed(() =>
-      pickBy(props.actions, (action) => action.displayed(props.item, store) === true)
+      pickBy(props.actions, (action) => action.displayed(props.item, store, ability) === true)
     )
 
     return {

packages/web-app-files/src/components/SideBar/Shares/Collaborators/RoleDropdown.vue

@@ -122,6 +122,7 @@ import {
 import * as uuid from 'uuid'
 import { defineComponent, inject, PropType, ComponentPublicInstance } from 'vue'
 import {
+  useAbility,
   useCapabilityFilesSharingAllowCustomPermissions,
   useCapabilityFilesSharingResharingDefault,
   useStore
@@ -166,7 +167,9 @@ export default defineComponent({
   emits: ['optionChange'],
   setup() {
     const store = useStore()
+    const ability = useAbility()
     return {
+      ability,
       resource: inject<Resource>('resource'),
       incomingParentShare: inject<Share>('incomingParentShare'),
       hasRoleCustomPermissions: useCapabilityFilesSharingAllowCustomPermissions(store),
@@ -202,7 +205,7 @@ export default defineComponent({
       return PeopleShareRoles.custom(this.resource.isFolder)
     },
     resourceIsSharable() {
-      return this.allowSharePermission && this.resource.canShare()
+      return this.allowSharePermission && this.resource.canShare({ ability: this.ability })
     },
     availableRoles() {
       if (this.resourceIsSpace) {

packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue

@@ -158,7 +158,8 @@ export default defineComponent({
   },
   setup() {
     const store = useStore()
-    const { can } = useAbility()
+    const ability = useAbility()
+    const { can } = ability
     const passwordPolicyService = usePasswordPolicyService()
     const hasResharing = useCapabilityFilesSharingResharing()
 
@@ -206,7 +207,7 @@ export default defineComponent({
         return false
       }
 
-      return unref(resource).canShare({ user: store.getters.user })
+      return unref(resource).canShare({ user: store.getters.user, ability })
     })
 
     const canEditLink = ({ permissions }: Share) => {

packages/web-app-files/src/components/SideBar/Shares/FileShares.vue

@@ -89,6 +89,7 @@
 <script lang="ts">
 import { mapGetters, mapActions, mapState, mapMutations } from 'vuex'
 import {
+  useAbility,
   useStore,
   useCapabilityProjectSpacesEnabled,
   useCapabilityShareJailEnabled,
@@ -125,6 +126,7 @@ export default defineComponent({
   },
   setup() {
     const store = useStore()
+    const ability = useAbility()
     const { getMatchingSpace } = useGetMatchingSpace()
 
     const resource = inject<Ref<Resource>>('resource')
@@ -164,6 +166,7 @@ export default defineComponent({
 
     return {
       ...useShares(),
+      ability,
       resource,
       space: inject<Ref<SpaceResource>>('space'),
       matchingSpace,
@@ -270,7 +273,7 @@ export default defineComponent({
       if (isShareJail && !this.hasResharing) {
         return false
       }
-      return this.resource.canShare({ user: this.user })
+      return this.resource.canShare({ user: this.user, ability: this.ability })
     },
 
     noResharePermsMessage() {

packages/web-app-files/src/index.ts

@@ -76,7 +76,7 @@ export const navItems = (context): AppNavigationItem[] => {
         path: `/${appInfo.id}/favorites`
       },
       enabled(capabilities) {
-        return capabilities.files?.favorites
+        return capabilities.files?.favorites && context.$ability.can('read-all', 'Favorite')
       },
       priority: 20
     },

packages/web-client/src/helpers/resource/functions.ts

@@ -155,8 +155,10 @@ export function buildResource(resource: WebDavResponseResource): Resource {
     canRename: function () {
       return this.permissions.indexOf(DavPermission.Renameable) >= 0
     },
-    canShare: function () {
-      return this.permissions.indexOf(DavPermission.Shareable) >= 0
+    canShare: function ({ ability }) {
+      return (
+        ability.can('create-all', 'Share') && this.permissions.indexOf(DavPermission.Shareable) >= 0
+      )
     },
     canCreate: function () {
       return this.permissions.indexOf(DavPermission.FolderCreateable) >= 0
@@ -182,7 +184,7 @@ export function buildResource(resource: WebDavResponseResource): Resource {
       return this.permissions.indexOf(DavPermission.Deny) >= 0
     },
     getDomSelector: () => extractDomSelector(id)
-  }
+  } satisfies Resource
   Object.defineProperty(r, 'nodeId', {
     get() {
       return extractNodeId(this.id)

packages/web-client/src/helpers/resource/types.ts

@@ -18,13 +18,15 @@ export type AbilityActions =
 export type AbilitySubjects =
   | 'Account'
   | 'Drive'
+  | 'Favorite'
   | 'Group'
   | 'Language'
   | 'Logo'
   | 'PublicLink'
   | 'ReadOnlyPublicLinkPassword'
   | 'Role'
   | 'Setting'
+  | 'Share'
 
 export type Ability = MongoAbility<[AbilityActions, AbilitySubjects]>
 export type AbilityRule = SubjectRawRule<AbilityActions, AbilitySubjects, any>
@@ -84,7 +86,7 @@ export interface Resource {
   canCreate?(): boolean
   canUpload?({ user }: { user?: User }): boolean
   canDownload?(): boolean
-  canShare?({ user }?: { user?: User }): boolean
+  canShare?({ user, ability }?: { user?: User; ability?: Ability }): boolean
   canRename?({ user }?: { user?: User; ability?: Ability }): boolean
   canBeDeleted?({ user }?: { user?: User; ability?: Ability }): boolean
   canBeRestored?(): boolean

packages/web-client/tests/unit/helpers/resource/resource.spec.ts → packages/web-client/tests/unit/helpers/resource/functions.spec.ts

@@ -1,9 +1,14 @@
+import { mock, mockDeep } from 'jest-mock-extended'
 import {
+  Ability,
   Resource,
+  WebDavResponseResource,
+  buildResource,
   extractDomSelector,
   extractExtensionFromFile,
   extractNameWithoutExtension
 } from '../../../../src/helpers'
+import { DavPermission, DavProperty } from '../../../../src/webdav/constants'
 
 describe('extractDomSelector', () => {
   it.each([
@@ -75,3 +80,47 @@ describe('filterResources', () => {
     })
   })
 })
+
+describe('buildResource', () => {
+  describe('canShare', () => {
+    it('is true when ability and share permissions are given', () => {
+      const webDavResponse = mockDeep<WebDavResponseResource>({
+        props: {
+          [DavProperty.Permissions]: DavPermission.Shareable,
+          [DavProperty.Tags]: undefined
+        }
+      })
+      const resource = buildResource(webDavResponse)
+      const ability = mock<Ability>()
+      ability.can.mockReturnValue(true)
+      expect(resource.canShare({ ability })).toBeTruthy()
+      expect(ability.can).toHaveBeenCalledWith('create-all', 'Share')
+    })
+    it('is false when ability is not given', () => {
+      const webDavResponse = mockDeep<WebDavResponseResource>({
+        props: {
+          [DavProperty.Permissions]: DavPermission.Shareable,
+          [DavProperty.Tags]: undefined
+        }
+      })
+      const resource = buildResource(webDavResponse)
+      const ability = mock<Ability>()
+      ability.can.mockReturnValue(false)
+      expect(resource.canShare({ ability })).toBeFalsy()
+      expect(ability.can).toHaveBeenCalledWith('create-all', 'Share')
+    })
+    it('is false when share permissions are not given', () => {
+      const webDavResponse = mockDeep<WebDavResponseResource>({
+        props: {
+          [DavProperty.Permissions]: '',
+          [DavProperty.Tags]: undefined
+        }
+      })
+      const resource = buildResource(webDavResponse)
+      const ability = mock<Ability>()
+      ability.can.mockReturnValue(true)
+      expect(resource.canShare({ ability })).toBeFalsy()
+      expect(ability.can).toHaveBeenCalledWith('create-all', 'Share')
+    })
+  })
+})

packages/web-pkg/src/components/AppBar/AppBar.vue

@@ -100,6 +100,7 @@ import {
   useFileActionsRestore
 } from '../../composables/actions'
 import {
+  useAbility,
   useCapabilitySpacesMaxQuota,
   useFileActionsToggleHideShare,
   useRouteMeta,
@@ -165,6 +166,7 @@ export default defineComponent({
   setup(props, { emit }) {
     const store = useStore()
     const { $gettext } = useGettext()
+    const { can } = useAbility()
 
     const { actions: acceptShareActions } = useFileActionsAcceptShare({ store })
     const { actions: hideShareActions } = useFileActionsToggleHideShare({ store })
@@ -188,7 +190,9 @@ export default defineComponent({
     const breadcrumbMaxWidth = ref<number>(0)
     const isSearchLocation = useActiveLocation(isLocationCommonActive, 'files-common-search')
 
-    const hasSharesNavigation = computed(() => useSlots().hasOwnProperty('navigation'))
+    const hasSharesNavigation = computed(
+      () => useSlots().hasOwnProperty('navigation') && can('create-all', 'Share')
+    )
 
     const batchActions = computed(() => {
       let actions = [

packages/web-pkg/src/composables/actions/files/useFileActionsCreateQuicklink.ts

@@ -56,7 +56,7 @@ export const useFileActionsCreateQuickLink = ({
             return false
           }
         }
-        return canShare(resources[0], store)
+        return canShare(resources[0], store, ability)
       },
       componentType: 'button',
       class: 'oc-files-actions-create-quicklink-trigger'

packages/web-pkg/src/composables/actions/files/useFileActionsFavorite.ts

@@ -8,6 +8,7 @@ import { FileAction, FileActionOptions, useIsFilesAppActive } from '../../action
 import { useStore } from '../../store'
 import { useRouter } from '../../router'
 import { useClientService } from '../../clientService'
+import { useAbility } from '../../ability'
 
 export const useFileActionsFavorite = ({ store }: { store?: Store<any> } = {}) => {
   store = store || useStore()
@@ -16,6 +17,8 @@ export const useFileActionsFavorite = ({ store }: { store?: Store<any> } = {}) =
   const clientService = useClientService()
   const hasFavorites = useCapabilityFilesFavorites()
   const isFilesAppActive = useIsFilesAppActive()
+  const ability = useAbility()
+
   const handler = ({ resources }: FileActionOptions) => {
     store
       .dispatch('Files/markFavorite', {
@@ -58,7 +61,7 @@ export const useFileActionsFavorite = ({ store }: { store?: Store<any> } = {}) =
           return false
         }
 
-        return unref(hasFavorites)
+        return unref(hasFavorites) && ability.can('create-all', 'Favorite')
       },
       componentType: 'button',
       class: 'oc-files-actions-favorite-trigger'

packages/web-pkg/src/composables/actions/files/useFileActionsShowShares.ts

@@ -10,10 +10,12 @@ import { useRouter } from '../../router'
 import { useStore } from '../../store'
 import { Store } from 'vuex'
 import { FileAction, FileActionOptions } from '../types'
+import { useAbility } from '../../ability'
 
 export const useFileActionsShowShares = ({ store }: { store?: Store<any> } = {}) => {
   store = store || useStore()
   const router = useRouter()
+  const ability = useAbility()
   const { $gettext } = useGettext()
   const isFilesAppActive = useIsFilesAppActive()
 
@@ -46,7 +48,7 @@ export const useFileActionsShowShares = ({ store }: { store?: Store<any> } = {})
             return false
           }
         }
-        return canShare(resources[0], store)
+        return canShare(resources[0], store, ability)
       },
       componentType: 'button',
       class: 'oc-files-actions-show-shares-trigger'

packages/web-pkg/src/quickActions.ts

@@ -8,15 +8,15 @@ import { Ability } from '@ownclouders/web-client/src/helpers/resource/types'
 import { Store } from 'vuex'
 import { ApplicationQuickActions } from './apps'
 
-export function canShare(item, store) {
+export function canShare(item: Resource, store: Store<any>, ability: Ability) {
   const { capabilities } = store.state.user
   if (!capabilities.files_sharing || !capabilities.files_sharing.api_enabled) {
     return false
   }
   if (item.isReceivedShare() && !capabilities.files_sharing.resharing) {
     return false
   }
-  return item.canShare()
+  return item.canShare({ ability })
 }
 
 export function showQuickLinkPasswordModal({ $gettext, store, passwordPolicyService }, onConfirm) {

packages/web-runtime/src/services/auth/abilities.ts

@@ -11,6 +11,11 @@ export const getAbilities = (
       { action: 'read-all', subject: 'Account' },
       { action: 'update-all', subject: 'Account' }
     ],
+    'Favorites.List.all': [{ action: 'read-all', subject: 'Favorite' }],
+    'Favorites.Write.all': [
+      { action: 'create-all', subject: 'Favorite' },
+      { action: 'update-all', subject: 'Favorite' }
+    ],
     'Groups.ReadWrite.all': [
       { action: 'create-all', subject: 'Group' },
       { action: 'delete-all', subject: 'Group' },
@@ -32,6 +37,10 @@ export const getAbilities = (
       { action: 'read-all', subject: 'Role' },
       { action: 'update-all', subject: 'Role' }
     ],
+    'Shares.Write.all': [
+      { action: 'create-all', subject: 'Share' },
+      { action: 'update-all', subject: 'Share' }
+    ],
     'Settings.ReadWrite.all': [
       { action: 'read-all', subject: 'Setting' },
       { action: 'update-all', subject: 'Setting' }

packages/web-runtime/tests/unit/services/auth/abilities.spec.ts

@@ -15,6 +15,15 @@ describe('getAbilities', () => {
     const expectedActions = ['create-all', 'delete-all', 'read-all', 'update-all']
     expect(abilities).toEqual(expectedActions.map((action) => ({ action, subject: 'Group' })))
   })
+  it.each([
+    { permissions: [''], expectedActions: [] },
+    { permissions: ['Favorites.List.all'], expectedActions: ['read-all'] },
+    { permissions: ['Favorites.Write.all'], expectedActions: ['create-all', 'update-all'] }
+  ])('gets correct abilities for subject "Favorites"', function (data) {
+    const abilities = getAbilities(data.permissions)
+    const expectedResult = data.expectedActions.map((action) => ({ action, subject: 'Favorite' }))
+    expect(abilities).toEqual(expectedResult)
+  })
   it('gets correct abilities for subject "Language"', function () {
     const abilities = getAbilities(['Language.ReadWrite.all'])
     const expectedActions = ['read-all', 'update-all']
@@ -35,6 +44,11 @@ describe('getAbilities', () => {
     const expectedActions = ['create-all']
     expect(abilities).toEqual(expectedActions.map((action) => ({ action, subject: 'PublicLink' })))
   })
+  it('gets correct abilities for subject "Share"', function () {
+    const abilities = getAbilities(['Shares.Write.all'])
+    const expectedActions = ['create-all', 'update-all']
+    expect(abilities).toEqual(expectedActions.map((action) => ({ action, subject: 'Share' })))
+  })
   it('gets correct abilities for subject "Setting"', function () {
     const abilities = getAbilities(['Settings.ReadWrite.all'])
     const expectedActions = ['read-all', 'update-all']