From c6164bba36f396ad9131b525310926348096c920 Mon Sep 17 00:00:00 2001 From: Cleopatra Enjeck M Date: Tue, 19 Mar 2024 13:34:58 +0100 Subject: [PATCH 1/3] feat: transfer context ownership Signed-off-by: Cleopatra Enjeck M --- src/modules/modals/Modals.vue | 6 ++ src/modules/modals/TransferContext.vue | 89 +++++++++++++++++++ .../partials/NavigationContextItem.vue | 11 +++ .../NcUserAndGroupPicker.vue | 1 - src/store/store.js | 15 ++++ 5 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 src/modules/modals/TransferContext.vue diff --git a/src/modules/modals/Modals.vue b/src/modules/modals/Modals.vue index 718097eca..14ca45c32 100644 --- a/src/modules/modals/Modals.vue +++ b/src/modules/modals/Modals.vue @@ -39,6 +39,7 @@ + @@ -60,6 +61,7 @@ import EditTable from './EditTable.vue' import EditContext from './EditContext.vue' import TransferTable from './TransferTable.vue' import CreateContext from './CreateContext.vue' +import TransferContext from './TransferContext.vue' export default { components: { @@ -78,6 +80,7 @@ export default { TransferTable, CreateContext, EditContext, + TransferContext, }, data() { @@ -98,6 +101,7 @@ export default { editTable: null, editContext: null, tableToTransfer: null, + contextToTransfer: null, } }, @@ -138,6 +142,7 @@ export default { // context subscribe('tables:context:create', () => { this.showModalCreateContext = true }) subscribe('tables:context:edit', contextId => { this.editContext = contextId }) + subscribe('tables:context:transfer', context => { this.contextToTransfer = context }) }, unmounted() { @@ -165,6 +170,7 @@ export default { unsubscribe('tables:table:transfer', table => { this.tableToTransfer = table }) unsubscribe('tables:context:create', () => { this.showModalCreateContext = true }) unsubscribe('tables:context:edit', contextId => { this.editContext = contextId }) + unsubscribe('tables:context:transfer', context => { this.contextToTransfer = context }) }, } diff --git a/src/modules/modals/TransferContext.vue b/src/modules/modals/TransferContext.vue new file mode 100644 index 000000000..a3028aadb --- /dev/null +++ b/src/modules/modals/TransferContext.vue @@ -0,0 +1,89 @@ + + + diff --git a/src/modules/navigation/partials/NavigationContextItem.vue b/src/modules/navigation/partials/NavigationContextItem.vue index 6170351c2..13ddd86a3 100644 --- a/src/modules/navigation/partials/NavigationContextItem.vue +++ b/src/modules/navigation/partials/NavigationContextItem.vue @@ -17,6 +17,12 @@ {{ t('tables', 'Edit application') }} + + + {{ t('tables', 'Transfer application') }} + @@ -27,6 +33,7 @@ import { mapGetters } from 'vuex' import TableIcon from 'vue-material-design-icons/Table.vue' import { emit } from '@nextcloud/event-bus' import PlaylistEdit from 'vue-material-design-icons/PlaylistEdit.vue' +import FileSwap from 'vue-material-design-icons/FileSwap.vue' import permissionsMixin from '../../../shared/components/ncTable/mixins/permissionsMixin.js' import svgHelper from '../../../shared/components/ncIconPicker/mixins/svgHelper.js' @@ -35,6 +42,7 @@ export default { components: { PlaylistEdit, + FileSwap, TableIcon, NcIconSvgWrapper, NcAppNavigationItem, @@ -73,6 +81,9 @@ export default { async editContext() { emit('tables:context:edit', this.context.id) }, + async transferContext() { + emit('tables:context:transfer', this.context) + }, }, } diff --git a/src/shared/components/ncUserAndGroupPicker/NcUserAndGroupPicker.vue b/src/shared/components/ncUserAndGroupPicker/NcUserAndGroupPicker.vue index 151452ad6..7289af1a2 100644 --- a/src/shared/components/ncUserAndGroupPicker/NcUserAndGroupPicker.vue +++ b/src/shared/components/ncUserAndGroupPicker/NcUserAndGroupPicker.vue @@ -64,7 +64,6 @@ export default { return this.newOwnerUserId }, set(v) { - console.info('newOwnerUserId set to ', v) this.$emit('update:newOwnerUserId', v) }, }, diff --git a/src/store/store.js b/src/store/store.js index 0095000dc..520a9a9d4 100644 --- a/src/store/store.js +++ b/src/store/store.js @@ -396,6 +396,21 @@ export default new Vuex.Store({ return true }, + async transferContext({ state, commit, dispatch }, { id, data }) { + try { + await axios.put(generateOcsUrl('/apps/tables/api/2/contexts/' + id + '/transfer'), data) + } catch (e) { + displayError(e, t('tables', 'Could not transfer application.')) + return false + } + + const contexts = state.contexts + const index = contexts.findIndex(t => t.id === id) + contexts.splice(index, 1) + commit('setContexts', [...contexts]) + return true + }, + async removeTable({ state, commit }, { tableId }) { try { await axios.delete(generateUrl('/apps/tables/table/' + tableId)) From 3cf2ca9b1129830d456798392e527430b50bc3ed Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 21 Mar 2024 18:51:04 +0100 Subject: [PATCH 2/3] enh(Contexts): extend permission check - checkPermission(ById) takes inherited permissions via Context into account - adds query to fetch all contexts containing a certain node - adds permission constants to Application Signed-off-by: Arthur Schiwon --- lib/AppInfo/Application.php | 7 +++ lib/Db/ContextMapper.php | 34 +++++++++++ lib/Service/PermissionsService.php | 94 ++++++++++++++++++++++++------ lib/Service/TableService.php | 6 ++ lib/Service/ViewService.php | 23 ++++---- 5 files changed, 136 insertions(+), 28 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 4d1ecec19..9da538a88 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -35,6 +35,13 @@ class Application extends App implements IBootstrap { public const NAV_ENTRY_MODE_RECIPIENTS = 1; public const NAV_ENTRY_MODE_ALL = 2; + public const PERMISSION_READ = 1; + public const PERMISSION_CREATE = 2; + public const PERMISSION_UPDATE = 4; + public const PERMISSION_DELETE = 8; + public const PERMISSION_MANAGE = 16; + public const PERMISSION_ALL = 31; + public function __construct() { parent::__construct(self::APP_ID); } diff --git a/lib/Db/ContextMapper.php b/lib/Db/ContextMapper.php index 91b6ae44a..2ab32b5bb 100644 --- a/lib/Db/ContextMapper.php +++ b/lib/Db/ContextMapper.php @@ -216,6 +216,40 @@ public function findById(int $contextId, ?string $userId = null): Context { return $this->formatResultRows($r, $userId); } + /** + * @return Context[] + * @throws Exception + */ + public function findAllContainingNode(int $nodeType, int $nodeId, string $userId): array { + $qb = $this->getFindContextBaseQuery($userId); + + $qb->andWhere($qb->expr()->eq('r.node_id', $qb->createNamedParameter($nodeId))) + ->andWhere($qb->expr()->eq('r.node_type', $qb->createNamedParameter($nodeType))); + + $result = $qb->executeQuery(); + $r = $result->fetchAll(); + + $contextIds = []; + foreach ($r as $row) { + $contextIds[$row['id']] = 1; + } + $contextIds = array_keys($contextIds); + unset($row); + + $resultEntities = []; + foreach ($contextIds as $contextId) { + $workArray = []; + foreach ($r as $row) { + if ($row['id'] === $contextId) { + $workArray[] = $row; + } + } + $resultEntities[] = $this->formatResultRows($workArray, $userId); + } + + return $resultEntities; + } + protected function applyOwnedOrSharedQuery(IQueryBuilder $qb, string $userId): void { $sharedToConditions = $qb->expr()->orX(); diff --git a/lib/Service/PermissionsService.php b/lib/Service/PermissionsService.php index 922bd4685..8106381b8 100644 --- a/lib/Service/PermissionsService.php +++ b/lib/Service/PermissionsService.php @@ -153,20 +153,7 @@ public function canManageContextById(int $contextId, ?string $userId = null): bo } public function canAccessView(View $view, ?string $userId = null): bool { - if($this->basisCheck($view, 'view', $userId)) { - return true; - } - - if ($userId) { - try { - $this->getSharedPermissionsIfSharedWithMe($view->getId(), 'view', $userId); - return true; - } catch (NotFoundError $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); - } - } - - return false; + return $this->canAccessNodeById(Application::NODE_TYPE_VIEW, $view->getId(), $userId); } /** @@ -458,6 +445,64 @@ public function getSharedPermissionsIfSharedWithMe(int $elementId, string $eleme // private methods ========================================================================== + /** + * @throws NotFoundError + */ + public function getPermissionIfAvailableThroughContext(int $nodeId, string $nodeType, string $userId): int { + $permissions = 0; + $found = false; + $iNodeType = match ($nodeType) { + 'table' => Application::NODE_TYPE_TABLE, + 'view' => Application::NODE_TYPE_VIEW, + }; + $contexts = $this->contextMapper->findAllContainingNode($iNodeType, $nodeId, $userId); + foreach ($contexts as $context) { + $found = true; + if ($context->getOwnerType() === Application::OWNER_TYPE_USER + && $context->getOwnerId() === $userId) { + // Making someone owner of a context, makes this person also having manage permissions on the node. + // This is sort of an intended "privilege escalation". + return Application::PERMISSION_ALL; + } + foreach ($context->getNodes() as $nodeRelation) { + $permissions |= $nodeRelation['permissions']; + } + } + if (!$found) { + throw new NotFoundError('Node not found in any context'); + } + return $permissions; + } + + /** + * @throws NotFoundError + */ + public function getPermissionArrayForNodeFromContexts(int $nodeId, string $nodeType, string $userId) { + $permissions = $this->getPermissionIfAvailableThroughContext($nodeId, $nodeType, $userId); + return [ + 'read' => (bool)($permissions & Application::PERMISSION_READ), + 'create' => (bool)($permissions & Application::PERMISSION_CREATE), + 'update' => (bool)($permissions & Application::PERMISSION_UPDATE), + 'delete' => (bool)($permissions & Application::PERMISSION_DELETE), + 'manage' => (bool)($permissions & Application::PERMISSION_MANAGE), + ]; + } + + private function hasPermission(int $existingPermissions, string $permissionName): bool { + $constantName = 'PERMISSION_' . strtoupper($permissionName); + try { + $permissionBit = constant(Application::class . "::$constantName"); + } catch (\Throwable $t) { + $this->logger->error('Unexpected permission string {permission}', [ + 'app' => Application::APP_ID, + 'permission' => $permissionName, + 'exception' => $t, + ]); + return false; + } + return (bool)($existingPermissions & $permissionBit); + } + /** * @param mixed $element * @param 'table'|'view' $nodeType @@ -470,13 +515,22 @@ private function checkPermission($element, string $nodeType, string $permission, return true; } - if ($userId) { + if (!$userId) { + return false; + } + + try { + return $this->getSharedPermissionsIfSharedWithMe($element->getId(), $nodeType, $userId)[$permission]; + } catch (NotFoundError $e) { try { - return $this->getSharedPermissionsIfSharedWithMe($element->getId(), $nodeType, $userId)[$permission]; + if ($this->hasPermission($this->getPermissionIfAvailableThroughContext($element->getId(), $nodeType, $userId), $permission)) { + return true; + } } catch (NotFoundError $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); } + $this->logger->error($e->getMessage(), ['exception' => $e]); } + return false; } @@ -495,6 +549,12 @@ private function checkPermissionById(int $elementId, string $nodeType, string $p try { return $this->getSharedPermissionsIfSharedWithMe($elementId, $nodeType, $userId)[$permission]; } catch (NotFoundError $e) { + try { + if ($this->hasPermission($this->getPermissionIfAvailableThroughContext($elementId, $nodeType, $userId), $permission)) { + return true; + } + } catch (NotFoundError $e) { + } $this->logger->error($e->getMessage(), ['exception' => $e]); } } diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index e28fe5fcb..03659e594 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -196,6 +196,12 @@ private function enhanceTable(Table $table, string $userId): void { $table->setIsShared(true); $table->setOnSharePermissions($permissions); } catch (NotFoundError $e) { + try { + $table->setOnSharePermissions($this->permissionsService->getPermissionArrayForNodeFromContexts($table->getId(), 'table', $userId)); + $table->setIsShared(true); + } catch (NotFoundError $e) { + } + } } if (!$table->getIsShared() || $table->getOnSharePermissions()['manage']) { diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 4593086a0..d509fa85c 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -322,24 +322,25 @@ private function enhanceView(View $view, string $userId): void { if ($userId !== '') { if ($userId !== $view->getOwnership()) { try { - $permissions = $this->shareService->getSharedPermissionsIfSharedWithMe($view->getId(), 'view', $userId); + try { + $permissions = $this->shareService->getSharedPermissionsIfSharedWithMe($view->getId(), 'view', $userId); + } catch (NotFoundError) { + $permissions = $this->permissionsService->getPermissionArrayForNodeFromContexts($view->getId(), 'view', $userId); + } $view->setIsShared(true); $canManageTable = false; try { - $manageTableShare = $this->shareService->getSharedPermissionsIfSharedWithMe($view->getTableId(), 'table', $userId); - $canManageTable = $manageTableShare['manage'] ?? false; + try { + $manageTableShare = $this->shareService->getSharedPermissionsIfSharedWithMe($view->getTableId(), 'table', $userId); + } catch (NotFoundError) { + $manageTableShare = $this->permissionsService->getPermissionArrayForNodeFromContexts($view->getTableId(), 'table', $userId); + } + $permissions['manageTable'] = $manageTableShare['manage'] ?? false; } catch (NotFoundError $e) { } catch (\Exception $e) { throw new InternalError($e->getMessage()); } - $view->setOnSharePermissions([ - 'read' => $permissions['read'] ?? false, - 'create' => $permissions['create'] ?? false, - 'update' => $permissions['update'] ?? false, - 'delete' => $permissions['delete'] ?? false, - 'manage' => $permissions['manage'] ?? false, - 'manageTable' => $canManageTable - ]); + $view->setOnSharePermissions($permissions); } catch (NotFoundError $e) { } catch (\Exception $e) { $this->logger->warning('Exception occurred while setting shared permissions: '.$e->getMessage().' No permissions granted.'); From 72afabb4b2462c74d9c9ca8d6cd884fd20fe48a2 Mon Sep 17 00:00:00 2001 From: Cleopatra Enjeck M Date: Tue, 26 Mar 2024 12:35:35 +0100 Subject: [PATCH 3/3] fix: load tables/views from transferred context Signed-off-by: Cleopatra Enjeck M --- src/pages/Context.vue | 10 ++++----- src/store/store.js | 49 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/pages/Context.vue b/src/pages/Context.vue index 173882478..8b7819f0e 100644 --- a/src/pages/Context.vue +++ b/src/pages/Context.vue @@ -130,13 +130,10 @@ export default { methods: { async reload() { - this.loading = true - if (this.activeContextId) { - await this.loadContext() + if (!this.activeContextId) { + return } - this.loading = false - }, - async loadContext() { + this.loading = true this.icon = await this.getContextIcon(this.activeContext.iconName) this.contextResources = [] await this.$store.dispatch('loadContext', { id: this.activeContextId }) @@ -175,6 +172,7 @@ export default { } } } + this.loading = false }, createColumn(isView, element) { emit('tables:column:create', { isView, element }) diff --git a/src/store/store.js b/src/store/store.js index 520a9a9d4..02c6a72d5 100644 --- a/src/store/store.js +++ b/src/store/store.js @@ -377,6 +377,7 @@ export default new Vuex.Store({ try { const res = await axios.get(generateOcsUrl('/apps/tables/api/2/contexts')) commit('setContexts', res.data.ocs.data) + await this.dispatch('getContextsTablesAndViews') } catch (e) { displayError(e, t('tables', 'Could not load applications.')) showError(t('tables', 'Could not fetch applications')) @@ -395,6 +396,54 @@ export default new Vuex.Store({ } return true }, + async getContextsTablesAndViews({ state }) { + for (const context of state.contexts) { + for (const node of Object.values(context?.nodes)) { + if (parseInt(node.node_type) === NODE_TYPE_TABLE) { + await this.dispatch('loadContextTable', { id: node.node_id }) + } else if (parseInt(node.node_type) === NODE_TYPE_VIEW) { + await this.dispatch('loadContextView', { id: node.node_id }) + } + } + + } + }, + + async loadContextTable({ commit, state, getters }, { id }) { + const table = getters.getTable(id) + if (table) { + return true + } + let res + try { + res = await axios.get(generateOcsUrl('/apps/tables/api/2/tables/' + id)) + const tables = state.tables + tables.push(res.data.ocs.data) + commit('setTables', tables) + } catch (e) { + displayError(e, t('tables', 'Could not load table.')) + showError(t('tables', 'Could not fetch table')) + } + return res?.data.ocs.data + }, + + async loadContextView({ commit, state, getters }, { id }) { + const view = getters.getView(id) + if (view) { + return true + } + let res + try { + res = await axios.get(generateUrl('/apps/tables/view/' + id)) + const views = state.views + views.push(res.data) + commit('setViews', views) + } catch (e) { + displayError(e, t('tables', 'Could not load view')) + showError(t('tables', 'Could not fetch view')) + } + return res?.data + }, async transferContext({ state, commit, dispatch }, { id, data }) { try {