MB-71275 - better permissions handling in the document editor

This issue came out of CBSE-22470, where a customer noted problems
with a user who lacked `collections!read` permission on some buckets,
but had the permission for other buckets. The bucket/scope/collection
menu in the documents page would allow the user to select both
permitted and non-permitted buckets, but no scopes/collections were
shown for the non-permitted buckets.

The user in question had permissions for Query System Catalog, but the
UI was trying to use the non-permitted `/pools/default` endpoint to
retrieve the list of scopes and collections for the bucket.

The UI would default to the `pools/default` endpoint if the user had
'cluster.collection[.:.:.].collections!read' permissions, which means
the right to list scopes/collections for at least one bucket. But that
doesn't mean that they have the right to list scopes/collections for
*every* bucket.

The code has been unchanged for the past 4 years, and this is the
first time a customer has run into the problem. However, it looks like
the use of permissions in the document editor was never really correct
after the addition of collections in release 7.0.

Problem 1: there are two separate APIs to list scopes/collections,
which require *different* permissions. The `/pools/default/buckets`
endpoint requires 'cluster.collection[bucket:.:.].collections!read'
permission. However, the same information can be retrieved from the
query service if the user has the 'Query System Catalog' role. A user
might have one of those permissions but not both, thus the code must
check and use the appropriate endpoint.

Problem 2: there are two separate APIs for retrieving documents. A
user with only 'data.docs!read' permission can't use the query
endpoint, but a user with 'n1ql.select!execute' can use either the
query endpoint or '/pools/default/buckets'. And of course these
permissions are independent of those described in Problem 1.

Problem 3: to determine whether to permit document editing, the UI was
only checking bucket-level permissions, and not collection-level
permissions. Thus it might allow the user to try and edit a document
in a read-only collection if there was some other collection in the
bucket that is writeable. The back-end would prevent the edit, but
that doesn't look good.

Solution 1: Retrieve the 'collections!read' permission for every
bucket when the document editor is initialized. The code was already
retrieving whether the user has Query Sytem Catalog, so now the code
can determine if the user has permission to fetch scope/collections,
and if so which of the two APIs to use.

Solution 2: Whenever a collection is selected in the document editor,
if needed fetch the permissions for that specific collection
w.r.t. 'docs!read', 'docs!upsert', 'docs!delete', and
'n1ql.select!execute'. With this information the code knows which APIs
are available for fetching documents, and whether the documents are
read-only, read-write, or write-only. (Yes, it is actually possible to
create documents in a collection where the user doesn't have read
access!)

Change-Id: Ie495af092f4330b7388060b265251e4a14f58db2
Reviewed-on: https://review.couchbase.org/c/query-ui/+/242881
Reviewed-by: Piotr Najda  <piotr.najda@couchbase.com>
Tested-by: Eben Haber <eben@couchbase.com>
Well-Formed: Restriction Checker

Author: ebenhaber

query-ui/angular-components/documents/qw.documents.component.js

@@ -93,7 +93,7 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
   }
 
   docViewer() {
-    return this.rbac.init && this.rbac.cluster.collection['.:.:.'].data.docs.read;
+    return this.rbac.init && (this.rbac.cluster.collection['.:.:.'].data.docs.read || this.rbac.cluster.collection['.:.:.'].data.docs.upsert);
   }
 
   constructor(
@@ -165,8 +165,9 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
     dec.how_to_query = how_to_query;
     dec.can_use_n1ql = can_use_n1ql;
     dec.has_indexes = has_indexes;
+    dec.get_n1ql_placeholder = get_n1ql_placeholder;
 
-    // wheneverthe collection menu is changed, remove any 'where' clause and offset
+    // whenever the collection menu is changed, remove any 'where' clause and offset
     dec.collectionMenuCallback = function(event) {
       //console.log("collectionMenuCallback: " + JSON.stringify(event));
       // MB-51579 - avoid race condition, make sure we have buckets specified
@@ -180,8 +181,12 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
         dec.searchForm.get('where_clause').setValue('');
         dec.searchForm.get('offset').setValue(0);
 
-        if (event.bucket && event.scope && event.collection)
-          retrieveDocs_inner();
+        if (event.bucket && event.scope && event.collection) {
+          // get permissions for new collection
+          qwMetadataService.checkCollectionPerms(dec.options.selected_bucket, dec.options.selected_scope, dec.options.selected_collection).then(function () {
+            retrieveDocs_inner();
+          });
+        }
       }
     };
 
