diff --git a/app/client/models/DocPageModel.ts b/app/client/models/DocPageModel.ts index 453840fd..2fb0a91f 100644 --- a/app/client/models/DocPageModel.ts +++ b/app/client/models/DocPageModel.ts @@ -299,7 +299,8 @@ Recovery mode opens the document to be fully accessible to owners, and inaccessi It also disables formulas. [{{error}}]", {error: err.message}) : isDenied ? t('Sorry, access to this document has been denied. [{{error}}]', {error: err.message}) - : t("Document owners can attempt to recover the document. [{{error}}]", {error: err.message}) + : t("Please reload the document and if the error persist, "+ + "contact the document owners to attempt a document recovery. [{{error}}]", {error: err.message}) ), hideCancel: true, extraButtons: !(isDocOwner && !isDenied) ? null : bigBasicButton( diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts index 110183e8..86f78bce 100644 --- a/app/server/lib/OIDCConfig.ts +++ b/app/server/lib/OIDCConfig.ts @@ -26,6 +26,9 @@ * If omitted, the name will either be the concatenation of "given_name" + "family_name" or the "name" attribute. * env GRIST_OIDC_SP_PROFILE_EMAIL_ATTR * The key of the attribute to use for the user's email. Defaults to "email". + * env GRIST_OIDC_IDP_END_SESSION_ENDPOINT + * If set, overrides the IdP's end_session_endpoint with an alternative URL to redirect user upon logout + * (for an IdP that has a logout endpoint but does not support the OIDC RP-Initiated Logout specification). * env GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT * If set to "true", on logout, there won't be any attempt to call the IdP's end_session_endpoint * (the user will remain logged in in the IdP). @@ -63,6 +66,7 @@ export class OIDCConfig { private _redirectUrl: string; private _namePropertyKey?: string; private _emailPropertyKey: string; + private _endSessionEndpoint: string; private _skipEndSessionEndpoint: boolean; private _ignoreEmailVerified: boolean; @@ -94,6 +98,11 @@ export class OIDCConfig { defaultValue: 'email', }); + this._endSessionEndpoint = section.flag('endSessionEndpoint').readString({ + envVar: 'GRIST_OIDC_IDP_END_SESSION_ENDPOINT', + defaultValue: '', + })!; + this._skipEndSessionEndpoint = section.flag('skipEndSessionEndpoint').readBool({ envVar: 'GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT', defaultValue: false, @@ -112,9 +121,11 @@ export class OIDCConfig { redirect_uris: [ this._redirectUrl ], response_types: [ 'code' ], }); - if (this._client.issuer.metadata.end_session_endpoint === undefined && !this._skipEndSessionEndpoint) { + if (this._client.issuer.metadata.end_session_endpoint === undefined && + !this._endSessionEndpoint && !this._skipEndSessionEndpoint) { throw new Error('The Identity provider does not propose end_session_endpoint. ' + - 'If that is expected, please set GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true'); + 'If that is expected, please set GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true ' + + 'or provide an alternative logout URL in GRIST_OIDC_IDP_END_SESSION_ENDPOINT'); } log.info(`OIDCConfig: initialized with issuer ${issuerUrl}`); } @@ -187,6 +198,10 @@ export class OIDCConfig { if (this._skipEndSessionEndpoint) { return redirectUrl.href; } + // Alternatively, we could use a logout URL specified by configuration. + if (this._endSessionEndpoint) { + return this._endSessionEndpoint; + } return this._client.endSessionUrl({ post_logout_redirect_uri: redirectUrl.href }); diff --git a/app/server/lib/Sharing.ts b/app/server/lib/Sharing.ts index 5f410ab9..68afd9f2 100644 --- a/app/server/lib/Sharing.ts +++ b/app/server/lib/Sharing.ts @@ -22,6 +22,7 @@ import {ActionHistory, asActionGroup, getActionUndoInfo} from './ActionHistory'; import {ActiveDoc} from './ActiveDoc'; import {makeExceptionalDocSession, OptDocSession} from './DocSession'; import {WorkCoordinator} from './WorkCoordinator'; +import {summarizeAction} from 'app/common/ActionSummarizer'; // Describes the request to apply a UserActionBundle. It includes a Client (so that broadcast // message can set `.fromSelf` property), and methods to resolve or reject the promise for when @@ -246,14 +247,14 @@ export class Sharing { try { - const isCalculate = (userActions.length === 1 && SYSTEM_ACTIONS.has(userActions[0][0] as string)); + const isSystemAction = (userActions.length === 1 && SYSTEM_ACTIONS.has(userActions[0][0] as string)); // `internal` is true if users shouldn't be able to undo the actions. Applies to: // - Calculate/UpdateCurrentTime because it's not considered as performed by a particular client. // - Adding attachment metadata when uploading attachments, // because then the attachment file may get hard-deleted and redo won't work properly. // - Action was rejected but it had some side effects (e.g. NOW() or UUID() formulas). const internal = - isCalculate || + isSystemAction || userActions.every(a => a[0] === "AddRecord" && a[1] === "_grist_Attachments") || !!failure; @@ -305,7 +306,7 @@ export class Sharing { // If the document has shut down in the meantime, and this was just a "Calculate" action, // return a trivial result. This is just to reduce noisy warnings in migration tests. - if (this._activeDoc.isShuttingDown && isCalculate) { + if (this._activeDoc.isShuttingDown && isSystemAction) { return { actionNum: localActionBundle.actionNum, retValues: [], @@ -336,7 +337,12 @@ export class Sharing { } await this._activeDoc.processActionBundle(ownActionBundle); - const actionSummary = await this._activeDoc.handleTriggers(localActionBundle); + // Don't trigger webhooks for single Calculate actions, this causes a deadlock on document load. + // See gh issue #799 + const isSingleCalculateAction = userActions.length === 1 && userActions[0][0] === 'Calculate'; + const actionSummary = !isSingleCalculateAction ? + await this._activeDoc.handleTriggers(localActionBundle) : + summarizeAction(localActionBundle); // Opportunistically use actionSummary to see if _grist_Shares was // changed. diff --git a/app/server/lib/Triggers.ts b/app/server/lib/Triggers.ts index 313cedf4..561e4eee 100644 --- a/app/server/lib/Triggers.ts +++ b/app/server/lib/Triggers.ts @@ -174,7 +174,7 @@ export class DocTriggers { const triggersTable = docData.getMetaTable("_grist_Triggers"); const getTableId = docData.getMetaTable("_grist_Tables").getRowPropFunc("tableId"); - const triggersByTableRef = _.groupBy(triggersTable.getRecords(), "tableRef"); + const triggersByTableRef = _.groupBy(triggersTable.getRecords().filter(t => t.enabled), "tableRef"); const triggersByTableId: Array<[string, Trigger[]]> = []; // First we need a list of columns which must be included in full in the action summary @@ -545,9 +545,7 @@ export class DocTriggers { tableDelta: TableDelta, ): boolean { let readyBefore: boolean; - if (!trigger.enabled) { - return false; - } else if (!trigger.isReadyColRef) { + if (!trigger.isReadyColRef) { // User hasn't configured a column, so all records are considered ready immediately readyBefore = recordDelta.existedBefore; } else { diff --git a/package.json b/package.json index 48dd5d5b..487b547f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "grist-core", - "version": "1.1.9", + "version": "1.1.10", "license": "Apache-2.0", "description": "Grist is the evolution of spreadsheets", "homepage": "https://github.com/gristlabs/grist-core", diff --git a/static/locales/en.client.json b/static/locales/en.client.json index 94e2116a..c5d5342e 100644 --- a/static/locales/en.client.json +++ b/static/locales/en.client.json @@ -695,7 +695,8 @@ "TOOLS": "TOOLS", "Tour of this Document": "Tour of this Document", "Validate Data": "Validate Data", - "Settings": "Settings" + "Settings": "Settings", + "API Console": "API Console" }, "TopBar": { "Manage Team": "Manage Team" diff --git a/static/locales/es.client.json b/static/locales/es.client.json index fe5862c7..0b133b98 100644 --- a/static/locales/es.client.json +++ b/static/locales/es.client.json @@ -540,7 +540,8 @@ "Columns_other": "Columnas", "Fields_one": "Campo", "Fields_other": "Campos", - "Add referenced columns": "Añadir columnas referenciadas" + "Add referenced columns": "Añadir columnas referenciadas", + "Reset form": "Restablecer el formulario" }, "RowContextMenu": { "Copy anchor link": "Copiar enlace de anclaje", @@ -614,7 +615,8 @@ "TOOLS": "HERRAMIENTAS", "Tour of this Document": "Recorrido por este documento", "Validate Data": "Validar datos", - "Settings": "Ajustes" + "Settings": "Ajustes", + "API Console": "Consola de la API" }, "TopBar": { "Manage Team": "Administrar equipo" @@ -1309,5 +1311,8 @@ "Delete card": "Borrar la tarjeta", "Copy anchor link": "Copiar enlace fijado", "Insert card": "Insertar la tarjeta" + }, + "HiddenQuestionConfig": { + "Hidden fields": "Campos ocultos" } } diff --git a/static/locales/it.client.json b/static/locales/it.client.json index 24fd3f1d..ec01fcab 100644 --- a/static/locales/it.client.json +++ b/static/locales/it.client.json @@ -145,7 +145,8 @@ "Series_one": "Serie", "Series_other": "Serie", "Sort & Filter": "Ordina e filtra", - "Add referenced columns": "Aggiungi colonne referenziate" + "Add referenced columns": "Aggiungi colonne referenziate", + "Reset form": "Resetta modulo" }, "RowContextMenu": { "Copy anchor link": "Copia link", @@ -1255,5 +1256,8 @@ "Delete card": "Elimina scheda", "Copy anchor link": "Copia il link", "Insert card": "Inserisci scheda" + }, + "HiddenQuestionConfig": { + "Hidden fields": "Campi nascosti" } } diff --git a/static/locales/ro.client.json b/static/locales/ro.client.json index 1543c35e..dcdd839d 100644 --- a/static/locales/ro.client.json +++ b/static/locales/ro.client.json @@ -445,7 +445,8 @@ "TRANSFORM": "TRANSFORMĂ", "SELECTOR FOR": "SELECTOR PENTRU", "Sort & Filter": "Sortare și filtrare", - "Widget": "Widget" + "Widget": "Widget", + "Reset form": "Resetare formular" }, "FloatingPopup": { "Maximize": "Maximizați", @@ -1255,5 +1256,8 @@ }, "sendToDrive": { "Sending file to Google Drive": "Se trimite fișierul la Google Drive" + }, + "HiddenQuestionConfig": { + "Hidden fields": "Câmpuri ascunse" } } diff --git a/static/locales/sl.client.json b/static/locales/sl.client.json index 658aa80a..4d094714 100644 --- a/static/locales/sl.client.json +++ b/static/locales/sl.client.json @@ -283,7 +283,8 @@ "Validate Data": "Potrdi podatke", "How-to Tutorial": "Vadnica kako narediti", "Tour of this Document": "Ogled tega dokumenta", - "Return to viewing as yourself": "Vrnite se k ogledu kot vi" + "Return to viewing as yourself": "Vrnite se k ogledu kot vi", + "API Console": "API Konzola" }, "pages": { "Rename": "Preimenuj", @@ -578,7 +579,8 @@ "TRANSFORM": "TRANSFORM", "SELECTOR FOR": "SELEKTOR ZA", "Sort & Filter": "Razvrščanje in filtriranje", - "Widget": "Pripomoček" + "Widget": "Pripomoček", + "Reset form": "Ponastavi obrazec" }, "FloatingPopup": { "Maximize": "Povečajte", @@ -1255,5 +1257,8 @@ "Delete card": "Briši kartico", "Copy anchor link": "Kopiraj sidrno povezavo", "Insert card": "Vstavi kartico" + }, + "HiddenQuestionConfig": { + "Hidden fields": "Skrita polja" } } diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 74bc00a3..22401c6f 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -19,7 +19,7 @@ import {delayAbort} from 'app/server/lib/serverUtils'; import axios, {AxiosRequestConfig, AxiosResponse} from 'axios'; import {delay} from 'bluebird'; import {assert} from 'chai'; -import * as express from 'express'; +import express from 'express'; import FormData from 'form-data'; import * as fse from 'fs-extra'; import * as _ from 'lodash'; @@ -3347,7 +3347,6 @@ function testDocApi() { }); describe('webhooks related endpoints', async function () { - /* Regression test for old _subscribe endpoint. /docs/{did}/webhooks should be used instead to subscribe */ @@ -3543,8 +3542,6 @@ function testDocApi() { assert.equal(webhookList.length, 3); }); - - it("POST /docs/{did}/tables/{tid}/_unsubscribe validates inputs for editors", async function () { const subscribeResponse = await subscribeWebhook(); @@ -3603,6 +3600,7 @@ function testDocApi() { assert.equal(accessResp.status, 200); await flushAuth(); }); + }); describe("Daily API Limit", () => { @@ -3876,6 +3874,7 @@ function testDocApi() { eventTypes?: string[], name?: string, memo?: string, + enabled?: boolean, }) { // Subscribe helper that returns a method to unsubscribe. const {data, status} = await axios.post( @@ -3884,7 +3883,7 @@ function testDocApi() { eventTypes: options?.eventTypes ?? ['add', 'update'], url: `${serving.url}/${endpoint}`, isReadyColumn: options?.isReadyColumn === undefined ? 'B' : options?.isReadyColumn, - ...pick(options, 'name', 'memo'), + ...pick(options, 'name', 'memo', 'enabled'), }, chimpy ); assert.equal(status, 200); @@ -4010,34 +4009,41 @@ function testDocApi() { } }); - it("delivers expected payloads from combinations of changes, with retrying and batching", - async function () { - // Create a test document. - const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; - const docId = await userApi.newDoc({name: 'testdoc'}, ws1); - const doc = userApi.getDocAPI(docId); - - // For some reason B is turned into Numeric even when given bools + async function createWebhooks({docId, tableId, eventTypesSet, isReadyColumn, enabled}: + {docId: string, tableId: string, eventTypesSet: string[][], isReadyColumn: string, enabled?: boolean} + ) { + // Ensure the isReady column is a Boolean await axios.post(`${serverUrl}/api/docs/${docId}/apply`, [ - ['ModifyColumn', 'Table1', 'B', {type: 'Bool'}], + ['ModifyColumn', tableId, isReadyColumn, {type: 'Bool'}], ], chimpy); - // Make a webhook for every combination of event types const subscribeResponses = []; const webhookIds: Record = {}; - for (const eventTypes of [ - ["add"], - ["update"], - ["add", "update"], - ]) { - const {data, status} = await axios.post( - `${serverUrl}/api/docs/${docId}/tables/Table1/_subscribe`, - {eventTypes, url: `${serving.url}/${eventTypes}`, isReadyColumn: "B"}, chimpy - ); - assert.equal(status, 200); + + for (const eventTypes of eventTypesSet) { + const data = await subscribe(String(eventTypes), docId, {tableId, eventTypes, isReadyColumn, enabled}); subscribeResponses.push(data); webhookIds[data.webhookId] = String(eventTypes); } + return {subscribeResponses, webhookIds}; + } + + it("delivers expected payloads from combinations of changes, with retrying and batching", + async function () { + // Create a test document. + const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; + const docId = await userApi.newDoc({name: 'testdoc'}, ws1); + const doc = userApi.getDocAPI(docId); + + // Make a webhook for every combination of event types + const {subscribeResponses, webhookIds} = await createWebhooks({ + docId, tableId: 'Table1', isReadyColumn: "B", + eventTypesSet: [ + ["add"], + ["update"], + ["add", "update"], + ] + }); // Add and update some rows, trigger some events // Values of A where B is true and thus the record is ready are [1, 4, 7, 8] @@ -4141,6 +4147,41 @@ function testDocApi() { ); }); + + [{ + itMsg: "doesn't trigger webhook that has been disabled", + enabled: false, + }, { + itMsg: "does trigger webhook that has been enable", + enabled: true, + }].forEach((ctx) => { + + it(ctx.itMsg, async function () { + // Create a test document. + const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; + const docId = await userApi.newDoc({name: 'testdoc'}, ws1); + const doc = userApi.getDocAPI(docId); + + await createWebhooks({ + docId, tableId: 'Table1', isReadyColumn: "B", eventTypesSet: [ ["add"] ], enabled: ctx.enabled + }); + + await doc.addRows("Table1", { + A: [42], + B: [true] + }); + + const queueRedisCalls = redisCalls.filter(args => args[1] === "webhook-queue-" + docId); + const redisPushIndex = queueRedisCalls.findIndex(args => args[0] === "rpush"); + + if (ctx.enabled) { + assert.isAbove(redisPushIndex, 0, "Should have pushed events to the redis queue"); + } else { + assert.equal(redisPushIndex, -1, "Should not have pushed any events to the redis queue"); + } + }); + + }); }); describe("/webhooks endpoint", function () { @@ -4598,6 +4639,37 @@ function testDocApi() { await unsubscribe1(); }); + it('should not block document load (gh issue #799)', async function () { + // Create a test document. + const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; + const docId = await userApi.newDoc({name: 'testdoc5'}, ws1); + const doc = userApi.getDocAPI(docId); + // Before #799, formula of this type would block document load because of a deadlock + // and make this test fail. + const formulaEvaluatedAtDocLoad = 'NOW()'; + + await axios.post(`${serverUrl}/api/docs/${docId}/apply`, [ + ['ModifyColumn', 'Table1', 'C', {isFormula: true, formula: formulaEvaluatedAtDocLoad}], + ], chimpy); + + const unsubscribeWebhook1 = await autoSubscribe('probe', docId); + + // Create a first row. + await doc.addRows("Table1", { + A: [1], + }); + + await doc.forceReload(); + + // Create a second row after document reload. + // This should not timeout. + await doc.addRows("Table1", { + A: [2], + }); + + await unsubscribeWebhook1(); + }); + it("should monitor failures", async () => { const webhook3 = await subscribe('probe', docId); const webhook4 = await subscribe('probe', docId);