(core) updates from grist-core

app/client/ui/PageWidgetPicker.ts

@@ -1,5 +1,6 @@
 import {BehavioralPromptsManager} from 'app/client/components/BehavioralPromptsManager';
 import {GristDoc} from 'app/client/components/GristDoc';
+import {FocusLayer} from 'app/client/lib/FocusLayer';
 import {makeT} from 'app/client/lib/localization';
 import {reportError} from 'app/client/models/AppModel';
 import {ColumnRec, TableRec, ViewSectionRec} from 'app/client/models/DocModel';
@@ -260,8 +261,7 @@ export function buildPageWidgetPicker(
     dom.create(PageWidgetSelect,
       value, tables, columns, onSaveCB, behavioralPromptsManager, options),
 
-    // gives focus and binds keydown events
-    (elem: any) => { setTimeout(() => elem.focus(), 0); },
+    elem => { FocusLayer.create(ctl, {defaultFocusElem: elem, pauseMousetrap: true}); },
     onKeyDown({
       Escape: () => ctl.close(),
       Enter: () => isValid() && onSaveCB()

app/client/widgets/FormulaAssistant.ts

@@ -35,7 +35,7 @@ import {Marked} from 'marked';
 import {markedHighlight} from 'marked-highlight';
 import {v4 as uuidv4} from 'uuid';
 
-const t = makeT('FormulaEditor');
+const t = makeT('FormulaAssistant');
 const testId = makeTestId('test-formula-editor-');
 
 const LOW_CREDITS_WARNING_BANNER_THRESHOLD = 10;

app/gen-server/ApiServer.ts

@@ -429,7 +429,7 @@ export class ApiServer {
         throw new ApiError('Name expected in the body', 400);
       }
       const name = req.body.name;
-      await this._dbManager.updateUserName(userId, name);
+      await this._dbManager.updateUser(userId, { name });
       res.sendStatus(200);
     }));
 

app/gen-server/lib/homedb/HomeDBManager.ts

@@ -56,7 +56,7 @@ import {
   readJson
 } from 'app/gen-server/sqlUtils';
 import {appSettings} from 'app/server/lib/AppSettings';
-import {getOrCreateConnection} from 'app/server/lib/dbUtils';
+import {createNewConnection, getOrCreateConnection} from 'app/server/lib/dbUtils';
 import {makeId} from 'app/server/lib/idUtils';
 import log from 'app/server/lib/log';
 import {Permit} from 'app/server/lib/Permit';