@@ -195,16 +200,23 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
     //
 
     dec.upsertAllowed = function() {
-      return this.rbac.init && this.options.selected_bucket &&
-          this.rbac.cluster.collection[this.options.selected_bucket + ':.:.'] &&
-          (this.rbac.cluster.collection[this.options.selected_bucket + ':.:.'].data.docs.upsert ||
-              this.rbac.cluster.bucket[this.options.selected_bucket].data.docs.upsert);
+      if (!this.rbac.init || !this.options.selected_bucket || !this.options.selected_scope || !this.options.selected_collection)
+        return false;
+
+      const fullName = `${dec.options.selected_bucket}:${dec.options.selected_scope}:${dec.options.selected_collection}`;
+
+      return (this.rbac.cluster.collection[fullName]?.data?.docs?.upsert ||
+        this.rbac.cluster.bucket[this.options.selected_bucket].data.docs.upsert);
     };
+
     dec.deleteAllowed = function() {
-      return this.rbac.init && this.options.selected_bucket &&
-          this.rbac.cluster.collection[this.options.selected_bucket + ':.:.'] &&
-          (this.rbac.cluster.collection[this.options.selected_bucket + ':.:.'].data.docs.delete ||
-              this.rbac.cluster.bucket[this.options.selected_bucket].data.docs.delete);
+      if (!this.rbac.init || !this.options.selected_bucket || !this.options.selected_scope || !this.options.selected_collection)
+        return false;
+
+      const fullName = `${dec.options.selected_bucket}:${dec.options.selected_scope}:${dec.options.selected_collection}`;
+
+      return (this.rbac.cluster.collection[fullName]?.data?.docs?.delete ||
+        this.rbac.cluster.bucket[this.options.selected_bucket].data.docs.delete);
     };
 
     //
@@ -285,10 +297,20 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
         return (false);
       }
 
+      // corner case - can't query if we can't read documents
+      if (dec.rbac.cluster.collection[`${dec.options.selected_bucket}:${dec.options.selected_scope}:${dec.options.selected_collection}`]?.data?.docs?.read !== true) {
+        dec.options.current_result = "No permission to read documents.";
+        return (false);
+      }
+
       // always use KV for single doc lookups by ID
       if (dec.options.show_id && dec.options.doc_id)
         return KV;
 
+      // N1QL is not available if the user lacks n1ql.select permissions on the current collection
+      if (dec.rbac.cluster.collection[`${dec.options.selected_bucket}:${dec.options.selected_scope}:${dec.options.selected_collection}`]?.n1ql.select.execute !== true)
+        return KV;
+
       // key range lookup or limit/offset with no WHERE clause
       // - use N1QL if primary index, otherwise KV (though fail if ephemeral)
       if ((!dec.options.show_id && (dec.options.doc_id_start || dec.options.doc_id_end)) || dec.options.where_clause.length == 0) {
@@ -325,7 +347,18 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
     //
 
     function can_use_n1ql() {
-      return (has_prim() || has_sec());
+      // N1QL is not available if the user lacks n1ql.select permissions on the current collection
+      return (dec.rbac.cluster.collection[`${dec.options.selected_bucket}:${dec.options.selected_scope}:${dec.options.selected_collection}`]?.n1ql.select.execute === true
+        && (has_prim() || has_sec()));
+    }
+
+    function get_n1ql_placeholder() {
+      if (dec.rbac.cluster.collection[`${dec.options.selected_bucket}:${dec.options.selected_scope}:${dec.options.selected_collection}`]?.n1ql.select.execute !== true)
+        return 'no query permissions';
+      else if (!has_indexes())
+        return 'no indexes available...';
+      else
+        return 'optional...';
     }
 
     //
@@ -756,6 +789,7 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
         case false: // error status
           showErrorDialog("Document Error", dec.options.current_result, true);
           dec.options.current_query = dec.options.selected_bucket;
+          refreshResults();
           break;
       }
     }
