Merge pull request #10602 from growilabs/fix/175827-omit-file-upload-restriction-feature-for-non-image-files

fix: Omit file upload restriction feature for non image files

apps/app/public/static/locales/en_US/admin.json

@@ -357,8 +357,6 @@
     "default_language": "Default language for new users",
     "default_mail_visibility": "Disclose e-mail for new users",
     "file_uploading": "File uploading",
-    "enable_files_except_image": "Enabling this option will allow upload of any file type. Without this option, only image file upload is supported.",
-    "attach_enable": "You can attach files other than image files if you enable this option.",
     "page_bulk_export_settings": "Page Bulk Export Settings",
     "enable_page_bulk_export": "Enable bulk export",
     "page_bulk_export_explanation": "Enables a feature that allows all users to export a page and all it's child pages at once from the menu. Exported data will be automatically deleted after the storage period has passed.",

apps/app/public/static/locales/fr_FR/admin.json

@@ -357,8 +357,6 @@
     "default_language": "Langue par défaut",
     "default_mail_visibility": "Mode d'affichage de l'adresse courriel",
     "file_uploading": "Téléversement de fichiers",
-    "enable_files_except_image": "Autoriser tout les types de fichiers",
-    "attach_enable": "Autorise le téléversement de tout les types de fichiers.",
     "page_bulk_export_settings": "Paramètres d'exportation de pages par lots",
     "enable_page_bulk_export": "Activer l'exportation groupée",
     "page_bulk_export_explanation": "Active une fonctionnalité qui permet à tous les utilisateurs d'exporter simultanément toutes les pages sélectionnées dans le menu des pages et leurs pages subordonnées. Les données exportées seront automatiquement supprimées une fois la période de conservation écoulée.",

apps/app/public/static/locales/ja_JP/admin.json

@@ -366,8 +366,6 @@
     "default_language": "新規ユーザーのデフォルト設定言語",
     "default_mail_visibility": "新規ユーザーの初期メール公開設定",
     "file_uploading": "ファイルアップロード",
-    "enable_files_except_image": "画像以外のファイルアップロードを許可",
-    "attach_enable": "許可をしている場合、画像以外のファイルをページに添付可能になります。",
     "page_bulk_export_settings": "ページ一括エクスポート設定",
     "enable_page_bulk_export": "一括エクスポートを有効にする",
     "page_bulk_export_explanation": "すべてのユーザーが、ページメニューから選択したページとその配下ページをまとめてエクスポートできる機能を有効化します。エクスポートされたデータは保存期間経過後に自動的に削除されます。",

apps/app/public/static/locales/ko_KR/admin.json

@@ -357,8 +357,6 @@
     "default_language": "새 사용자를 위한 기본 언어",
     "default_mail_visibility": "새 사용자를 위한 이메일 공개",
     "file_uploading": "파일 업로드",
-    "enable_files_except_image": "이 옵션을 활성화하면 모든 파일 형식을 업로드할 수 있습니다. 이 옵션이 없으면 이미지 파일 업로드만 지원됩니다.",
-    "attach_enable": "이 옵션을 활성화하면 이미지 파일 외의 파일을 첨부할 수 있습니다.",
     "page_bulk_export_settings": "페이지 대량 내보내기 설정",
     "enable_page_bulk_export": "대량 내보내기 활성화",
     "page_bulk_export_explanation": "모든 사용자가 메뉴에서 한 번에 페이지와 모든 하위 페이지를 내보낼 수 있는 기능을 활성화합니다. 내보낸 데이터는 저장 기간이 지나면 자동으로 삭제됩니다.",

apps/app/public/static/locales/zh_CN/admin.json

@@ -366,8 +366,6 @@
     "default_language": "新用户的默认语言",
     "default_mail_visibility": "新用户的默认电子邮件可见性",
     "file_uploading": "文件上传",
-    "enable_files_except_image": "启用此选项将允许上传任何文件类型。如果没有此选项，则仅支持图像文件上载。",
-    "attach_enable": "如果启用此选项，则可以附加图像文件以外的文件。",
     "page_bulk_export_settings": "页面批量导出设置",
     "enable_page_bulk_export": "启用批量导出",
     "page_bulk_export_explanation": "启用一项功能，允许所有用户一次性导出从页面菜单中选择的所有页面及其下级页面。保留期限过后，导出的数据将自动删除。",