@@ -70,6 +70,7 @@ import {
   Brackets,
   Connection,
   DatabaseType,
+  DataSourceOptions,
   EntityManager,
   ObjectLiteral,
   SelectQueryBuilder,
@@ -248,7 +249,6 @@ export type BillingOptions = Partial<Pick<BillingAccount,
 export class HomeDBManager extends EventEmitter {
   private _usersManager = new UsersManager(this, this._runInTransaction.bind(this));
   private _connection: Connection;
-  private _dbType: DatabaseType;
   private _exampleWorkspaceId: number;
   private _exampleOrgId: number;
   private _idPrefix: string = "";  // Place this before ids in subdomains, used in routing to
@@ -258,6 +258,10 @@ export class HomeDBManager extends EventEmitter {
   // In restricted mode, documents should be read-only.
   private _restrictedMode: boolean = false;
 
+  private get _dbType(): DatabaseType {
+    return this._connection.driver.options.type;
+  }
+
   /**
    * Five aclRules, each with one group (with the names 'owners', 'editors', 'viewers',
    * 'guests', and 'members') are created by default on every new entity (Organization,
@@ -348,7 +352,10 @@ export class HomeDBManager extends EventEmitter {
 
   public async connect(): Promise<void> {
     this._connection = await getOrCreateConnection();
-    this._dbType = this._connection.driver.options.type;
+  }
+
+  public async createNewConnection(overrideConf?: Partial<DataSourceOptions>): Promise<void> {
+    this._connection = await createNewConnection(overrideConf);
   }
 
   // make sure special users and workspaces are available
@@ -461,10 +468,6 @@ export class HomeDBManager extends EventEmitter {
     }
   }
 
-  public async updateUserName(userId: number, name: string) {
-    return this._usersManager.updateUserName(userId, name);
-  }
-
   public async updateUserOptions(userId: number, props: Partial<UserOptions>) {
     return this._usersManager.updateUserOptions(userId, props);
   }
@@ -472,14 +475,14 @@ export class HomeDBManager extends EventEmitter {
   /**
    * @see UsersManager.prototype.getUserByLoginWithRetry
    */
-  public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise<User|undefined> {
+  public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise<User> {
     return this._usersManager.getUserByLoginWithRetry(email, options);
   }
 
   /**
    * @see UsersManager.prototype.getUserByLogin
    */
-  public async getUserByLogin(email: string, options: GetUserOptions = {}): Promise<User|undefined> {
+  public async getUserByLogin(email: string, options: GetUserOptions = {}): Promise<User> {
     return this._usersManager.getUserByLogin(email, options);
   }
 
@@ -4362,7 +4365,6 @@ export class HomeDBManager extends EventEmitter {
     });
     return verifyEntity(orgQuery);
   }
-
 }
 
 // Return a QueryResult reflecting the output of a query builder.

app/gen-server/lib/homedb/Interfaces.ts

@@ -34,7 +34,7 @@ export type NonGuestGroup = Group & { name: roles.NonGuestRole };
 
 export type Resource = Organization|Workspace|Document;
 
-export type RunInTransaction = (
+export type RunInTransaction = <T>(
   transaction: EntityManager|undefined,
-  op: ((manager: EntityManager) => Promise<any>)
-) => Promise<any>;
+  op: ((manager: EntityManager) => Promise<T>)
+) => Promise<T>;

app/gen-server/lib/homedb/UsersManager.ts

@@ -97,7 +97,7 @@ export class UsersManager {
   public async testClearUserPrefs(emails: string[]) {
     return await this._connection.transaction(async manager => {
       for (const email of emails) {
-        const user = await this.getUserByLogin(email, {manager});
+        const user = await this.getExistingUserByLogin(email, manager);
         if (user) {
           await manager.delete(Pref, {userId: user.id});
         }
@@ -116,7 +116,7 @@ export class UsersManager {
    */
   public getAnonymousUserId(): number {
     const id = this._specialUserIds[ANONYMOUS_USER_EMAIL];
-    if (!id) { throw new Error("Anonymous user not available"); }
+    if (!id) { throw new Error("'Anonymous' user not available"); }
     return id;
   }
 
@@ -125,7 +125,7 @@ export class UsersManager {
    */
   public getPreviewerUserId(): number {
     const id = this._specialUserIds[PREVIEWER_EMAIL];
-    if (!id) { throw new Error("Previewer user not available"); }
+    if (!id) { throw new Error("'Previewer' user not available"); }
     return id;
   }
 
@@ -134,7 +134,7 @@ export class UsersManager {
    */
   public getEveryoneUserId(): number {
     const id = this._specialUserIds[EVERYONE_EMAIL];
-    if (!id) { throw new Error("'everyone' user not available"); }
+    if (!id) { throw new Error("'Everyone' user not available"); }
     return id;
   }
 
@@ -143,7 +143,7 @@ export class UsersManager {
    */
   public getSupportUserId(): number {
     const id = this._specialUserIds[SUPPORT_EMAIL];
-    if (!id) { throw new Error("'support' user not available"); }
+    if (!id) { throw new Error("'Support' user not available"); }
     return id;
   }
 
@@ -221,9 +221,6 @@ export class UsersManager {
           profile,
           manager
         });
-        if (!newUser) {
-          throw new ApiError("Unable to create user", 500);
-        }
         // No need to survey this user.
         newUser.isFirstTimeUser = false;
         await newUser.save();
@@ -286,13 +283,7 @@ export class UsersManager {
     return { user, isWelcomed };
   }
 
-  public async updateUserName(userId: number, name: string) {
-    const user = await User.findOne({where: {id: userId}});
-    if (!user) { throw new ApiError("unable to find user", 400); }
-    user.name = name;
-    await user.save();
-  }
-
+  // TODO: rather use the updateUser() method, if that makes sense?
   public async updateUserOptions(userId: number, props: Partial<UserOptions>) {
     const user = await User.findOne({where: {id: userId}});
     if (!user) { throw new ApiError("unable to find user", 400); }
@@ -321,7 +312,7 @@ export class UsersManager {
   // for an email key conflict failure. This is in case our transaction conflicts with a peer
   // doing the same thing. This is quite likely if the first page visited by a previously
   // unseen user fires off multiple api calls.
-  public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise<User|undefined> {
+  public async getUserByLoginWithRetry(email: string, options: GetUserOptions = {}): Promise<User> {
     try {
       return await this.getUserByLogin(email, options);
     } catch (e) {
@@ -361,10 +352,10 @@ export class UsersManager {
    * unset/outdated fields of an existing record.
    *
    */
-  public async getUserByLogin(email: string, options: GetUserOptions = {}): Promise<User|undefined> {
+  public async getUserByLogin(email: string, options: GetUserOptions = {}) {
     const {manager: transaction, profile, userOptions} = options;
     const normalizedEmail = normalizeEmail(email);
-    const userByLogin = await this._runInTransaction(transaction, async manager => {
+    return await this._runInTransaction(transaction, async manager => {
       let needUpdate = false;
       const userQuery = manager.createQueryBuilder()
         .select('user')
@@ -473,9 +464,8 @@ export class UsersManager {
         // In principle this could be optimized, but this is simpler to maintain.
         user = await userQuery.getOne();
       }
-      return user;
+      return user!;
     });
-    return userByLogin;
   }
 
   /**
@@ -520,6 +510,63 @@ export class UsersManager {
     };
   }
 
+  public async initializeSpecialIds(): Promise<void> {
+    await this._maybeCreateSpecialUserId({
+      email: ANONYMOUS_USER_EMAIL,
+      name: "Anonymous"
+    });
+    await this._maybeCreateSpecialUserId({
+      email: PREVIEWER_EMAIL,
+      name: "Preview"
+    });
+    await this._maybeCreateSpecialUserId({
+      email: EVERYONE_EMAIL,
+      name: "Everyone"
+    });
+    await this._maybeCreateSpecialUserId({
+      email: SUPPORT_EMAIL,
+      name: "Support"
+    });
+  }
+
+  /**
+   *
+   * Take a list of user profiles coming from the client's session, correlate
+   * them with Users and Logins in the database, and construct full profiles
+   * with user ids, standardized display emails, pictures, and anonymous flags.
+   *
+   */
+  public async completeProfiles(profiles: UserProfile[]): Promise<FullUser[]> {
+    if (profiles.length === 0) { return []; }
+    const qb = this._connection.createQueryBuilder()
+      .select('logins')
+      .from(Login, 'logins')
+      .leftJoinAndSelect('logins.user', 'user')
+      .where('logins.email in (:...emails)', {emails: profiles.map(profile => normalizeEmail(profile.email))});
+    const completedProfiles: {[email: string]: FullUser} = {};
+    for (const login of await qb.getMany()) {
+      completedProfiles[login.email] = {
+        id: login.user.id,
+        email: login.displayEmail,
+        name: login.user.name,
+        picture: login.user.picture,
+        anonymous: login.user.id === this.getAnonymousUserId(),
+        locale: login.user.options?.locale
+      };
+    }
+    return profiles.map(profile => completedProfiles[normalizeEmail(profile.email)])
+      .filter(fullProfile => fullProfile);
+  }
+
+  /**
+   * ==================================
+   *
+   * Below methods are public but not exposed by HomeDBManager
+   *
+   * They are meant to be used internally (i.e. by homedb/ modules)
+   *
+   */
+
   // Looks up the emails in the permission delta and adds them to the users map in
   // the delta object.
   // Returns a QueryResult based on the validity of the passed in PermissionDelta object.
@@ -589,25 +636,6 @@ export class UsersManager {
     };
   }
 
-  public async initializeSpecialIds(): Promise<void> {
-    await this._maybeCreateSpecialUserId({
-      email: ANONYMOUS_USER_EMAIL,
-      name: "Anonymous"
-    });
-    await this._maybeCreateSpecialUserId({
-      email: PREVIEWER_EMAIL,
-      name: "Preview"
-    });
-    await this._maybeCreateSpecialUserId({
-      email: EVERYONE_EMAIL,
-      name: "Everyone"
-    });
-    await this._maybeCreateSpecialUserId({
-      email: SUPPORT_EMAIL,
-      name: "Support"
-    });
-  }
-
   /**
    * Check for anonymous user, either encoded directly as an id, or as a singular
    * profile (this case arises during processing of the session/access/all endpoint
@@ -684,34 +712,6 @@ export class UsersManager {
     return members;
   }
 
-  /**
-   *
-   * Take a list of user profiles coming from the client's session, correlate
-   * them with Users and Logins in the database, and construct full profiles
-   * with user ids, standardized display emails, pictures, and anonymous flags.
-   *
-   */
-  public async completeProfiles(profiles: UserProfile[]): Promise<FullUser[]> {
-    if (profiles.length === 0) { return []; }
-    const qb = this._connection.createQueryBuilder()
-      .select('logins')
-      .from(Login, 'logins')
-      .leftJoinAndSelect('logins.user', 'user')
-      .where('logins.email in (:...emails)', {emails: profiles.map(profile => normalizeEmail(profile.email))});
-    const completedProfiles: {[email: string]: FullUser} = {};
-    for (const login of await qb.getMany()) {
-      completedProfiles[login.email] = {
-        id: login.user.id,
-        email: login.displayEmail,
-        name: login.user.name,
-        picture: login.user.picture,
-        anonymous: login.user.id === this.getAnonymousUserId(),
-        locale: login.user.options?.locale
-      };
-    }
-    return profiles.map(profile => completedProfiles[normalizeEmail(profile.email)])
-      .filter(profile => profile);
-  }
 
   // For the moment only the support user can add both everyone@ and anon@ to a
   // resource, since that allows spam. TODO: enhance or remove.
@@ -735,7 +735,7 @@ export class UsersManager {
       // user if a bunch of servers start simultaneously and the user doesn't exist
       // yet.
       const user = await this.getUserByLoginWithRetry(profile.email, {profile});
-      if (user) { id = this._specialUserIds[profile.email] = user.id; }
+      id = this._specialUserIds[profile.email] = user.id;
     }
     if (!id) { throw new Error(`Could not find or create user ${profile.email}`); }
     return id;

app/server/companion.ts

@@ -166,10 +166,6 @@ export function addSiteCommand(program: commander.Command,
       const profile = {email, name: email};
       const db = await getHomeDBManager();
       const user = await db.getUserByLogin(email, {profile});
-      if (!user) {
-        // This should not happen.
-        throw new Error('failed to create user');
-      }
       db.unwrapQueryResult(await db.addOrg(user, {
         name: domain,
         domain,

app/server/lib/DocApi.ts

@@ -972,11 +972,12 @@ export class DocWorkerApi {
 
     // Reload a document forcibly (in fact this closes the doc, it will be automatically
     // reopened on use).
-    this._app.post('/api/docs/:docId/force-reload', canEdit, throttled(async (req, res) => {
-      const activeDoc = await this._getActiveDoc(req);
+    this._app.post('/api/docs/:docId/force-reload', canEdit, async (req, res) => {
+      const mreq = req as RequestWithLogin;
+      const activeDoc = await this._getActiveDoc(mreq);
       await activeDoc.reloadDoc();
       res.json(null);
-    }));
+    });
 
     this._app.post('/api/docs/:docId/recover', canEdit, throttled(async (req, res) => {
       const recoveryModeRaw = req.body.recoveryMode;

app/server/lib/OIDCConfig.ts

@@ -47,7 +47,9 @@
  *        A JSON object with extra client metadata to pass to openid-client. Optional.
  *        Be aware that setting this object may override any other values passed to the openid client.
  *        More info: https://github.com/panva/node-openid-client/tree/main/docs#new-clientmetadata-jwks-options
- *
+ *    env GRIST_OIDC_SP_HTTP_TIMEOUT
+ *        The timeout in milliseconds for HTTP requests to the IdP. The default value is set to 3500 by the
+ *        openid-client library. See: https://github.com/panva/node-openid-client/blob/main/docs/README.md#customizing-http-requests
  *
  * This version of OIDCConfig has been tested with Keycloak OIDC IdP following the instructions
  * at:
@@ -66,7 +68,7 @@
 import * as express from 'express';
 import { GristLoginSystem, GristServer } from './GristServer';
 import {
-  Client, ClientMetadata, Issuer, errors as OIDCError, TokenSet, UserinfoResponse
+  Client, ClientMetadata, custom, Issuer, errors as OIDCError, TokenSet, UserinfoResponse
 } from 'openid-client';
 import { Sessions } from './Sessions';
 import log from 'app/server/lib/log';
@@ -137,6 +139,9 @@ export class OIDCConfig {
       envVar: 'GRIST_OIDC_IDP_CLIENT_SECRET',
       censor: true,
     });
+    const httpTimeout = section.flag('httpTimeout').readInt({
+      envVar: 'GRIST_OIDC_SP_HTTP_TIMEOUT',
+    });
     this._namePropertyKey = section.flag('namePropertyKey').readString({
       envVar: 'GRIST_OIDC_SP_PROFILE_NAME_ATTR',
     });
@@ -173,6 +178,9 @@ export class OIDCConfig {
     this._protectionManager = new ProtectionsManager(enabledProtections);
 
     this._redirectUrl = new URL(CALLBACK_URL, spHost).href;
+    custom.setHttpOptionsDefaults({
+      ...(httpTimeout !== undefined ? {timeout: httpTimeout} : {}),
+    });
     await this._initClient({ issuerUrl, clientId, clientSecret, extraMetadata });
 
     if (this._client.issuer.metadata.end_session_endpoint === undefined &&

app/server/lib/TestLogin.ts

@@ -25,10 +25,8 @@ export async function getTestLoginSystem(): Promise<GristLoginSystem> {
           if (process.env.TEST_SUPPORT_API_KEY) {
             const dbManager = gristServer.getHomeDBManager();
             const user = await dbManager.getUserByLogin(SUPPORT_EMAIL);
-            if (user) {
-              user.apiKey = process.env.TEST_SUPPORT_API_KEY;
-              await user.save();
-            }
+            user.apiKey = process.env.TEST_SUPPORT_API_KEY;
+            await user.save();
           }
           return "test-login";
         },

app/server/lib/dbUtils.ts

@@ -45,38 +45,53 @@ export async function updateDb(connection?: Connection) {
   await synchronizeProducts(connection, true);
 }
 
+export function getConnectionName() {
+  return process.env.TYPEORM_NAME || 'default';
+}
+
 /**
  * Get a connection to db if one exists, or create one. Serialized to
  * avoid duplication.
  */
 const connectionMutex = new Mutex();
+
+async function buildConnection(overrideConf?: Partial<DataSourceOptions>) {
+  const settings = getTypeORMSettings(overrideConf);
+  const connection = await createConnection(settings);
+  // When using Sqlite, set a busy timeout of 3s to tolerate a little
+  // interference from connections made by tests. Logging doesn't show
+  // any particularly slow queries, but bad luck is possible.
+  // This doesn't affect when Postgres is in use. It also doesn't have
+  // any impact when there is a single connection to the db, as is the
+  // case when Grist is run as a single process.
+  if (connection.driver.options.type === 'sqlite') {
+    await connection.query('PRAGMA busy_timeout = 3000');
+  }
+  return connection;
+}
+
 export async function getOrCreateConnection(): Promise<Connection> {
-  return connectionMutex.runExclusive(async() => {
+  return connectionMutex.runExclusive(async () => {
     try {
       // If multiple servers are started within the same process, we
       // share the database connection.  This saves locking trouble
       // with Sqlite.
-      const connection = getConnection();
-      return connection;
+      return getConnection(getConnectionName());
     } catch (e) {
       if (!String(e).match(/ConnectionNotFoundError/)) {
         throw e;
       }
-      const connection = await createConnection(getTypeORMSettings());
-      // When using Sqlite, set a busy timeout of 3s to tolerate a little
-      // interference from connections made by tests. Logging doesn't show
-      // any particularly slow queries, but bad luck is possible.
-      // This doesn't affect when Postgres is in use. It also doesn't have
-      // any impact when there is a single connection to the db, as is the
-      // case when Grist is run as a single process.
-      if (connection.driver.options.type === 'sqlite') {
-        await connection.query('PRAGMA busy_timeout = 3000');
-      }
-      return connection;
+      return buildConnection();
     }
   });
 }
 
+export async function createNewConnection(overrideConf?: Partial<DataSourceOptions>): Promise<Connection> {
+  return connectionMutex.runExclusive(async () => {
+    return buildConnection(overrideConf);
+  });
+}
+
 export async function runMigrations(connection: Connection) {
   // on SQLite, migrations fail if we don't temporarily disable foreign key
   // constraint checking.  This is because for sqlite typeorm copies each
@@ -103,7 +118,7 @@ export async function undoLastMigration(connection: Connection) {
 // Replace the old janky ormconfig.js file, which was always a source of
 // pain to use since it wasn't properly integrated into the typescript
 // project.
-export function getTypeORMSettings(): DataSourceOptions {
+export function getTypeORMSettings(overrideConf?: Partial<DataSourceOptions>): DataSourceOptions {
   // If we have a redis server available, tell typeorm.  Then any queries built with
   // .cache() called on them will be cached via redis.
   // We use a separate environment variable for the moment so that we don't have to
@@ -120,7 +135,7 @@ export function getTypeORMSettings(): DataSourceOptions {
   } : undefined;
 
   return {
-    "name": process.env.TYPEORM_NAME || "default",
+    "name": getConnectionName(),
     "type": (process.env.TYPEORM_TYPE as any) || "sqlite",  // officially, TYPEORM_CONNECTION -
                                                    // but if we use that, this file will never
                                                    // be read, and we can't configure
@@ -144,5 +159,6 @@ export function getTypeORMSettings(): DataSourceOptions {
     ],
     ...JSON.parse(process.env.TYPEORM_EXTRA || "{}"),
     ...cache,
+    ...overrideConf,
   };
 }