A card no longer offers the attachment "+" when uploads are blocked for the whole site.

#6870 asks for a site-wide switch that turns attachments off, because the
per-board allowsAttachments flag has to be set on every board. That switch is
there already: Admin Panel / Attachments / Transfer limits, "Attachments upload"
and "API upload" set to Blocked, and the server refuses on every path - the
upload pipeline, api.attachment.upload and .copy, and the /api/attachment
upload, upload-background and copy routes. What was missing is that the card
still showed the "+" button and accepted a drop for an upload that would be
refused.

attachmentUploadPolicy tells a signed-in client the one boolean (nothing about
the limits), asked at most once a minute, and the "+" and the drop/paste upload
follow it. No second setting is added, so there is nothing that could disagree
with what the server enforces. tests/attachmentUploadPolicy.test.cjs pins the
client, the method, and the server enforcement it relies on, and that no
"disableAttachments" setting exists.

Thanks to Raymond-Voogt and xet7 !

Refs #6870, not closed: the reporter asked for a single Features checkbox, and the
two Blocked limits are two settings.

client/components/cards/attachments.jade

@@ -87,7 +87,7 @@ template(name="attachmentGallery")
   .attachment-gallery
 
     //- #6796: a small "+" like every other section's, not a full-width box.
-    if canModifyCard
+    if canAddAttachment
       a.card-label.add-label.add-attachment.js-add-attachment(title="{{_ 'add-attachment'}}" aria-label="{{_ 'add-attachment'}}")
         i.fa.fa-plus
 

client/components/cards/attachments.js

@@ -16,6 +16,7 @@ import { attachmentMigrationManager } from '/client/lib/attachmentMigrationManag
 import Attachments from '/models/attachments';
 import { Utils } from '/client/lib/utils';
 import { buildAttachmentUploadConfig } from '/client/lib/attachmentUploadConfig';
+import { attachmentUploadsBlocked } from '/client/lib/attachmentUploadPolicy';
 const { cleanFileName } = require('/imports/lib/fileNameDisplay');
 const { attachmentTileCompact } = require('/models/lib/attachmentTile.cjs');
 import { formatDateTime } from '/imports/lib/dateUtils';
