(core) updates from grist-core

.github/workflows/main.yml

@@ -28,6 +28,13 @@ jobs:
           - ':nbrowser-^[M-O]:'
           - ':nbrowser-^[P-S]:'
           - ':nbrowser-^[^A-S]:'
+        include:
+          - tests: ':lint:python:client:common:smoke:'
+            node-version: 14.x
+            python-version: '3.10'
+          - tests: ':lint:python:client:common:smoke:'
+            node-version: 14.x
+            python-version: '3.11'
     steps:
       - uses: actions/checkout@v3
 

README.md

@@ -260,6 +260,7 @@ GRIST_DEFAULT_PRODUCT  | if set, this controls enabled features and limits of ne
 GRIST_DEFAULT_LOCALE  | Locale to use as fallback when Grist cannot honour the browser locale.
 GRIST_DOMAIN        | in hosted Grist, Grist is served from subdomains of this domain.  Defaults to "getgrist.com".
 GRIST_EXPERIMENTAL_PLUGINS | enables experimental plugins
+GRIST_ENABLE_REQUEST_FUNCTION | enables the REQUEST function. This function performs HTTP requests in a similar way to `requests.request`. This function presents a significant security risk, since it can let users call internal endpoints when Grist is available publicly. This function can also cause performance issues. Unset by default.
 GRIST_HIDE_UI_ELEMENTS | comma-separated list of UI features to disable. Allowed names of parts: `helpCenter,billing,templates,multiSite,multiAccounts,sendToDrive,tutorials`. If a part also exists in GRIST_UI_FEATURES, it will still be disabled.
 GRIST_HOME_INCLUDE_STATIC | if set, home server also serves static resources
 GRIST_HOST          | hostname to use when listening on a port.

app/common/ActionBundle.ts

