Merge branch 'broadcast' into calendar

CHANGELOG.md

@@ -1,3 +1,23 @@
+# WIP
+
+* OnlyOffice
+  * inform OnlyOffice of userlist changes
+  * rename doc and slide editors
+  * handle different lock formats for docs and slides
+    * relative to sheets
+  * handle some cursor logic outside of sheets
+  * handle locks when integrating remote checkpoints in strict mode
+  * OnlyOffice renamed buttons in slides and docs and we need to hardcode CSS that hides them by their randomly generated IDs
+  * support CryptPad cursor colors in OnlyOffice by adding opacity value
+  * use the appropriate APIs to detect if the document is modified
+  * display users cursor colors in the toolbar next to their name
+  * handle errors when migrating in embed mode
+  * change the method we use to lock the whole sheet since OnlyOffice changed their internal API's behaviour
+* bad channel IDs stored in your drive or accessed via bad links (corrupted somehow)
+  * don't try to join invalid channels
+  * don't try to get their metadata
+* prompt premium users to cancel their subscriptions before deleting their accounts
+
 # 4.3.1
 
 This minor release addresses some bugs discovered after deploying and tagging 4.3.0

lib/commands/metadata.js

@@ -12,6 +12,17 @@ Data.getMetadataRaw = function (Env, channel /* channelName */, _cb) {
     if (channel.length !== HK.STANDARD_CHANNEL_LENGTH &&
         channel.length !== HK.ADMIN_CHANNEL_LENGTH) { return cb("INVALID_CHAN_LENGTH"); }
 
+    // return synthetic metadata for admin broadcast channels as a safety net
+    // in case anybody manages to write metadata
+    /*
+    if (channel.length === HK.ADMIN_CHANNEL_LENGTH) { // XXX
+        return void cb(void 0, {
+            channel: channel,
+            creation: +new Date(),
+            owners: Env.admins,
+        });
+    } */
+
     var cached = Env.metadata_cache[channel];
     if (HK.isMetadataMessage(cached)) {
         Env.checkCache(channel);
@@ -141,7 +152,7 @@ Data.setMetadata = function (Env, safeKey, data, cb, Server) {
                 const metadata_cache = Env.metadata_cache;
 
                 // update the cached metadata
-                metadata_cache[channel] = metadata;
+                metadata_cache[channel] = metadata; // XXX guard against malicious takeover of the broadcast channel
 
                 // it's easy to check if the channel is restricted
                 const isRestricted = metadata.restricted;

lib/decrees.js

@@ -27,6 +27,7 @@ DISABLE_INTEGRATED_EVICTION
 // BROADCAST
 SET_LAST_BROADCAST_HASH
 SET_SURVEY_URL
+SET_MAINTENANCE
 
 NOT IMPLEMENTED:
 
@@ -129,10 +130,10 @@ var args_isString = function (args) {
     return Array.isArray(args) && typeof(args[0]) === "string";
 };
 var args_isMaintenance = function (args) {
-    return Array.isArray(args) && args[0] && args[0].end && args[0].start;
+    return Array.isArray(args) && args[0] && args[0].end && args[0].start; // XXX we could validate that these are numbers && !isNaN
 };
 
-var makeBroadcastSetter = function (attr) {
+var makeBroadcastSetter = function (attr) { // XXX could pass extra validation here?
     return function (Env, args) {
         if (!args_isString(args) && !args_isMaintenance(args)) {
             throw new Error('INVALID_ARGS');
@@ -149,7 +150,7 @@ var makeBroadcastSetter = function (attr) {
 commands.SET_LAST_BROADCAST_HASH = makeBroadcastSetter('lastBroadcastHash');
 
 // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['SET_SURVEY_URL', [url]]], console.log)
-commands.SET_SURVEY_URL = makeBroadcastSetter('surveyURL');
+commands.SET_SURVEY_URL = makeBroadcastSetter('surveyURL'); // XXX anticipate language-specific surveys
 
 // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['SET_MAINTENANCE', [{start: +Date, end: +Date}]]], console.log)
 // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['SET_MAINTENANCE', [""]]], console.log)

server.js

@@ -113,8 +113,7 @@ var setHeaders = (function () {
 
             // Don't set CSP headers on /api/config because they aren't necessary and they cause problems
             // when duplicated by NGINX in production environments
-            // XXX /api/broadcast too?
-            if (/^\/api\/config/.test(req.url)) { return; }
+            if (/^\/api\/(broadcast|config)/.test(req.url)) { return; }
             // targeted CSP, generic policies, maybe custom headers
             const h = [
                     /^\/common\/onlyoffice\/.*\/index\.html.*/,
@@ -273,7 +272,7 @@ var serveConfig = (function () {
     };
 }());
 
-var serveBroadcast = (function () {
+var serveBroadcast = (function () { // XXX deduplicate
     var cacheString = function () {
         return (Env.FRESH_KEY? '-' + Env.FRESH_KEY: '') + (Env.DEV_MODE? '-' + (+new Date()): '');
     };
@@ -284,7 +283,7 @@ var serveBroadcast = (function () {
             maintenance = undefined;
         }
         return [
-            'define(function(){',
+            'define(function(){', // XXX maybe this could just be JSON
             'return ' + JSON.stringify({
                 lastBroadcastHash: Env.lastBroadcastHash,
                 surveyURL: Env.surveyURL,

www/admin/app-admin.less

@@ -14,6 +14,10 @@
 
     display: flex;
     flex-flow: column;
+    a {
+        color: @cryptpad_color_link;
+        text-decoration: underline;
+    }
 
     .cp-admin-setlimit-form, .cp-admin-broadcast-form {
         label {

www/admin/inner.js

@@ -970,6 +970,7 @@ define([
     var getApi = function (cb) {
         return function () {
             require(['/api/broadcast?'+ (+new Date())], function (Broadcast) {
+                // XXX require.s.contexts._ can be used to erase old loaded objects
                 cb(Broadcast);
             });
         };
@@ -1060,7 +1061,7 @@ define([
         var form = h('div.cp-admin-broadcast-form');
         var $form = $(form).appendTo($div);
 
-        var refresh = getApi(function (Broadcast) {
+        var refresh = getApi(function (/* Broadcast */) { // XXX unused argument
             var button = h('button.btn.btn-primary', Messages.admin_broadcastButton);
             var $button = $(button);
             var removeButton = h('button.btn.btn-danger', Messages.admin_broadcastCancel);
@@ -1219,7 +1220,7 @@ define([
                 }
                 if (error) {
                     console.error('One of the selected languages has no data');
-                    return false;
+                    return false; // XXX better error handling?
                 }
                 return {
                     defaultLanguage: defaultLanguage,
@@ -1229,8 +1230,8 @@ define([
 
             var send = function (data) {
                 $button.prop('disabled', 'disabled');
-                data.time = +new Date();
-                common.mailbox.sendTo('BROADCAST_CUSTOM', data, {}, function (err, data) {
+                data.time = +new Date(); // XXX not used anymore?
+                common.mailbox.sendTo('BROADCAST_CUSTOM', data, {}, function (err /*, data */) { // XXX unused argument
                     if (err) {
                         $button.prop('disabled', '');
                         console.error(err);
@@ -1249,13 +1250,13 @@ define([
                 send(data);
             });
 
-            UI.confirmButton(removeButton, {
+            UI.confirmButton(removeButton, { // XXX table jank
                 classes: 'btn-danger',
             }, function () {
                 if (!activeUid) { return; }
                 common.mailbox.sendTo('BROADCAST_DELETE', {
                     uid: activeUid
-                }, {}, function (err, data) {
+                }, {}, function (err /* , data */) { // XXX unused argument
                     if (err) { return UI.warn(Messages.error); }
                     UI.log(Messages.saved);
                     refresh();
@@ -1311,7 +1312,8 @@ define([
             var end = h('input');
             var $start = $(start);
             var $end = $(end);
-            var endPickr = Flatpickr(end, {
+            // XXX new Date().toLocaleString('fr-fr', {month: 'long'}).replace(/./, c => c.toUpperCase())
+            var endPickr = Flatpickr(end, { // XXX translations?
                 enableTime: true,
                 minDate: new Date()
             });
@@ -1349,7 +1351,7 @@ define([
                         return;
                     }
                     // Maintenance applied, send notification
-                    common.mailbox.sendTo('BROADCAST_MAINTENANCE', {}, {}, function (err, data) {
+                    common.mailbox.sendTo('BROADCAST_MAINTENANCE', {}, {}, function (/* err, data */) { // XXX unused arguments
                         refresh();
                         checkLastBroadcastHash();
                     });
@@ -1411,7 +1413,7 @@ define([
                     common.openUnsafeURL(Broadcast.surveyURL);
                 });
                 active = h('div.cp-broadcast-active', [
-                    h('p', a),
+                    h('p', a), // XXX spacing around this element is really cramped
                     removeButton
                 ]);
             }
@@ -1445,7 +1447,7 @@ define([
                     // Maintenance applied, send notification
                     common.mailbox.sendTo('BROADCAST_SURVEY', {
                         url: data
-                    }, {}, function (err, data) {
+                    }, {}, function (/* err, data */) { // XXX unused arguments
                         refresh();
                         checkLastBroadcastHash();
                     });

www/common/application_config_internal.js

@@ -162,7 +162,7 @@ define(function() {
     // making it much faster to open new tabs.
     config.disableWorkers = false;
 
-    //config.surveyURL = "";
+    //config.surveyURL = ""; // XXX remove this?
 
     // Teams are always loaded during the initial loading screen (for the first tab only if
     // SharedWorkers are available). Allowing users to be members of multiple teams can

www/common/onlyoffice/inner.js

@@ -1409,7 +1409,7 @@ define([
                                 // Migration required but read-only: continue...
                                 if (readOnly) {
                                     setEditable(true);
-                                    getEditor().setViewModeDisconnect();
+                                    try { getEditor().asc_setRestriction(true); } catch (e) {}
                                 } else {
                                     // No changes after the cp: migrate now
                                     onMigrateRdy.fire();
@@ -1434,12 +1434,9 @@ define([
                             return;
                         }
 
-                        if (lock) {
-                            getEditor().setViewModeDisconnect();
-                        } else if (readOnly) {
-                            try {
-                                getEditor().asc_setRestriction(true);
-                            } catch (e) {}
+                        if (lock || readOnly) {
+                            try { getEditor().asc_setRestriction(true); } catch (e) {}
+                            //getEditor().setViewModeDisconnect(); // can't be used anymore, display an OO error popup
                         } else {
                             setEditable(true);
                             deleteOfflineLocks();
@@ -1459,7 +1456,6 @@ define([
                             }
                         }
 
-
                         if (isLockedModal.modal && force) {
                             isLockedModal.modal.closeModal();
                             delete isLockedModal.modal;
@@ -1469,7 +1465,8 @@ define([
                         }
 
                         if (APP.template) {
-                            getEditor().setViewModeDisconnect();
+                            try { getEditor().asc_setRestriction(true); } catch (e) {}
+                            //getEditor().setViewModeDisconnect();
                             UI.removeLoadingScreen();
                             makeCheckpoint(true);
                             return;
@@ -1483,7 +1480,7 @@ define([
                             } catch (e) {}
                         }
 
-                        if (APP.migrate && !readOnly) {
+                        if (lock && !readOnly) {
                             onMigrateRdy.fire();
                         }
 
@@ -2065,7 +2062,7 @@ define([
                 UI.removeModals();
                 UI.confirm(Messages.oo_uploaded, function (yes) {
                     try {
-                        getEditor().setViewModeDisconnect();
+                        getEditor().asc_setRestriction(true);
                     } catch (e) {}
                     if (!yes) { return; }
                     common.gotoURL();
@@ -2232,7 +2229,9 @@ define([
             APP.history = true;
             APP.template = true;
             var editor = getEditor();
-            if (editor) { editor.setViewModeDisconnect(); }
+            if (editor) {
+                try { getEditor().asc_setRestriction(true); } catch (e) {}
+            }
             var content = parsed.content;
 
             // Get checkpoint
@@ -2372,7 +2371,7 @@ define([
                 var setHistoryMode = function (bool) {
                     if (bool) {
                         APP.history = true;
-                        getEditor().setViewModeDisconnect();
+                        try { getEditor().asc_setRestriction(true); } catch (e) {}
                         return;
                     }
                     // Cancel button: redraw from lastCp
@@ -2579,7 +2578,11 @@ define([
                     APP.onLocal();
                 } else {
                     msg = h('div.alert.alert-warning.cp-burn-after-reading', Messages.oo_sheetMigration_anonymousEditor);
-                    $(APP.helpMenu.menu).after(msg);
+                    if (APP.helpMenu) {
+                        $(APP.helpMenu.menu).after(msg);
+                    } else {
+                        $('#cp-app-oo-editor').prepend(msg);
+                    }
                     readOnly = true;
                 }
             } else if (content && content.version <= 3) { // V2 or V3
@@ -2591,7 +2594,11 @@ define([
                     APP.onLocal();
                 } else {
                     msg = h('div.alert.alert-warning.cp-burn-after-reading', Messages.oo_sheetMigration_anonymousEditor);
-                    $(APP.helpMenu.menu).after(msg);
+                    if (APP.helpMenu) {
+                        $(APP.helpMenu.menu).after(msg);
+                    } else {
+                        $('#cp-app-oo-editor').prepend(msg);
+                    }
                     readOnly = true;
                 }
             }

www/common/outer/async-store.js

@@ -3178,7 +3178,7 @@ define([
             });
         };
 
-        Store.newVersionReload = function () {
+        Store.newVersionReload = function () { // XXX not used anymore?
             broadcast([], "NETWORK_RECONNECT");
         };
         Store.disconnect = function () {

www/common/outer/mailbox-handlers.js

@@ -687,7 +687,7 @@ define([
 
 
     // Broadcast
-    var broadcasts = {};
+    //var broadcasts = {}; // XXX defined but never used ?
     handlers['BROADCAST_MAINTENANCE'] = function (ctx, box, data, cb) {
         var msg = data.msg;
         var uid = msg.uid;
@@ -708,7 +708,7 @@ define([
         var dismiss = !content.url;
         cb(dismiss, old);
     };
-    var activeCustom
+    var activeCustom;
     handlers['BROADCAST_CUSTOM'] = function (ctx, box, data, cb) {
         var msg = data.msg;
         var uid = msg.uid;

www/common/outer/mailbox.js

@@ -341,7 +341,7 @@ proxy.mailboxes = {
                 };
                 var notify = box.ready;
                 Handlers.add(ctx, box, message, function (dismissed, toDismiss, setAsLKH) {
-                    if (setAsLKH) {
+                    if (setAsLKH) { // XXX confirm whether this if is used?
                         // Update LKH
                         box.data.lastKnownHash = hash;
                         box.data.viewed = [];

www/common/outer/team.js

@@ -1382,15 +1382,15 @@ define([
 
         // Viewer to editor
         if (user.role === "VIEWER" && data.data.role !== "VIEWER") {
-            changeEditRights(ctx, teamId, user, true, function (err) {
-                return void cb({error: err});
+            changeEditRights(ctx, teamId, user, true, function (obj) {
+                return void cb(obj);
             });
         }
 
         // Editor to viewer
         if (user.role !== "VIEWER" && data.data.role === "VIEWER") {
-            changeEditRights(ctx, teamId, user, false, function (err) {
-                return void cb({error: err});
+            changeEditRights(ctx, teamId, user, false, function (obj) {
+                return void cb(obj);
             });
         }
 

www/common/toolbar.js

@@ -1032,7 +1032,7 @@ MessengerUI, Messages) {
     Messages.broadcast_maintenance = "A maintenance is planned between <b>{0}</b> and <b>{1}</b>"; // XXX
     var createMaintenance = function (toolbar, config) {
         var $notif = toolbar.$top.find('.'+MAINTENANCE_CLS);
-        var button = h('button.cp-maintenance-wrench.fa.fa-wrench');
+        var button = h('button.cp-maintenance-wrench.fa.fa-wrench'); // XXX might need some color contrast
         $notif.append(button);
 
 

www/support/ui.js

@@ -445,7 +445,7 @@ define([
         };
         ctx.FM = common.createFileManager(fmConfig);
 
-        ui.send = function (id, type, data, dest) {
+        ui.send = function (id, type, data, dest) { // XXX confirm that this is actually used
             return send(ctx, id, type, data, dest);
         };
         ui.sendForm = function (id, form, dest) {