@@ -467,6 +468,10 @@ Template.attachmentGallery.onCreated(function () {
 });
 
 Template.attachmentGallery.helpers({
+  // #6870: the "+" button - a card the user may edit, unless uploads are blocked site-wide
+  canAddAttachment() {
+    return Utils.canModifyCard() && !attachmentUploadsBlocked();
+  },
   // #6796: the small file button instead of the preview tile
   // (models/lib/attachmentTile.cjs).
   attachmentTileCompact(file) {
@@ -672,6 +677,9 @@ export async function handleFileUpload(card, files) {
     return [];
   }
 
+  // #6870: the administrator blocked uploads for the whole site; the server refuses them too
+  if (attachmentUploadsBlocked()) return [];
+
   // Check if board allows attachments
   const board = card.board();
   if (!board || !board.allowsAttachments) {

client/lib/attachmentUploadPolicy.js

@@ -0,0 +1,23 @@
+import { Meteor } from 'meteor/meteor';
+import { ReactiveVar } from 'meteor/reactive-var';
+
+// #6870: whether the administrator blocked attachment uploads for the whole site
+// (models/lib/attachmentUploadPolicy.js). Asked of the server once a minute at most; until it
+// answers - or if it cannot - uploads are shown as before, because the server refuses them either
+// way and a wrong guess here only costs a button.
+const blocked = new ReactiveVar(false);
+let askedAt = 0;
+
+export function refreshUploadPolicy() {
+  if (Date.now() - askedAt < 60 * 1000) return;
+  askedAt = Date.now();
+  Meteor.callAsync('attachmentUploadPolicy')
+    .then(policy => blocked.set(!!(policy && policy.uploadsBlocked === true)))
+    .catch(() => { askedAt = 0; });
+}
+
+// reactive
+export function attachmentUploadsBlocked() {
+  refreshUploadPolicy();
+  return blocked.get();
+}

models/lib/attachmentUploadPolicy.js

@@ -0,0 +1,25 @@
+'use strict';
+
+// Whether this installation lets people upload attachments (#6870).
+//
+// The switch already exists: Admin Panel / Attachments / Transfer limits has "Attachments upload"
+// and "API upload" set to Blocked (limitSettings.attachmentsUploadBlocked / apiUploadBlocked).
+// The server refuses an upload on every path when it is on - the upload pipeline
+// (models/attachments.server.js) and the API methods and routes (server/attachmentApi.js,
+// server/routes/attachmentApi.js). What was missing is that the card still offered the "+" button
+// and a drop zone for an upload that would be refused. This is the one decision the server
+// method that tells the client and the tests share, so there is no second setting that could
+// disagree with the one the server enforces.
+//
+// Only the boolean leaves the server: the rest of the limits (sizes, storage paths) are for
+// administrators.
+
+function uploadPolicy(limitSettings) {
+  const limits = limitSettings && typeof limitSettings === 'object' ? limitSettings : {};
+  return {
+    // anything but a real `true` keeps uploads as they were
+    uploadsBlocked: limits.attachmentsUploadBlocked === true,
+  };
+}
+
+module.exports = { uploadPolicy };

server/methods/attachmentUploadPolicy.js

@@ -0,0 +1,21 @@
+import { Meteor } from 'meteor/meteor';
+import AttachmentStorageSettings from '/models/attachmentStorageSettings';
+const { uploadPolicy } = require('/models/lib/attachmentUploadPolicy');
+
+// #6870: tells a signed-in client whether uploads are blocked site-wide, so a card does not offer a
+// "+" button and a drop zone for an upload the server will refuse. The refusal itself is enforced
+// on the server whatever the client does (models/attachments.server.js, server/attachmentApi.js);
+// this only decides what is shown. Only the boolean is returned.
+Meteor.methods({
+  async attachmentUploadPolicy() {
+    if (!this.userId) throw new Meteor.Error('error-notAuthorized');
+    let limitSettings;
+    try {
+      limitSettings = (await AttachmentStorageSettings.findOneAsync({}))?.limitSettings;
+    } catch (error) {
+      // an unreadable setting leaves uploads as they were, as the server's own check does
+      limitSettings = undefined;
+    }
+    return uploadPolicy(limitSettings);
+  },
+});

tests/attachmentUploadPolicy.test.cjs

@@ -0,0 +1,68 @@
+'use strict';
+
+// #6870: attachments off for the whole installation. The switch exists already - Admin Panel /
+// Attachments / Transfer limits, "Attachments upload" and "API upload" set to Blocked - and the
+// server refuses on every path. What the card lacked was to STOP OFFERING the upload. This pins
+// the client side, and the server enforcement it relies on, so the two cannot drift apart.
+//
+// Run: node tests/attachmentUploadPolicy.test.cjs
+
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const { test } = require('node:test');
+const { uploadPolicy } = require('../models/lib/attachmentUploadPolicy');
+
+const read = rel => fs.readFileSync(path.join(__dirname, '..', rel), 'utf8');
+
+test('only a real true blocks uploads', () => {
+  assert.deepEqual(uploadPolicy({ attachmentsUploadBlocked: true }), { uploadsBlocked: true });
+  for (const v of [undefined, null, {}, [], 'x', { attachmentsUploadBlocked: false },
+    { attachmentsUploadBlocked: 'true' }, { attachmentsUploadBlocked: 1 }, { apiUploadBlocked: true }]) {
+    assert.deepEqual(uploadPolicy(v), { uploadsBlocked: false }, JSON.stringify(v));
+  }
+});
+
+test('negative: the answer carries only the boolean, nothing about limits or storage', () => {
+  const answer = uploadPolicy({ attachmentsUploadBlocked: true, attachmentsUploadMaxBytes: 5, apiUploadMaxBytes: 7, secret: 'x' });
+  assert.deepEqual(Object.keys(answer), ['uploadsBlocked']);
+});
+
+test('the method answers signed-in users only, and falls back to "not blocked" on a read error', () => {
+  const src = read('server/methods/attachmentUploadPolicy.js');
+  assert.match(src, /async attachmentUploadPolicy\(\) \{\s*if \(!this\.userId\) throw new Meteor\.Error\('error-notAuthorized'\);/);
+  assert.match(src, /catch \(error\) \{[^}]*limitSettings = undefined;/);
+});
+
+test('the card offers the "+" only when uploads are not blocked', () => {
+  const jade = read('client/components/cards/attachments.jade');
+  assert.match(jade, /if canAddAttachment\s*\n\s*a\.card-label\.add-label\.add-attachment\.js-add-attachment/);
+  assert.doesNotMatch(jade, /if canModifyCard\s*\n\s*a\.card-label\.add-label\.add-attachment/, 'the old condition is gone');
+  const js = read('client/components/cards/attachments.js');
+  assert.match(js, /canAddAttachment\(\) \{\s*return Utils\.canModifyCard\(\) && !attachmentUploadsBlocked\(\);/);
+  // and a drop or paste does nothing either
+  assert.match(js, /if \(attachmentUploadsBlocked\(\)\) return \[\];\s*\n\s*\/\/ Check if board allows attachments/);
+});
+
+test('the client asks once a minute at most and shows uploads until the server answers', () => {
+  const src = read('client/lib/attachmentUploadPolicy.js');
+  assert.match(src, /new ReactiveVar\(false\)/);
+  assert.match(src, /Date\.now\(\) - askedAt < 60 \* 1000/);
+  assert.match(src, /catch\(\(\) => \{ askedAt = 0; \}\)/);
+});
+
+test('the server refuses on every upload path whatever the client shows', () => {
+  const pipeline = read('models/attachments.server.js');
+  assert.match(pipeline, /uploadsBlocked = settings\?\.limitSettings\?\.attachmentsUploadBlocked === true/);
+  const methods = read('server/attachmentApi.js');
+  const routes = read('server/routes/attachmentApi.js');
+  // api.attachment.upload and api.attachment.copy, the HTTP upload, upload-background and copy
+  assert.equal((methods.match(/if \(apiUploadBlocked\)/g) || []).length >= 2, true, 'upload and copy methods');
+  assert.equal((routes.match(/if \(apiUploadBlocked\)/g) || []).length >= 3, true, 'upload, background and copy routes');
+});
+
+test('negative: there is no second "disable attachments" setting to disagree with the server', () => {
+  for (const f of ['models/settings.js', 'server/publications/settings.js', 'client/components/settings/settingBody.jade']) {
+    assert.doesNotMatch(read(f), /disableAttachments/, f);
+  }
+});