(core) treat summary tables like formulas for access control purposes

Summary:
This unsets the `direct` flag for actions emitted when summary tables are updated. That means those actions will be ignored for access control purposes. So if a user has the right to change a source table, the resulting changes to the summary won't result in the overall action bundle being forbidden.

I don't think I've actually seen the use case that inspired this issue being filed. I could imagine perhaps a user forbidden from creating rows globally making permitted updates that could add rows in a summary (and it being desirable to allow that).

Test Plan: added tests

Reviewers: jarek

Reviewed By: jarek

Subscribers: dsagal, alexmojaki, jarek

Differential Revision: https://phab.getgrist.com/D3022

app/common/ActiveDocAPI.ts

@@ -78,9 +78,7 @@ export interface ImportOptions {
  */
 interface BaseQuery {
   tableId: string;
-  filters: {
-    [colId: string]: any[];
-  };
+  filters: QueryFilters;
 }
 
 /**
@@ -101,6 +99,14 @@ export interface ServerQuery extends BaseQuery {
   limit?: number;
 }
 
+/**
+ * Type of the filters option to queries.
+ */
+export interface QueryFilters {
+  // TODO: check if "any" can be replaced with "CellValue".
+  [colId: string]: any[];
+}
+
 export type QueryOperation = "in" | "intersects";
 
 /**

app/common/UserAPI.ts

@@ -1,5 +1,5 @@
 import {ActionSummary} from 'app/common/ActionSummary';
-import {ApplyUAResult} from 'app/common/ActiveDocAPI';
+import {ApplyUAResult, QueryFilters} from 'app/common/ActiveDocAPI';
 import {BaseAPI, IOptions} from 'app/common/BaseAPI';
 import {BillingAPI, BillingAPIImpl} from 'app/common/BillingAPI';
 import {BrowserSettings} from 'app/common/BrowserSettings';
@@ -337,7 +337,7 @@ export interface UserAPI {
  * reasons, such as downloads.
  */
 export interface DocAPI {
-  getRows(tableId: string): Promise<TableColValues>;
+  getRows(tableId: string, options?: { filters?: QueryFilters }): Promise<TableColValues>;
   updateRows(tableId: string, changes: TableColValues): Promise<number[]>;
   addRows(tableId: string, additions: BulkColValues): Promise<number[]>;
   removeRows(tableId: string, removals: number[]): Promise<number[]>;
@@ -728,8 +728,9 @@ export class DocAPIImpl extends BaseAPI implements DocAPI {
     this._url = `${url}/api/docs/${docId}`;
   }
 
-  public async getRows(tableId: string): Promise<TableColValues> {
-    return this.requestJson(`${this._url}/tables/${tableId}/data`);
+  public async getRows(tableId: string, options?: { filters?: QueryFilters }): Promise<TableColValues> {
+    const query = options?.filters ? ("?filter=" + encodeURIComponent(JSON.stringify(options.filters))) : '';
+    return this.requestJson(`${this._url}/tables/${tableId}/data${query}`);
   }
 
   public async updateRows(tableId: string, changes: TableColValues): Promise<number[]> {

sandbox/grist/table.py

@@ -294,7 +294,12 @@ class Table(object):
     if summary_table._summary_simple:
       @usertypes.formulaType(usertypes.Reference(summary_table.table_id))
       def _updateSummary(rec, table):  # pylint: disable=unused-argument
-        return summary_table.lookupOrAddDerived(**{c: getattr(rec, c) for c in groupby_cols})
+        try:
+          # summary table output should be treated as we treat formula columns, for acl purposes
+          self._engine.user_actions.enter_indirection()
+          return summary_table.lookupOrAddDerived(**{c: getattr(rec, c) for c in groupby_cols})
+        finally:
+          self._engine.user_actions.leave_indirection()
     else:
       @usertypes.formulaType(usertypes.ReferenceList(summary_table.table_id))
       def _updateSummary(rec, table):  # pylint: disable=unused-argument
@@ -333,9 +338,14 @@ class Table(object):
             new_row_ids.append(None)
 
         if new_row_ids and not self._engine.is_triggered_by_table_action(summary_table.table_id):
-          result += self._engine.user_actions.BulkAddRecord(
-            summary_table.table_id, new_row_ids, values_to_add
-          )
+          try:
+            # summary table output should be treated as we treat formula columns, for acl purposes
+            self._engine.user_actions.enter_indirection()
+            result += self._engine.user_actions.BulkAddRecord(
+              summary_table.table_id, new_row_ids, values_to_add
+            )
+          finally:
+            self._engine.user_actions.leave_indirection()
 
         return result
 

sandbox/grist/useractions.py

@@ -30,6 +30,11 @@ log = logger.Logger(__name__, logger.INFO)
 _current_module = sys.modules[__name__]
 _action_types = {}
 
+# When distinguishing actions directly requested by the user from actions that
+# are indirect consequences of those actions (specifically, adding rows to summary tables)
+# we count levels of indirection. Zero indirection levels implies an action is directly
+# requested.
+DIRECT_ACTION = 0
 
 # Fields of _grist_Tables_column table that may be modified using ModifyColumns useraction.
 _modifiable_col_fields = {'type', 'widgetOptions', 'formula', 'isFormula', 'label',
@@ -147,19 +152,34 @@ class UserActions(object):
     self._summary = summary.SummaryActions(self, self._docmodel)
     self._import_actions = import_actions.ImportActions(self, self._docmodel, eng)
     self._allow_changes = False
+    self._indirection_level = DIRECT_ACTION
 
     # Map of methods implementing particular (action_name, table_id) combinations. It mirrors
     # global _action_method_overrides, but with methods *bound* to this UserActions instance.
     self._overrides = {key: method.__get__(self, UserActions)
                        for key, method in six.iteritems(_action_method_overrides)}
 
+  def enter_indirection(self):
+    """
+    Mark any actions following this call as being indirect, until leave_indirection is
+    called. Nesting is supported (but not used).
+    """
+    self._indirection_level += 1
+
+  def leave_indirection(self):
+    """
+    Undo an enter_indirection.
+    """
+    self._indirection_level -= 1
+    assert self._indirection_level >= 0
+
   def _do_doc_action(self, action):
     if hasattr(action, 'simplify'):
       # Convert bulk actions to single actions if possible, or None if it affects no rows.
       action = action.simplify()
     if action:
       self._engine.out_actions.stored.append(action)
-      self._engine.out_actions.direct.append(True)
+      self._engine.out_actions.direct.append(self._indirection_level == DIRECT_ACTION)
       self._engine.apply_doc_action(action)
 
   def _bulk_action_iter(self, table_id, row_ids, col_values=None):