diff --git a/lib/Db/RowMapper.php b/lib/Db/RowMapper.php index 04d2ec4c9..91bd5d7eb 100644 --- a/lib/Db/RowMapper.php +++ b/lib/Db/RowMapper.php @@ -149,7 +149,6 @@ private function addOrderByRules(IQueryBuilder &$qb, $sortArray) { $orderString = 'JSON_EXTRACT(data, CONCAT( JSON_UNQUOTE(JSON_SEARCH(JSON_EXTRACT(data, \'$[*].columnId\'), \'one\', :'.$sortColumnPlaceholder.')), \'.value\'))'; if (str_starts_with($sortRule['columnType'],'number')) { $orderString = 'CAST('.$orderString.' as int)'; - //TODO: Better solution? } $qb->addOrderBy($qb->createFunction($orderString),$sortMode); $qb->setParameter($sortColumnPlaceholder,$sortRule['columnId'], $qb::PARAM_INT); @@ -294,7 +293,6 @@ private function getAllColumnIdsFromView(View $view): array { foreach ($sorts as $sortRule) { $neededColumnIds[] = $sortRule['columnId']; } - // TODO add sorting ids return array_unique($neededColumnIds); } diff --git a/lib/Db/ViewMapper.php b/lib/Db/ViewMapper.php index 2d5e2a6e6..fc77e3776 100644 --- a/lib/Db/ViewMapper.php +++ b/lib/Db/ViewMapper.php @@ -94,29 +94,35 @@ public function findAllNotBaseViews(?int $tableId = null): array { */ public function search(string $term = null, ?string $userId = null, ?int $limit = null, ?int $offset = null): array { $qb = $this->db->getQueryBuilder(); - $shareQuery = $this->db->getQueryBuilder(); + $shareViewQuery = $this->db->getQueryBuilder(); + $shareTableQuery = $this->db->getQueryBuilder(); // get view ids, that are shared with the given user // only makes sense if a user is given, otherwise will always get all shares doubled if ($userId !== null && $userId !== '') { - $shareQuery->selectDistinct('node_id') + $shareViewQuery->selectDistinct('node_id') ->from('tables_shares') ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('view', IQueryBuilder::PARAM_STR))) ->andWhere($qb->expr()->eq('receiver', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))); + $shareTableQuery->selectDistinct('node_id') + ->from('tables_shares') + ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('table', IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('receiver', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))); } - $qb->select('*') - ->from($this->table); + $qb->select('v.*') + ->from($this->table, 'v') + ->leftJoin('v','tables_tables', 't','t.id = v.table_id'); if ($userId !== null && $userId !== '') { - $qb->andWhere($qb->expr()->eq('created_by', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))); - $qb->orWhere($shareQuery->expr()->in('id', $qb->createFunction($shareQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY)); + $qb->andWhere($qb->expr()->eq('ownership', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->orWhere($shareViewQuery->expr()->in('v.id', $qb->createFunction($shareViewQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY)) + ->orWhere($shareTableQuery->expr()->in('v.table_id', $qb->createFunction($shareTableQuery->getSQL()), IQueryBuilder::PARAM_INT_ARRAY)); } - //TODO: Created by instead of ownership? if ($term) { $qb->andWhere($qb->expr()->iLike( - 'title', + 'v.title', $qb->createNamedParameter( '%' . $this->db->escapeLikeParameter($term) . '%', IQueryBuilder::PARAM_STR) diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index e221a60de..6d4215944 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -53,7 +53,7 @@ public function __construct( */ public function findAllByTable(int $tableId, ?int $viewId = null, ?string $userId = null): array { try { - if ($this->permissionsService->canReadTableColumnsByTableId($tableId, $userId) || ($viewId != null && $this->permissionsService->canReadTableColumnsByViewId($viewId, $userId))) { + if ($this->permissionsService->canReadColumnsByTableId($tableId, $userId) || ($viewId != null && $this->permissionsService->canReadColumnsByViewId($viewId, $userId))) { return $this->mapper->findAllByTable($tableId); } else { throw new PermissionError('no read access to table id = '.$tableId); diff --git a/lib/Service/ImportService.php b/lib/Service/ImportService.php index 41cce5844..a14bc9b47 100644 --- a/lib/Service/ImportService.php +++ b/lib/Service/ImportService.php @@ -55,7 +55,9 @@ public function __construct(PermissionsService $permissionsService, LoggerInterf * @throws NotFoundError */ public function import(int $tableId, int $viewId, string $path, bool $createMissingColumns = true): array { - //TODO: Permission add to this view + if (!$this->permissionsService->canCreateRowsByViewId($viewId)) { + throw new PermissionError('create row at the view id = '.$viewId.' is not allowed.'); + } if ($this->userManager->get($this->userId) === null) { $error = 'No user in context, can not import data. Cancel.'; $this->logger->debug($error); diff --git a/lib/Service/PermissionsService.php b/lib/Service/PermissionsService.php index f339a9094..bde551022 100644 --- a/lib/Service/PermissionsService.php +++ b/lib/Service/PermissionsService.php @@ -88,68 +88,28 @@ public function canAccessView($view, ?string $userId = null): bool { * @param string|null $userId * @return bool */ - public function canReadElement($element, string $nodeType, ?string $userId = null): bool { - return $this->checkPermission($element, $nodeType, 'read', $userId); + public function canManageView(View $view, ?string $userId = null): bool { + return $this->checkPermission($view, 'view', 'manage', $userId); } - /** - * @param Table|View $element - * @param string|null $userId - * @return bool - */ - public function canCreateElement($element, string $nodeType, ?string $userId = null): bool { - return $this->checkPermission($element, $nodeType, 'create', $userId); - } - - /** - * @param Table|View $element - * @param string|null $userId - * @return bool - */ - public function canUpdateElement($element, string $nodeType, ?string $userId = null): bool { - return $this->checkPermission($element, $nodeType, 'update', $userId); - } - - /** - * @param Table|View $element - * @param string|null $userId - * @return bool - */ - public function canDeleteElement($element, string $nodeType, ?string $userId = null): bool { - return $this->checkPermission($element, $nodeType, 'delete', $userId); + public function canManageTable(Table $table, ?string $userId = null): bool { + return $this->checkPermission($table, 'table', 'manage', $userId); } - - /** - * @param Table|View $element - * @param string|null $userId - * @return bool - */ - public function canManageElement($element, string $nodeType, ?string $userId = null): bool { - return $this->checkPermission($element, $nodeType, 'manage', $userId); + public function canManageTableById(int $tableId, ?string $userId = null): bool { + $table = $this->tableMapper->find($tableId); + return $this->canManageTable($table, $userId); } // ***** COLUMNS permissions ***** - public function canReadTableColumnsByViewId(int $viewId, ?string $userId = null): bool { - try { - $view = $this->viewMapper->find($viewId); - // if you can read the table, you also can read its columns - return $this->canReadRowsByElementId($view->getId(), 'view', $userId); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - } - return false; + public function canReadColumnsByViewId(int $viewId, ?string $userId = null): bool { + return $this->canReadRowsByElementId($viewId, 'view', $userId); } - public function canReadTableColumnsByTableId(int $tableId, ?string $userId = null): bool { - try { - $table = $this->tableMapper->find($tableId); - // if you can read the table, you also can read its columns - return $this->canReadRowsByElementId($table->getId(), 'table', $userId); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - } - return false; + public function canReadColumnsByTableId(int $tableId, ?string $userId = null): bool { + return $this->canReadRowsByElementId($tableId, 'table', $userId); } /** @@ -158,22 +118,7 @@ public function canReadTableColumnsByTableId(int $tableId, ?string $userId = nul * @return bool */ public function canCreateColumns(Table $table, ?string $userId = null): bool { - // this is the same permission as to update a table - return $this->canManageElement($table, 'table', $userId); - } - - /** - * @param int $tableId - * @param string|null $userId - * @return bool - */ - public function canCreateColumnsByTableId(int $tableId, ?string $userId = null): bool { - try { - $table = $this->tableMapper->find($tableId); - return $this->canCreateColumns($table, $userId); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - } - return false; + return $this->canManageTable($table, $userId); } /** @@ -182,13 +127,7 @@ public function canCreateColumnsByTableId(int $tableId, ?string $userId = null): * @return bool */ public function canUpdateColumnsByTableId(int $tableId, ?string $userId = null): bool { - try { - $table = $this->tableMapper->find($tableId); - // this is the same permission as to update a table - return $this->canManageElement($table, 'table', $userId); - } catch (\Exception $e) { - } - return false; + return $this->canManageTableById($tableId, $userId); } /** @@ -197,13 +136,7 @@ public function canUpdateColumnsByTableId(int $tableId, ?string $userId = null): * @return bool */ public function canDeleteColumnsByTableId(int $tableId, ?string $userId = null): bool { - try { - $table = $this->tableMapper->find($tableId); - // this is the same permission as to update a table - return $this->canManageElement($table, 'table', $userId); - } catch (\Exception $e) { - } - return false; + return $this->canManageTableById($tableId, $userId); } @@ -219,6 +152,10 @@ public function canReadRowsByElementId(int $elementId, string $nodeType, ?string return $this->checkPermissionById($elementId, $nodeType, 'read', $userId); } + public function canReadRowsByElement($element, string $nodeType, ?string $userId = null): bool { + return $this->checkPermission($element, $nodeType, 'read', $userId); + } + /** * @param int $tableId * @param string|null $userId @@ -360,7 +297,7 @@ private function checkPermission($element, string $nodeType, string $permission, try { return $this->getSharedPermissionsIfSharedWithMe($element->getId(), $nodeType, $userId)[$permission]; - } catch (InternalError|NotFoundError $e) { + } catch (NotFoundError $e) { } return false; } @@ -374,7 +311,7 @@ private function checkPermissionById(int $elementId, string $nodeType, string $p try { return $this->getSharedPermissionsIfSharedWithMe($elementId, $nodeType, $userId)[$permission]; - } catch (InternalError|NotFoundError $e) { + } catch (NotFoundError $e) { } return false; } diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index b800155ab..a2b141706 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -304,10 +304,10 @@ public function delete(int $id, int $viewId, string $userId): Row { * @throws \OCP\DB\Exception */ public function deleteAllByTable(int $tableId, ?string $userId = null): int { - /*// security + // security if (!$this->permissionsService->canDeleteRowsByTableId($tableId, $userId)) { throw new PermissionError('delete all rows for table id = '.$tableId.' is not allowed.'); - }TODO: If you can delete a table you should be allowed to delete the rows?! */ + } return $this->mapper->deleteAllByTable($tableId); } @@ -321,11 +321,11 @@ public function deleteColumnDataFromRows(int $columnId):void { $rows = $this->mapper->findAllWithColumn($columnId); // security - /*if (count($rows) > 0) { + if (count($rows) > 0) { if (!$this->permissionsService->canUpdateRowsByTableId($rows[0]->getTableId())) { throw new PermissionError('update row id = '.$rows[0]->getId().' within '.__FUNCTION__.' is not allowed.'); } - } TODO: Is this necessary? You only do that if you are allowed to delete columns. Then you should also be allowed to delete rows*/ + } foreach ($rows as $row) { /* @var $row Row */ diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index 972ec48e3..ac0a80ad5 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -192,7 +192,7 @@ public function find(int $id, bool $skipTableEnhancement = false, ?string $userI $table = $this->mapper->find($id); // security - if (!$this->permissionsService->canReadElement($table, 'table', $userId)) { + if (!$this->permissionsService->canManageTable($table, $userId)) { throw new PermissionError('PermissionError: can not read table with id '.$id); } @@ -267,7 +267,7 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $userId $table = $this->mapper->find($id); // security - if (!$this->permissionsService->canUpdateElement($table, 'table', $userId)) { + if (!$this->permissionsService->canManageTable($table, $userId)) { throw new PermissionError('PermissionError: can not update table with id '.$id); } @@ -328,7 +328,7 @@ public function delete(int $id, ?string $userId = null): Table { $item = $this->mapper->find($id); // security - if (!$this->permissionsService->canDeleteElement($item, 'table', $userId)) { + if (!$this->permissionsService->canManageTable($item, $userId)) { throw new PermissionError('PermissionError: can not delete table with id '.$id); } diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index a02c8c605..3802dcc0d 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -66,7 +66,7 @@ private function findAllGeneralised(Table $table, bool $includeBaseView = true, try { // security - if (!$this->permissionsService->canReadElement($table, 'table', $userId)) { + if (!$this->permissionsService->canManageTable($table, $userId)) { throw new PermissionError('PermissionError: can not read views for tableId '.$table->getId()); } @@ -89,11 +89,11 @@ public function findBaseView(Table $table, bool $skipTableEnhancement = false, ? $userId = $this->permissionsService->preCheckUserId($userId); // $userId can be set or '' try { - $baseView = $this->mapper->findBaseView($table->getId()); // security - if (!$this->permissionsService->canReadElement($baseView, 'view', $userId)) { + if (!$this->permissionsService->canManageTable($table, $userId)) { throw new PermissionError('PermissionError: can not read views for tableId '.$table->getId()); } + $baseView = $this->mapper->findBaseView($table->getId()); if(!$skipTableEnhancement) $this->enhanceView($baseView, $userId); return $baseView; } catch (\OCP\DB\Exception $e) { @@ -166,7 +166,7 @@ public function create(string $title, ?string $emoji, Table $table, bool $isBase $userId = $this->permissionsService->preCheckUserId($userId, false); // $userId is set // security - if (!$this->permissionsService->canUpdateElement($table, 'table', $userId)) { + if (!$this->permissionsService->canManageTable($table, $userId)) { throw new PermissionError('PermissionError: can not create view'); } @@ -220,7 +220,7 @@ public function update(int $id, array $data, ?string $userId = null, bool $skipT $view = $this->mapper->find($id); // security - if (!$this->permissionsService->canManageElement($view, 'view', $userId)) { + if (!$this->permissionsService->canManageView($view, $userId)) { throw new PermissionError('PermissionError: can not update view with id '.$id); } @@ -257,7 +257,7 @@ public function delete(int $id, ?string $userId = null): View { } // security - if (!$this->permissionsService->canManageElement($view, 'view', $userId)) { + if (!$this->permissionsService->canManageView($view, $userId)) { throw new PermissionError('PermissionError: can not delete view with id '.$id); } $this->shareService->deleteAllForView($view); @@ -283,7 +283,7 @@ public function deleteByObject(View $view, ?string $userId = null): View { try { // security - if (!$this->permissionsService->canManageElement($view, 'view', $userId)) { + if (!$this->permissionsService->canManageView($view, $userId)) { throw new PermissionError('PermissionError: can not delete view with id '.$view->getId()); } // delete all shares for that table @@ -334,7 +334,14 @@ private function enhanceView(View &$view, string $userId): void { } } - if (!$this->permissionsService->canReadElement($view, 'view', $userId)) return; + if (!$this->permissionsService->canReadRowsByElement($view, 'view', $userId)) return; + // add the rows count + try { + $view->setRowsCount($this->rowService->getViewRowsCount($view, $userId)); + } catch (InternalError|PermissionError $e) { + } + + if (!$this->permissionsService->canManageTableById($view->getTableId(), $userId)) return; // set hasShares if this table is shared by you (you share it with somebody else) // (senseless if we have no user in context) @@ -348,12 +355,6 @@ private function enhanceView(View &$view, string $userId): void { } catch (InternalError $e) { } } - - // add the rows count - try { - $view->setRowsCount($this->rowService->getViewRowsCount($view, $userId)); - } catch (InternalError|PermissionError $e) { - } } /** @@ -365,9 +366,9 @@ private function enhanceView(View &$view, string $userId): void { */ public function deleteAllByTable(Table $table, ?string $userId = null): View { // security - /*if (!$this->permissionsService->canDeleteRowsByTableId($tableId, $userId)) { - throw new PermissionError('delete all rows for table id = '.$tableId.' is not allowed.'); - } TODO: If you can delete a table you should be allowed to delete the views?!*/ + if (!$this->permissionsService->canManageTable($table, $userId)) { + throw new PermissionError('delete all rows for table id = '.$table->getId().' is not allowed.'); + } $views = $this->findAll($table,$userId); foreach ($views as $view) { if($view->getIsBaseView()) { diff --git a/src/modules/main/modals/ViewSettings.vue b/src/modules/main/modals/ViewSettings.vue index a2c0a310c..a83c961a8 100644 --- a/src/modules/main/modals/ViewSettings.vue +++ b/src/modules/main/modals/ViewSettings.vue @@ -231,7 +231,6 @@ export default { } const res = await this.$store.dispatch('updateView', { id, data }) if (res) { - // TODO: Only reload if something changed this.$emit('reload-view') return res } else { diff --git a/src/modules/main/sections/Modals.vue b/src/modules/main/sections/Modals.vue deleted file mode 100644 index f4933a8cc..000000000 --- a/src/modules/main/sections/Modals.vue +++ /dev/null @@ -1,44 +0,0 @@ - - - diff --git a/src/modules/navigation/modals/Import.vue b/src/modules/navigation/modals/Import.vue index cc04d34fb..4b53275e0 100644 --- a/src/modules/navigation/modals/Import.vue +++ b/src/modules/navigation/modals/Import.vue @@ -170,10 +170,10 @@ export default { computed: { ...mapGetters(['activeView']), canCreateMissingColumns() { - return this.canManageElement(this.view) + return this.canManageTable(this.view) }, getCreateMissingColumns() { - return this.canManageElement(this.view) && this.createMissingColumns + return this.canManageTable(this.view) && this.createMissingColumns }, }, @@ -183,7 +183,7 @@ export default { if (this.activeView.tableId === this.view.tableId) { this.waitForReload = true await this.$store.dispatch('loadTablesFromBE') - await this.$store.dispatch('loadViewsSharedWithMeFromBE') //TODO: Test if importing in shared Table + await this.$store.dispatch('loadViewsSharedWithMeFromBE') await this.$store.dispatch('loadColumnsFromBE', { view: this.view }) await this.$store.dispatch('loadRowsFromBE', { viewId: this.view.id }) this.waitForReload = false diff --git a/src/modules/navigation/partials/NavigationViewItem.vue b/src/modules/navigation/partials/NavigationViewItem.vue index c2853675a..4996ca6c7 100644 --- a/src/modules/navigation/partials/NavigationViewItem.vue +++ b/src/modules/navigation/partials/NavigationViewItem.vue @@ -35,14 +35,6 @@ @click="actionShowShare"> {{ t('tables', 'Share') }} -