@@ -1253,7 +1287,10 @@ class QwDocumentsComponent extends MnLifeCycleHooksToStream {
           metadataUpdate(meta);
           // MB-51579 - when collection unspecified, don't try to retrieve documents
           if (dec.options.selected_bucket && dec.options.selected_scope && dec.options.selected_collection && dec.docViewer())
-            retrieveDocs();
+            // get permissions for new collection
+            qwMetadataService.checkCollectionPerms(dec.options.selected_bucket, dec.options.selected_scope, dec.options.selected_collection).then(function () {
+              retrieveDocs_inner();
+            });
         });
       });
     }

query-ui/angular-components/documents/qw.documents.html

@@ -9,7 +9,7 @@
 -->
 
 <div *ngIf="!docViewer()">
-  Insufficient permissions to view documents. User must have at least Data Reader on one or more
+  Insufficient permissions to view documents. User must have at least Data Reader or Writer on one or more
   collections, and also the ability to view scopes and collections in that bucket.
 </div>
 
@@ -151,7 +151,7 @@ <h5 class="inline">Document ID&nbsp;</h5>
          (change)="dec.where_changed()"
          [attr.disabled]="(!dec.can_use_n1ql() || (dec.options.show_id && dec.options.doc_id) || (!dec.options.show_id && (dec.options.doc_id_start || dec.options.doc_id_end))) || null"
          class="width-12"
-         placeholder="{{dec.has_indexes() ? 'optional...' : 'no indexes available...'}}">
+         placeholder="{{dec.get_n1ql_placeholder()}}">
     </div>
 
     <div style="display:none;" class="resp-show-sml margin-top-half width-12"></div>

query-ui/angular-components/documents/qw.documents.module.js