@@ -72,6 +72,8 @@ export interface SandboxActionBundle {
 // Represents a unique call to the Python REQUEST function
 export interface SandboxRequest {
   url: string;
+  method: string;
+  body?: string;
   params: Record<string, string> | null;
   headers: Record<string, string> | null;
   deps: unknown;  // pass back to the sandbox unchanged in the response

app/gen-server/ApiServer.ts

@@ -11,10 +11,10 @@ import {getAuthorizedUserId, getUserId, getUserProfiles, RequestWithLogin} from
 import {getSessionUser, linkOrgWithEmail} from 'app/server/lib/BrowserSession';
 import {expressWrap} from 'app/server/lib/expressWrap';
 import {RequestWithOrg} from 'app/server/lib/extractOrg';
+import {getTemplateOrg} from 'app/server/lib/gristSettings';
 import log from 'app/server/lib/log';
 import {addPermit, clearSessionCacheIfNeeded, getDocScope, getScope, integerParam,
         isParameterOn, optStringParam, sendOkReply, sendReply, stringParam} from 'app/server/lib/requestUtils';
-import {getTemplateOrg} from 'app/server/lib/sendAppPage';
 import {IWidgetRepository} from 'app/server/lib/WidgetRepository';
 
 import {User} from './entity/User';

app/server/devServerMain.ts

@@ -66,6 +66,11 @@ export async function main() {
     process.env.GRIST_EXPERIMENTAL_PLUGINS = "1";
   }
 
+  // Experimental plugins are enabled by default for devs
+  if (!process.env.GRIST_ENABLE_REQUEST_FUNCTION) {
+    process.env.GRIST_ENABLE_REQUEST_FUNCTION = "1";
+  }
+
   // For tests, it is useful to start with the database in a known state.
   // If TEST_CLEAN_DATABASE is set, we reset the database before starting.
   if (process.env.TEST_CLEAN_DATABASE) {

app/server/lib/ActiveDoc.ts

@@ -89,6 +89,7 @@ import {Authorizer} from 'app/server/lib/Authorizer';
 import {checksumFile} from 'app/server/lib/checksumFile';
 import {Client} from 'app/server/lib/Client';
 import {DEFAULT_CACHE_TTL, DocManager} from 'app/server/lib/DocManager';
+import {getTemplateOrg} from 'app/server/lib/gristSettings';
 import {ICreateActiveDocOptions} from 'app/server/lib/ICreate';
 import {makeForkIds} from 'app/server/lib/idUtils';
 import {GRIST_DOC_SQL, GRIST_DOC_WITH_TABLE1_SQL} from 'app/server/lib/initialDocSql';
@@ -97,7 +98,6 @@ import log from 'app/server/lib/log';
 import {LogMethods} from "app/server/lib/LogMethods";
 import {NullSandbox, UnavailableSandboxMethodError} from 'app/server/lib/NullSandbox';
 import {DocRequests} from 'app/server/lib/Requests';
-import {getTemplateOrg} from 'app/server/lib/sendAppPage';
 import {shortDesc} from 'app/server/lib/shortDesc';
 import {TableMetadataLoader} from 'app/server/lib/TableMetadataLoader';
 import {DocTriggers} from "app/server/lib/Triggers";

app/server/lib/AppEndpoint.ts

@@ -20,10 +20,11 @@ import {DocStatus, IDocWorkerMap} from 'app/server/lib/DocWorkerMap';
 import {expressWrap} from 'app/server/lib/expressWrap';
 import {DocTemplate, GristServer} from 'app/server/lib/GristServer';
 import {getCookieDomain} from 'app/server/lib/gristSessions';
+import {getTemplateOrg} from 'app/server/lib/gristSettings';
 import {getAssignmentId} from 'app/server/lib/idUtils';
 import log from 'app/server/lib/log';
 import {adaptServerUrl, addOrgToPathIfNeeded, pruneAPIResult, trustOrigin} from 'app/server/lib/requestUtils';
-import {getTemplateOrg, ISendAppPageOptions} from 'app/server/lib/sendAppPage';
+import {ISendAppPageOptions} from 'app/server/lib/sendAppPage';
 
 export interface AttachOptions {
   app: express.Application;                // Express app to which to add endpoints

app/server/lib/Requests.ts

@@ -80,16 +80,21 @@ export class DocRequests {
 
   private async _handleSingleRequestRaw(request: SandboxRequest): Promise<Response> {
     try {
-      if (process.env.GRIST_EXPERIMENTAL_PLUGINS != '1') {
+      if (process.env.GRIST_ENABLE_REQUEST_FUNCTION != '1') {
         throw new Error("REQUEST is not enabled");
       }
-      const {url, params, headers} = request;
+      const {url, method, body, params, headers} = request;
       const urlObj = new URL(url);
       log.rawInfo("Handling sandbox request", {host: urlObj.host, docId: this._activeDoc.docName});
       for (const [param, value] of Object.entries(params || {})) {
         urlObj.searchParams.append(param, value);
       }
-      const response = await fetch(urlObj.toString(), {headers: headers || {}, agent: proxyAgent(urlObj)});
+      const response = await fetch(urlObj.toString(), {
+        headers: headers || {},
+        agent: proxyAgent(urlObj),
+        method,
+        body
+      });
       const content = await response.buffer();
       const {status, statusText} = response;
       const encoding = httpEncoding(response.headers.get('content-type'), content);

app/server/lib/gristSettings.ts

@@ -0,0 +1,13 @@
+import {appSettings} from 'app/server/lib/AppSettings';
+
+export function getTemplateOrg() {
+  let org = appSettings.section('templates').flag('org').readString({
+    envVar: 'GRIST_TEMPLATE_ORG',
+  });
+  if (!org) { return null; }
+
+  if (process.env.GRIST_ID_PREFIX) {
+    org += `-${process.env.GRIST_ID_PREFIX}`;
+  }
+  return org;
+}

app/server/lib/sendAppPage.ts

@@ -3,10 +3,10 @@ import {isAffirmative} from 'app/common/gutil';
 import {getTagManagerSnippet} from 'app/common/tagManager';
 import {Document} from 'app/common/UserAPI';
 import {SUPPORT_EMAIL} from 'app/gen-server/lib/HomeDBManager';
-import {appSettings} from 'app/server/lib/AppSettings';
 import {isAnonymousUser, isSingleUserMode, RequestWithLogin} from 'app/server/lib/Authorizer';
 import {RequestWithOrg} from 'app/server/lib/extractOrg';
 import {GristServer} from 'app/server/lib/GristServer';
+import {getTemplateOrg} from 'app/server/lib/gristSettings';
 import {getSupportedEngineChoices} from 'app/server/lib/serverUtils';
 import {readLoadedLngs, readLoadedNamespaces} from 'app/server/localization';
 import * as express from 'express';
@@ -154,18 +154,6 @@ export function makeSendAppPage(opts: {
   };
 }
 
-export function getTemplateOrg() {
-  let org = appSettings.section('templates').flag('org').readString({
-    envVar: 'GRIST_TEMPLATE_ORG',
-  });
-  if (!org) { return null; }
-
-  if (process.env.GRIST_ID_PREFIX) {
-    org += `-${process.env.GRIST_ID_PREFIX}`;
-  }
-  return org;
-}
-
 function shouldSupportAnon() {
   // Enable UI for anonymous access if a flag is explicitly set in the environment
   return process.env.GRIST_SUPPORT_ANON === "true";

sandbox/grist/functions/info.py

@@ -4,13 +4,14 @@
 from __future__ import absolute_import
 import datetime
 import hashlib
-import json
+import json as json_module
 import math
 import numbers
 import re
 
 import chardet
 import six
+from six.moves import urllib_parse
 
 import column
 import docmodel
@@ -653,20 +654,70 @@ def is_error(value):
       or (isinstance(value, float) and math.isnan(value)))
 
 
+def _replicate_requests_body_args(data=None, json=None):
+  """
+  Replicate some of the behaviour of requests.post, specifically the data and 
+  json args.
+
+  Returns a tuple of (body, extra_headers)
+  """
+  if data is None and json is None:
+      return None, {}
+
+  elif data is not None and json is None:
+    if isinstance(data, str):
+      body = data
+      extra_headers = {}
+    else:
+      body = urllib_parse.urlencode(data)
+      extra_headers = {
+        "Content-Type": "application/x-www-form-urlencoded",
+      }
+    return body, extra_headers
+
+  elif json is not None and data is None:
+    if isinstance(json, str):
+      body = json
+    else:
+      body = json_module.dumps(json)
+    extra_headers = {
+      "Content-Type": "application/json",
+    }
+    return body, extra_headers
+
+  elif data is not None and json is not None:
+    # From testing manually with requests 2.28.2, data overrides json if both
+    # supplied. However, this is probably a mistake on behalf of the caller, so
+    # we choose to throw an error instead
+    raise ValueError("`data` and `json` cannot be supplied to REQUEST at the same time")
+
+
 @unimplemented
 # ^ This excludes this function from autocomplete while in beta
 # and marks it as unimplemented in the docs.
 # It also makes grist-help expect to see the string 'raise NotImplemented' in the function source,
 # which it does now, because of this comment. Removing this comment will currently break the docs.
-def REQUEST(url, params=None, headers=None):
+def REQUEST(url, params=None, headers=None, method="GET", data=None, json=None):
-  # Makes a GET HTTP request with an API similar to `requests.get`.
+  # Makes an HTTP request with an API similar to `requests.request`.
   # Actually jumps through hoops internally to make the request asynchronously (usually)
   # while feeling synchronous to the formula writer.
 
+  # When making a POST or PUT request, REQUEST supports `data` and `json` args, from `requests.request`:
+  #   - `args` as str: Used as the request body
+  #   - `args` as other types: Form encoded and used as the request body. The correct header is also set.
+  #   - `json` as str: Used as the request body. The correct header is also set.
+  #   - `json` as other types: JSON encoded and set as the request body. The correct header is also set.
+  body, _headers = _replicate_requests_body_args(data=data, json=json)
+
+  # Extra headers that make us consistent with requests.post must not override
+  # user-supplied headers.
+  _headers.update(headers or {})
+
   # Requests are identified by a string key in various places.
   # The same arguments should produce the same key so the request is only made once.
-  args = dict(url=url, params=params, headers=headers)
+  args = dict(url=url, params=params, headers=_headers, method=method, body=body)
-  args_json = json.dumps(args, sort_keys=True)
+
+  args_json = json_module.dumps(args, sort_keys=True)
   key = hashlib.sha256(args_json.encode()).hexdigest()
 
   # This may either return the raw response data or it may raise a special exception
@@ -701,7 +752,7 @@ class Response(object):
     return self.content.decode(self.encoding)
 
   def json(self, **kwargs):
-    return json.loads(self.text, **kwargs)
+    return json_module.loads(self.text, **kwargs)
 
   @property
   def ok(self):

sandbox/grist/test_requests.py

@@ -4,6 +4,7 @@ import unittest
 import test_engine
 import testutil
 from functions import CaseInsensitiveDict, Response, HTTPError
+from functions.info import _replicate_requests_body_args
 
 
 class TestCaseInsensitiveDict(unittest.TestCase):
@@ -73,6 +74,45 @@ class TestResponse(unittest.TestCase):
     self.assertEqual(r.text, text)
 
 
+class TestRequestsPostInterface(unittest.TestCase):
+    def test_no_post_args(self):
+        body, headers = _replicate_requests_body_args()
+
+        assert body is None
+        assert headers == {}
+
+    def test_data_as_dict(self):
+        body, headers = _replicate_requests_body_args(data={"foo": "bar"})
+
+        assert body == "foo=bar"
+        assert headers == {"Content-Type": "application/x-www-form-urlencoded"}
+
+    def test_data_as_string(self):
+        body, headers = _replicate_requests_body_args(data="some_content")
+
+        assert body == "some_content"
+        assert headers == {}
+
+    def test_json_as_dict(self):
+        body, headers = _replicate_requests_body_args(json={"foo": "bar"})
+
+        assert body == '{"foo": "bar"}'
+        assert headers == {"Content-Type": "application/json"}
+
+    def test_json_as_string(self):
+        body, headers = _replicate_requests_body_args(json="invalid_but_ignored")
+
+        assert body == "invalid_but_ignored"
+        assert headers == {"Content-Type": "application/json"}
+
+    def test_data_and_json_together(self):
+        with self.assertRaises(ValueError):
+            body, headers = _replicate_requests_body_args(
+                json={"foo": "bar"},
+                data={"quux": "jazz"}
+            )
+
+
 class TestRequestFunction(test_engine.EngineTestCase):
   sample = testutil.parse_test_sample({
     "SCHEMA": [
@@ -98,12 +138,14 @@ r = REQUEST('my_url', headers={'foo': 'bar'}, params={'b': 1, 'a': 2})
 r.__dict__
 """
     out_actions = self.modify_column("Table1", "Request", formula=formula)
-    key = '9d305be9664924aaaf7ebb0bab2e4155d1fa1b9dcde53e417f1a9f9a2c7e09b9'
+    key = 'd7f8cedf177ab538bf7dadf66e77a525486a29a41ce4520b2c89a33e39095fed'
     deps = {'Table1': {'Request': [1, 2]}}
     args = {
       'url': 'my_url',
       'headers': {'foo': 'bar'},
       'params': {'a': 2, 'b': 1},
+      'method': 'GET',
+      'body': None,
       'deps': deps,
     }
     self.assertEqual(out_actions.requests, {key: args})

static/locales/en.client.json

@@ -76,7 +76,10 @@
         "Activation": "Activation",
         "Billing Account": "Billing Account",
         "Support Grist": "Support Grist",
-        "Upgrade Plan": "Upgrade Plan"
+        "Upgrade Plan": "Upgrade Plan",
+        "Sign In": "Sign In",
+        "Sign Up": "Sign Up",
+        "Use This Template": "Use This Template"
     },
     "ViewAsDropdown": {
         "View As": "View As",
@@ -112,7 +115,8 @@
         "Home Page": "Home Page",
         "Legacy": "Legacy",
         "Personal Site": "Personal Site",
-        "Team Site": "Team Site"
+        "Team Site": "Team Site",
+        "Grist Templates": "Grist Templates"
     },
     "AppModel": {
         "This team site is suspended. Documents can be read, but not modified.": "This team site is suspended. Documents can be read, but not modified."

test/nbrowser/CellColor.ts

@@ -306,6 +306,7 @@ describe('CellColor', function() {
     // Empty cell to clear error from converting toggle to date
     await cell.click();
     await driver.sendKeys(Key.DELETE);
+    await gu.waitAppFocus(true);
 
     const clip = cell.find('.field_clip');
 

test/nbrowser/SelectByRefList.ts

@@ -232,7 +232,7 @@ async function checkSelectingRecords(selectBy: string, sourceData: string[][], n
   for (let rowNum = 1; rowNum <= 3; rowNum++) {
     // Click an anchor link
     const anchorCell = gu.getCell({section: "Anchors", rowNum, col: 1});
-    await anchorCell.find('.test-tb-link').click();
+    await driver.withActions(a => a.click(anchorCell.find('.test-tb-link')));
 
     // Check that navigation to the link target worked
     assert.equal(await gu.getActiveSectionTitle(), "LINKTARGET");

test/nbrowser/SelectBySummary.ts

@@ -1,25 +1,26 @@
 import * as _ from 'lodash';
-import {addToRepl, assert, driver} from 'mocha-webdriver';
+import {assert, driver} from 'mocha-webdriver';
 import {enterRulePart, findDefaultRuleSet} from 'test/nbrowser/aclTestUtils';
 import * as gu from 'test/nbrowser/gristUtils';
-import {server, setupTestSuite} from 'test/nbrowser/testUtils';
+import {setupTestSuite} from 'test/nbrowser/testUtils';
 
 describe('SelectBySummary', function() {
   this.timeout(50000);
-  setupTestSuite();
+  const cleanup = setupTestSuite();
-  addToRepl('gu2', gu);
+  let headers: Record<string, string>;
   gu.bigScreen();
 
   before(async function() {
-    await server.simulateLogin("Chimpy", "chimpy@getgrist.com", 'nasa');
+    const session = await gu.session().teamSite.login();
-    const doc = await gu.importFixturesDoc('chimpy', 'nasa', 'Horizon',
+    await session.tempDoc(cleanup, 'SelectBySummary.grist');
-      'SelectBySummary.grist', false);
+    headers = {
-    await driver.get(`${server.getHost()}/o/nasa/doc/${doc.id}`);
+      Authorization: `Bearer ${session.getApiKey()}`
-    await gu.waitForDocToLoad();
+    };
   });
 
-  it('should filter a source table selected by a summary table', async function() {
+  it('should filter a source table selected by a summary table (first option)', async function() {
     await checkSelectingRecords(
+      headers,
       ['onetwo'],
       [
         '1', '16',
@@ -40,8 +41,11 @@ describe('SelectBySummary', function() {
         ],
       ],
     );
+  });
 
+  it('should filter a source table selected by a summary table (second option)', async function() {
     await checkSelectingRecords(
+      headers,
       ['choices'],
       [
         'a', '14',
@@ -67,9 +71,11 @@ describe('SelectBySummary', function() {
         ],
       ],
     );
+  });
 
-
+  it('should filter a source table selected by a summary table (both options)', async function() {
     await checkSelectingRecords(
+      headers,
       ['onetwo', 'choices'],
       [
         '1', 'a', '6',
@@ -104,7 +110,6 @@ describe('SelectBySummary', function() {
         ],
       ],
     );
-
   });
 
   it('should create new rows in the source table (link target) with correct default values',
@@ -153,6 +158,7 @@ describe('SelectBySummary', function() {
     // selecting by the two less detailed summaries.
     // There was a bug previously that this would not work while the summary source table (Table1) was hidden.
     await checkSelectingRecords(
+      headers,
       ['onetwo'],
       [
         '1', '16',
@@ -175,6 +181,7 @@ describe('SelectBySummary', function() {
     );
 
     await checkSelectingRecords(
+      headers,
       ['choices'],
       [
         'a', '14',
@@ -208,6 +215,7 @@ describe('SelectBySummary', function() {
  * to the corresponding subarray of `targetData`.
  */
 async function checkSelectingRecords(
+  headers: Record<string, string>,
   groubyColumns: string[],
   summaryData: string[],
   targetData: string[][],
@@ -243,7 +251,7 @@ async function checkSelectingRecords(
     );
     if (targetSection === 'TABLE1') {
       assert.equal(await countCell.getText(), numTargetRows.toString());
-      const csvCells = await gu.downloadSectionCsvGridCells(targetSection);
+      const csvCells = await gu.downloadSectionCsvGridCells(targetSection, headers);
       // visible cells text uses newlines to separate list items, CSV export uses commas
       const expectedCsvCells = targetGroup.map(s => s.replace("\n", ", "));
       assert.deepEqual(csvCells, expectedCsvCells);
@@ -259,7 +267,7 @@ async function checkSelectingRecords(
     for (let rowNum = 1; rowNum <= 8; rowNum++) {
       // Click an anchor link
       const anchorCell = gu.getCell({section: "Anchors", rowNum, col: 1});
-      await anchorCell.find('.test-tb-link').click();
+      await driver.withActions(a => a.click(anchorCell.find('.test-tb-link')));
 
       // Check that navigation to the link target worked
       assert.equal(await gu.getActiveSectionTitle(), "TABLE1");

test/nbrowser/gristUtils.ts

@@ -61,6 +61,7 @@ export const uploadFixtureDoc = homeUtil.uploadFixtureDoc.bind(homeUtil);
 export const getWorkspaceId = homeUtil.getWorkspaceId.bind(homeUtil);
 export const listDocs = homeUtil.listDocs.bind(homeUtil);
 export const createHomeApi = homeUtil.createHomeApi.bind(homeUtil);
+export const getApiKey = homeUtil.getApiKey.bind(homeUtil);
 export const simulateLogin = homeUtil.simulateLogin.bind(homeUtil);
 export const removeLogin = homeUtil.removeLogin.bind(homeUtil);
 export const enableTips = homeUtil.enableTips.bind(homeUtil);
@@ -2047,6 +2048,13 @@ export class Session {
     return createHomeApi(this.settings.name, this.settings.orgDomain, this.settings.email);
   }
 
+  public getApiKey(): string|null {
+    if (this.settings.email === 'anon@getgrist.com') {
+      return getApiKey(null);
+    }
+    return getApiKey(this.settings.name, this.settings.email);
+  }
+
   // Get the id of this user.
   public async getUserId(): Promise<number> {
     await this.login();

test/nbrowser/homeUtil.ts

@@ -316,9 +316,14 @@ export class HomeUtil {
   // A helper to create a UserAPI instance for a given useranme and org, that targets the home server
   // Username can be null for anonymous access.
   public createHomeApi(username: string|null, org: string, email?: string): UserAPIImpl {
+    const apiKey = this.getApiKey(username, email);
+    return this._createHomeApiUsingApiKey(apiKey, org);
+  }
+
+  public getApiKey(username: string|null, email?: string): string | null {
     const name = (username || '').toLowerCase();
     const apiKey = username && ((email && this._apiKey.get(email)) || `api_key_for_${name}`);
-    return this._createHomeApiUsingApiKey(apiKey, org);
+    return apiKey;
   }
 
   /**

test/test_under_docker.sh

@@ -41,6 +41,7 @@ docker run --name $DOCKER_CONTAINER --rm \
   --env GRIST_LOG_LEVEL=$GRIST_LOG_LEVEL \
   --env GRIST_LOG_SKIP_HTTP=${DEBUG:-false} \
   --env TEST_SUPPORT_API_KEY=api_key_for_support \
+  --env GRIST_TEMPLATE_ORG=templates \
   ${TEST_IMAGE:-gristlabs/grist} &
 
 DOCKER_PID="$!"