apps/app/src/client/components/Admin/App/AppSetting.jsx

@@ -35,14 +35,12 @@ const AppSetting = (props) => {
       globalLang: adminAppContainer.state.globalLang || 'en-US',
       // Convert boolean to string for radio button value
       isEmailPublishedForNewUser: String(adminAppContainer.state.isEmailPublishedForNewUser ?? true),
-      fileUpload: adminAppContainer.state.fileUpload ?? false,
     });
   }, [
     adminAppContainer.state.title,
     adminAppContainer.state.confidential,
     adminAppContainer.state.globalLang,
     adminAppContainer.state.isEmailPublishedForNewUser,
-    adminAppContainer.state.fileUpload,
     reset,
   ]);
 
@@ -57,7 +55,6 @@ const AppSetting = (props) => {
       // Convert string 'true'/'false' to boolean
       const isEmailPublished = data.isEmailPublishedForNewUser === 'true' || data.isEmailPublishedForNewUser === true;
       await adminAppContainer.changeIsEmailPublishedForNewUserShow(isEmailPublished);
-      await adminAppContainer.changeFileUpload(data.fileUpload);
 
       await adminAppContainer.updateAppSettingHandler();
       toastSuccess(t('commons:toaster.update_successed', { target: t('commons:headers.app_settings') }));
@@ -163,34 +160,6 @@ const AppSetting = (props) => {
         </div>
       </div>
 
-      <div className="row mb-2">
-        <label
-          className="text-start text-md-end col-md-3 col-form-label"
-        >
-          {/* {t('admin:app_setting.file_uploading')} */}
-        </label>
-        <div className="col-md-6">
-          <div className="form-check form-check-info">
-            <input
-              type="checkbox"
-              id="cbFileUpload"
-              className="form-check-input"
-              {...register('fileUpload')}
-            />
-            <label
-              className="form-label form-check-label"
-              htmlFor="cbFileUpload"
-            >
-              {t('admin:app_setting.enable_files_except_image')}
-            </label>
-          </div>
-
-          <p className="form-text text-muted">
-            {t('admin:app_setting.attach_enable')}
-          </p>
-        </div>
-      </div>
-
       <AdminUpdateButtonRow type="submit" disabled={adminAppContainer.state.retrieveError != null} />
     </form>
   );

apps/app/src/client/services/AdminAppContainer.js

@@ -21,7 +21,6 @@ export default class AdminAppContainer extends Container {
       confidential: '',
       globalLang: '',
       isEmailPublishedForNewUser: true,
-      fileUpload: '',
 
       isV5Compatible: null,
       siteUrl: '',
@@ -62,7 +61,6 @@ export default class AdminAppContainer extends Container {
       confidential: appSettingsParams.confidential,
       globalLang: appSettingsParams.globalLang,
       isEmailPublishedForNewUser: appSettingsParams.isEmailPublishedForNewUser,
-      fileUpload: appSettingsParams.fileUpload,
       isV5Compatible: appSettingsParams.isV5Compatible,
       siteUrl: appSettingsParams.siteUrl,
       siteUrlUseOnlyEnvVars: appSettingsParams.siteUrlUseOnlyEnvVars,
@@ -110,13 +108,6 @@ export default class AdminAppContainer extends Container {
     this.setState({ isEmailPublishedForNewUser });
   }
 
-  /**
-   * Change fileUpload
-   */
-  changeFileUpload(fileUpload) {
-    this.setState({ fileUpload });
-  }
-
   /**
    * Change site url
    */
@@ -198,7 +189,6 @@ export default class AdminAppContainer extends Container {
       confidential: this.state.confidential,
       globalLang: this.state.globalLang,
       isEmailPublishedForNewUser: this.state.isEmailPublishedForNewUser,
-      fileUpload: this.state.fileUpload,
     });
     const { appSettingParams } = response.data;
     return appSettingParams;

apps/app/src/interfaces/file-uploader.ts

@@ -13,6 +13,7 @@ export type FileUploadType =
 // file upload type strings you can specify in the env variable
 export const FileUploadTypeForEnvVar = {
   ...FileUploadType,
+  none: 'none',
   mongo: 'mongo',
   mongodb: 'mongodb',
   gcp: 'gcp',

apps/app/src/server/routes/apiv3/app-settings/index.ts

@@ -327,7 +327,6 @@ module.exports = (crowi) => {
       body('confidential'),
       body('globalLang').isIn(i18n.locales),
       body('isEmailPublishedForNewUser').isBoolean(),
-      body('fileUpload').isBoolean(),
     ],
     siteUrlSetting: [
       // https://regex101.com/r/5Xef8V/1
@@ -401,7 +400,6 @@ module.exports = (crowi) => {
         isEmailPublishedForNewUser: configManager.getConfig(
           'customize:isEmailPublishedForNewUser',
         ),
-        fileUpload: configManager.getConfig('app:fileUpload'),
         useOnlyEnvVarsForIsBulkExportPagesEnabled: configManager.getConfig(
           'env:useOnlyEnvVars:app:isBulkExportPagesEnabled',
         ),
@@ -564,7 +562,6 @@ module.exports = (crowi) => {
         'app:globalLang': req.body.globalLang,
         'customize:isEmailPublishedForNewUser':
           req.body.isEmailPublishedForNewUser,
-        'app:fileUpload': req.body.fileUpload,
       };
 
       try {
@@ -576,7 +573,6 @@ module.exports = (crowi) => {
           isEmailPublishedForNewUser: configManager.getConfig(
             'customize:isEmailPublishedForNewUser',
           ),
-          fileUpload: configManager.getConfig('app:fileUpload'),
         };
 
         const parameters = {

apps/app/src/server/service/config-manager/config-definition.ts

@@ -45,7 +45,6 @@ export const CONFIG_KEYS = [
   'app:title',
   'app:timezone',
   'app:globalLang',
-  'app:fileUpload',
   'app:fileUploadType',
   'app:plantumlUri',
   'app:drawioUri',
@@ -56,7 +55,6 @@ export const CONFIG_KEYS = [
   'app:maxFileSize',
   'app:fileUploadTimeout',
   'app:fileUploadTotalLimit',
-  'app:fileUploadDisabled',
   'app:elasticsearchVersion',
   'app:elasticsearchUri',
   'app:elasticsearchRequestTimeout',
@@ -388,12 +386,6 @@ export const CONFIG_DEFINITIONS = {
   'app:globalLang': defineConfig<string>({
     defaultValue: 'en_US',
   }),
-  'app:fileUpload': defineConfig<boolean>({
-    defaultValue: false,
-  }),
-  'app:fileUploadDisabled': defineConfig<boolean>({
-    defaultValue: false,
-  }),
   'app:fileUploadType': defineConfig<AttachmentMethodType>({
     envVarName: 'FILE_UPLOAD',
     defaultValue: AttachmentMethodType.aws,

apps/app/src/server/service/config-manager/config-manager.spec.ts

@@ -303,13 +303,11 @@ describe('ConfigManager test', () => {
       const dbConfig: Partial<TestConfigData> = {
         'app:title': { value: undefined }, // db value is explicitly undefined
         'app:siteUrl': { value: undefined }, // another undefined value
-        'app:fileUpload': undefined, // db config entry itself is undefined
         'app:fileUploadType': { value: 'gridfs' }, // db has valid value
       };
       const envConfig: Partial<TestConfigData> = {
         'app:title': { value: 'GROWI' },
         'app:siteUrl': { value: 'https://example.com' },
-        'app:fileUpload': { value: true },
         'app:fileUploadType': { value: 'aws' },
         // Add control flags for env vars
         'env:useOnlyEnvVars:app:siteUrl': { value: false },
@@ -322,7 +320,6 @@ describe('ConfigManager test', () => {
       expect(configManager.getConfig('app:siteUrl')).toBe(
         'https://example.com',
       ); // Should fallback to env when db value is undefined
-      expect(configManager.getConfig('app:fileUpload')).toBe(true); // Should fallback to env when db config is undefined
       expect(configManager.getConfig('app:fileUploadType')).toBe('gridfs'); // Should use db value when valid
     });
   });

apps/app/src/server/service/file-uploader/file-uploader.ts

@@ -79,10 +79,10 @@ export abstract class AbstractFileUploader implements FileUploader {
   }
 
   getIsUploadable() {
-    return (
-      !configManager.getConfig('app:fileUploadDisabled') &&
-      this.isValidUploadSettings()
-    );
+    const isFileUploadDisabled =
+      configManager.getConfig('app:fileUploadType') === 'none';
+
+    return !isFileUploadDisabled && this.isValidUploadSettings();
   }
 
   /**
@@ -117,11 +117,7 @@ export abstract class AbstractFileUploader implements FileUploader {
   abstract isValidUploadSettings(): boolean;
 
   getFileUploadEnabled() {
-    if (!this.getIsUploadable()) {
-      return false;
-    }
-
-    return !!configManager.getConfig('app:fileUpload');
+    return this.getIsUploadable();
   }
 
   abstract listFiles();

apps/app/src/server/service/file-uploader/none.ts

@@ -0,0 +1,129 @@
+import type { Response } from 'express';
+import type { Readable } from 'stream';
+
+import type { ICheckLimitResult } from '~/interfaces/attachment';
+import type Crowi from '~/server/crowi';
+import type { RespondOptions } from '~/server/interfaces/attachment';
+import type { IAttachmentDocument } from '~/server/models/attachment';
+
+import {
+  AbstractFileUploader,
+  type SaveFileParam,
+  type TemporaryUrl,
+} from './file-uploader';
+
+/**
+ * NoneFileUploader is a placeholder uploader when file upload is disabled.
+ * All write operations are disabled, but read operations return empty results.
+ */
+class NoneFileUploader extends AbstractFileUploader {
+  /**
+   * @inheritdoc
+   */
+  override getIsUploadable(): boolean {
+    return false;
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override isValidUploadSettings(): boolean {
+    return false;
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override async isWritable(): Promise<boolean> {
+    return false;
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override getIsReadable(): boolean {
+    return false;
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override listFiles(): [] {
+    return [];
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override saveFile(_param: SaveFileParam): Promise<void> {
+    throw new Error('File upload is disabled');
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override deleteFile(_attachment: IAttachmentDocument): void {
+    throw new Error('File upload is disabled');
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override deleteFiles(_attachments: IAttachmentDocument[]): void {
+    throw new Error('File upload is disabled');
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override checkLimit(_uploadFileSize: number): Promise<ICheckLimitResult> {
+    return Promise.resolve({
+      isUploadable: false,
+      errorMessage: 'File upload is disabled',
+    });
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override async uploadAttachment(
+    _readable: Readable,
+    _attachment: IAttachmentDocument,
+  ): Promise<void> {
+    throw new Error('File upload is disabled');
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override respond(
+    res: Response,
+    _attachment: IAttachmentDocument,
+    _opts?: RespondOptions,
+  ): void {
+    res.status(404).send('File upload is disabled');
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override findDeliveryFile(
+    _attachment: IAttachmentDocument,
+  ): Promise<NodeJS.ReadableStream> {
+    throw new Error('File upload is disabled');
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override generateTemporaryUrl(
+    _attachment: IAttachmentDocument,
+    _opts?: RespondOptions,
+  ): Promise<TemporaryUrl> {
+    throw new Error('File upload is disabled');
+  }
+}
+
+module.exports = (crowi: Crowi) => {
+  return new NoneFileUploader(crowi);
+};

apps/app/src/server/service/g2g-transfer.ts

@@ -618,9 +618,8 @@ export class G2GTransferReceiverService implements Receiver {
     const { fileUploadService } = this.crowi;
     const version = getGrowiVersion();
     const userUpperLimit = configManager.getConfig('security:userUpperLimit');
-    const fileUploadDisabled = configManager.getConfig(
-      'app:fileUploadDisabled',
-    );
+    const fileUploadDisabled =
+      configManager.getConfig('app:fileUploadType') === 'none';
     const fileUploadTotalLimit = fileUploadService.getFileUploadTotalLimit();
     const isWritable = await fileUploadService.isWritable();
 

apps/app/src/server/service/installer.ts

@@ -127,7 +127,6 @@ export class InstallerService {
     await configManager.updateConfigs(
       {
         'app:installed': true,
-        'app:fileUpload': true,
         'app:isV5Compatible': true,
         'app:globalLang': globalLang,
       },