@@ -50,7 +50,7 @@ let documentsStates = [
       name: 'app.admin.docs.editor',
       data: {
         title: "Documents",  // appears in breadcrumbs in title bar
-        permissions: "cluster.collection['.:.:.'].data.docs.read",
+        permissions: "cluster.collection['.:.:.'].data.docs.read || cluster.collection['.:.:.'].data.docs.upsert",
       },
       params: { // can parameters be sent via the URL?
         bucket: {

query-ui/angular-directives/qw.collection.menu.component.js

@@ -251,7 +251,13 @@ class QwCollectionMenu extends MnLifeCycleHooksToStream {
         this.scopes[selectedBucket].indexOf(selectedScope) < 0))
         this.keyspaceForm.get("scopeName").setValue(this.scopes[selectedBucket][0]);
 
-    // make sure we have collections to work with
+    // if no scopes, blank out scope selection
+    else if (this.scopes[selectedBucket].length === 0) {
+      this.keyspaceForm.get("scopeName").setValue('');
+      this.keyspaceForm.get("collectionName").setValue('');
+    }
+
+      // make sure we have collections to work with
     if (!selectedScope || !this.collections[selectedBucket] ||
       !this.collections[selectedBucket][selectedScope])
       return;

query-ui/angular-directives/qw.collection.menu.html

@@ -36,6 +36,6 @@
        *ngIf="errors && compat.atLeast70"
        placement="auto"
        container="body"
-       class="fa-warning icon orange-3 cursor-pointer"
+       class="fa-warning icon orange-3 cursor-pointer margin-right-half"
        [ngbTooltip]="errors">
     </span>

query-ui/angular-directives/qw.json.table.editor.directive.js

@@ -344,7 +344,7 @@ function createHTMLFromJson(json) {
 
   // if json not array, it must be error message string
   else {
-    resultHTML = wrapperStart + json + wrapperEnd;
+    resultHTML = `${wrapperStart} <div class="error text-small"> ${json} </div> ${wrapperEnd}`;
   }
 
   return (resultHTML);

query-ui/angular-services/qw.collections.service.js

@@ -170,10 +170,27 @@ function getQwCollectionsService(
 
 
   function refreshScopesAndCollectionsForBucket(bucket, proxy) {
-    const canUseEndpoints = qms.rbac && qms.rbac.cluster.collection['.:.:.'].collections.read;
-    return canUseEndpoints ?
-      refreshScopesAndCollectionsForBucketUsingApi(bucket, proxy) :
-      refreshScopesAndCollectionsForBucketUsingQuery(bucket, proxy);
+    // if we are fetching data from a remote cluster, we don't have any permission info, must use API and hope
+    // for the best
+    if (proxy || !qms.rbac) {
+      return refreshScopesAndCollectionsForBucketUsingApi(bucket, proxy);
+    }
+
+    // otherwise, which API do we have permission to use, if any?
+    const canUseEndpoints = qms.rbac?.cluster.collection[`${bucket}:.:.`].collections.read;
+    const canUseQuery = qms.rbac?.cluster.n1ql.meta.read;
+    if (canUseEndpoints)
+      return refreshScopesAndCollectionsForBucketUsingApi(bucket, proxy);
+    else if (canUseQuery)
+      return refreshScopesAndCollectionsForBucketUsingQuery(bucket, proxy);
+
+    // no permissions - return an error
+    const meta = getMeta(proxy);
+    meta.errors.length = 0;
+    meta.errors.push(`No permission to read scopes and collections for ${bucket}. User needs either 'cluster.collection[${bucket}:.:.].collections!read' or 'cluster.n1ql.meta!read'`);
+    meta.scopes[bucket] = [];
+    meta.collections[bucket] = {};
+    return Promise.resolve(meta);
   }
 
   function refreshScopesAndCollectionsForBucketUsingQuery(bucket, proxy) {

query-ui/angular-services/qw.metadata.service.js

@@ -72,6 +72,32 @@ class QwMetadataService {
                 error => {This.compat = {err: error}});
     }
 
+    // find out what permissions we have on a selected collection
+    checkCollectionPerms(bucket, scope, collection) {
+        const fullName = `${bucket}:${scope}:${collection}`;
+        const perms = [
+            `cluster.collection[${fullName}].data.docs!read`,
+            `cluster.collection[${fullName}].data.docs!upsert`,
+            `cluster.collection[${fullName}].data.docs!delete`,
+            `cluster.collection[${fullName}].n1ql.select!execute`,
+        ];
+
+        // do we need to get the permissions?
+        if (this.rbac.cluster.collection[fullName]?.data?.docs?.read === undefined ||
+          this.rbac.cluster.collection[fullName]?.data?.docs?.upsert === undefined ||
+          this.rbac.cluster.collection[fullName]?.data?.docs?.delete === undefined ||
+          this.rbac.cluster.collection[fullName]?.n1ql?.select?.execute === undefined) {
+            return this.qwHttp.post('/pools/default/checkPermissions',perms.join(','))
+              .then(result => {
+                  this.decodePermissions(result.data,true)
+                },
+                err => {this.rbac = err});
+        }
+        else
+            return Promise.resolve();
+    }
+
+
     decodePoolsDefault(poolDefault) {
         let This = this;
         // figure out compat version
@@ -122,6 +148,9 @@ class QwMetadataService {
 
             ...this.bucketList.map(bucketName => 'cluster.collection[' + bucketName + ':.:.].n1ql.udf!manage'),
             ...this.bucketList.map(bucketName => 'cluster.collection[' + bucketName + ':.:.].n1ql.udf_external!manage'),
+
+          // permissions needed for reading scopes & collections
+            ...this.bucketList.map(bucketName => 'cluster.collection[' + bucketName + ':.:.].collections!read'),
         ];
 
         let promise = this.qwHttp.post('/pools/default/checkPermissions',perms.join(','))
@@ -133,7 +162,7 @@ class QwMetadataService {
 
 
     // decode permissions from the array of 'name':<bool> to a tree
-    decodePermissions(permissions) {
+    decodePermissions(permissions, skipIndexes) {
         //console.log("Got permissions: " + JSON.stringify(permissions,null,2));
 
         this.rbac.init = true;
@@ -172,10 +201,12 @@ class QwMetadataService {
 
         // get index info, if possible. If query nodes exist, use a query,
         // which works even if /indexStatus API is forbidden
-        if (this.queryNodes.length > 0)
-            return this.getIndexesN1QL();
-        else
-            return this.getIndexesREST();
+        if (!skipIndexes) {
+            if (this.queryNodes.length > 0)
+                return this.getIndexesN1QL();
+            else
+                return this.getIndexesREST();
+        }
     }
 
     // do we have enough permissions to run queries?

query-ui/ui-current/main.js

@@ -42,7 +42,7 @@ angular
       state: 'app.admin.docs.editor',
       includedByState: 'app.admin.docs',
       plugIn: 'workbenchTab',
-      ngShow: "rbac.cluster.collection['.:.:.'].data.docs.read",
+      ngShow: "rbac.cluster.collection['.:.:.'].data.docs.read || rbac.cluster.collection['.:.:.'].data.docs.upsert",
       index: 0
     });