Applications: Transfer ownership

[juliushaertl]

• [x] Backend API
• [x] Frontend implementation

[enjeck]

@juliushaertl @blizzz When we transfer a context, don't we want to transfer (or share) the tables/views too? Otherwise, the new owner is unable to access the Context's tables/views because they don't have the right permissions

[blizzz]

Once the Context is created the connected tables and views are untangled from the permissions of the owner. So that all should continue to work. So the feature spec. If it does not, it's a bug.

[enjeck]

Once the Context is created the connected tables and views are untangled from the permissions of the owner. So that all should continue to work. So the feature spec. If it does not, it's a bug.

I checked phpmyadmin, and the ownership is still with the original owner. And I'm unable to access tables from transfered contexts using the table#index or table#show routes :confused:

[blizzz]

Once the Context is created the connected tables and views are untangled from the permissions of the owner. So that all should continue to work. So the feature spec. If it does not, it's a bug.

I checked phpmyadmin, and the ownership is still with the original owner. And I'm unable to access tables from transfered contexts using the table#index or table#show routes 😕

Untangled in the meaning that it table and view ownership or management permissions turn irrelevant (as long as the resource is connected to the Context)

[blizzz]

Oh I am starting to understand.

Checking out your feature branch, the ownership is transferred. But obviously the resources cannot be loaded :man_facepalming:

I understand all tables and views (at least their definitions) are loaded up front, for I do not see subsequent requests.

Options:

• change view and tables endpoint (or permission service) to also return tables and view that are available via Contexts. They would appear in the sidebar than as well, at least if we do not provide further information about their source so the frontend can filter them out.
• lazy load requests when opening a contexts, but would require a different endpoint. would not be very swift experience in the user interface.
• additional endpoint to load those tables and views subsequent (or parallel) to the regular fetching of tables and views.

What's your take?

[enjeck]

• lazy load requests when opening a contexts, but would require a different endpoint. would not be very swift experience in the user interface.

I was trying this out. I couldn't use the table#show route, as it fails with permission error. Would I need to create something custom for this?

[blizzz]

• lazy load requests when opening a contexts, but would require a different endpoint. would not be very swift experience in the user interface.

I was trying this out. I couldn't use the table#show route, as it fails with permission error. Would I need to create something custom for this?

That's on me, on the backend side.

[blizzz]

Please try with this patch as quick fix:

diff --git a/lib/Service/PermissionsService.php b/lib/Service/PermissionsService.php
index 922bd468..2789d5ae 100644
--- a/lib/Service/PermissionsService.php
+++ b/lib/Service/PermissionsService.php
@@ -242,7 +242,20 @@ class PermissionsService {
    public function canReadColumnsByTableId(int $tableId, ?string $userId = null): bool {
        $canReadRows = $this->checkPermissionById($tableId, 'table', 'read', $userId);
        $canCreateRows = $this->checkPermissionById($tableId, 'table', 'create', $userId);
-       return $canCreateRows || $canReadRows;
+       if ($canReadRows || $canCreateRows) {
+           return true;
+       }
+
+       $contexts = $this->contextMapper->findAll($userId ?? $this->userId);
+       foreach ($contexts as $context) {
+           $nodes = $context->getNodes();
+           foreach ($nodes as $node) {
+               if ((int)$node['node_type'] === Application::NODE_TYPE_TABLE && $node['node_id'] === $tableId) {
+                   return true;
+               }
+           }
+       }
+       return false;
    }

    /**
@@ -553,7 +566,23 @@ class PermissionsService {

        try {
            $element = $nodeType === 'table' ? $this->tableMapper->find($elementId) : $this->viewMapper->find($elementId);
-           return $this->basisCheck($element, $nodeType, $userId);
+           if ($this->basisCheck($element, $nodeType, $userId)) {
+               return true;
+           }
+           $contextMapper = \OCP\Server::get(ContextMapper::class);
+           $contexts = $contextMapper->findAll($userId ?? $this->userId);
+           foreach ($contexts as $context) {
+               $nodes = $context->getNodes();
+               foreach ($nodes as $node) {
+                   if ($nodeType === 'table' && (int)$node['node_type'] === Application::NODE_TYPE_TABLE && $node['node_id'] === $elementId) {
+                       return true;
+                   }
+                   if ($nodeType === 'view' && (int)$node['node_type'] === Application::NODE_TYPE_VIEW && $node['node_id'] === $elementId) {
+                       return true;
+                   }
+               }
+           }
+           // FIXME: avoid duplications
        } catch (DoesNotExistException|MultipleObjectsReturnedException|\Exception $e) {
            $this->logger->warning('Exception occurred: '.$e->getMessage());
        }

I could not test yet, and a call is approaching.

[blizzz]

Updated the patch, but it is probably not sufficient. Need to have a better look at the Permissions tomorrow.

[blizzz]

@enjeck please try with #947 and fetching the table. The PR is based on your PR :tokyo_tower:

[blizzz]

@enjeck please try with #947 and fetching the table. The PR is based on your PR 🗼

I updated that PR, there was a few more adjustements needed. But now I can definitely can read a table or view via API that is accessible purely via context ownership.