Skip to content

Commit

Permalink
First step of cleanup, Solves TODOs
Browse files Browse the repository at this point in the history 
Signed-off-by: Philipp Hempel <Philipp.Hempel1@web.de>
  • Loading branch information
@Hephi2
Hephi2 committed Jul 21, 2023
1 parent b714887 commit 3634c6a
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 175 deletions.
2 changes: 0 additions & 2 deletions lib/Db/RowMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down
22 changes: 14 additions & 8 deletions lib/Db/ViewMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/ColumnService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion lib/Service/ImportService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
103 changes: 20 additions & 83 deletions lib/Service/PermissionsService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}


Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions lib/Service/RowService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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 */
Expand Down
6 changes: 3 additions & 3 deletions lib/Service/TableService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
35 changes: 18 additions & 17 deletions lib/Service/ViewService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
}
}

/**
Expand All @@ -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()) {
Expand Down
Loading

0 comments on commit 3634c6a

Please sign in to comment.