Просмотр исходного кода

Fix problem where headers were not being set

arvid-e 9 месяцев назад
Родитель
Сommit
b3483bef59

+ 102 - 6
apps/app/src/server/routes/apiv3/content-disposition-settings.js

@@ -36,6 +36,42 @@ module.exports = (crowi) => {
   const activityEvent = crowi.event('activity');
 
 
+  /**
+   * @swagger
+  * /markdown-setting:
+  * get:
+  *  tags: [Markdown Settings]
+  *  summary: Get content disposition settings for configurable MIME types
+  *    description: Retrieve the current `inline` or `attachment` disposition setting for each configurable MIME type.
+  *  security:
+  *    - cookieAuth: []
+  *    - adminRequired: []
+  *  responses:
+  *    200:
+  *      description: Successfully retrieved content disposition settings.
+  *      content:
+  *        application/json:
+  *          schema:
+  *            type: object
+  *            properties:
+  *              contentDispositionSettings:
+  *                type: object
+  *                description: An object mapping configurable MIME types to their current inline disposition status.
+  *                additionalProperties:
+  *                  type: boolean
+  *                  description: true if inline, false if attachment.
+  *                example:
+  *                  image/png: true
+  *                  application/pdf: false
+  *                  text/plain: true
+  *    401:
+  *      $ref: '#/components/responses/401'
+  *    403:
+  *    $ref: '#/components/responses/403'
+  *  500:
+  *     $ref: '#/components/responses/500'
+  *
+   */
   router.get('/', loginRequiredStrictly, adminRequired, async(req, res) => {
     const promises = CONFIGURABLE_MIME_TYPES_FOR_DISPOSITION.map(async(mimeType) => {
       const configKey = `attachments:contentDisposition:${mimeType}:inline`;
@@ -61,15 +97,77 @@ module.exports = (crowi) => {
   });
 
 
+  /**
+   * @swagger
+   *
+   * /markdown-setting/{mimeType}:
+   * put:
+   * tags: [Markdown Settings]
+   * summary: Update content disposition setting for a specific MIME type
+   * description: Set the `inline` or `attachment` disposition for a given configurable MIME type.
+   * security:
+   *   - cookieAuth: []
+   *   - adminRequired: []
+   * parameters:
+   *   - in: path
+   *     name: mimeType
+   *     schema:
+   *       type: string
+   *       pattern: "^.+\\/.+$"
+   *       required: true
+   *       description: The MIME type to update (e.g., 'image/png', 'application/pdf'). Must be one of the configurable types.
+   *     examples:
+   *       imagePng:
+   *         value: image/png
+   *       applicationPdf:
+   *         value: application/pdf
+   * requestBody:
+   *   content:
+   *     application/json:
+   *       schema:
+   *         type: object
+   *         required:
+   *           - isInline
+   *         properties:
+   *           isInline:
+   *             type: boolean
+   *             description: Set to `true` for inline disposition, `false` for attachment disposition.
+   *             example: true
+   * responses:
+   *   200:
+   *     description: Successfully updated content disposition setting.
+   *     content:
+   *       application/json:
+   *         schema:
+   *           type: object
+   *           properties:
+   *             mimeType:
+   *               type: string
+   *               description: The updated MIME type.
+   *             isInline:
+   *               type: boolean
+   *               description: The new disposition status (`true` for inline, `false` for attachment).
+   *             example:
+   *               mimeType: image/png
+   *               isInline: true
+   *   400:
+   *     $ref: '#/components/responses/400'
+   *   401:
+   *     $ref: '#/components/responses/401'
+   *   403:
+   *     $ref: '#/components/responses/403'
+   *   500:
+   *     $ref: '#/components/responses/500'
+   */
   router.put('/:mimeType(*)',
     loginRequiredStrictly,
     adminRequired,
     addActivity,
-    validator.updateContentDisposition, // Validate path and body
+    validator.updateContentDisposition,
     apiV3FormValidator,
     async(req, res) => {
-      const { mimeType } = req.params; // Get mimeType from URL path
-      const { isInline } = req.body; // Get isInline from request body
+      const { mimeType } = req.params;
+      const { isInline } = req.body;
 
       const configKey = `attachments:contentDisposition:${mimeType}:inline`;
 
@@ -80,15 +178,13 @@ module.exports = (crowi) => {
         // Retrieve the updated value to send back in the response (best practice)
         const updatedIsInline = await crowi.configManager.getConfig(configKey);
 
-        // Emit activity event for auditing
         const parameters = {
-          action: SupportedAction.ACTION_ADMIN_ATTACHMENT_DISPOSITION_UPDATE, // need to define this SupportedAction
+          action: SupportedAction.ACTION_ADMIN_ATTACHMENT_DISPOSITION_UPDATE,
           mimeType,
           isInline: updatedIsInline,
         };
         activityEvent.emit('update', res.locals.activity._id, parameters);
 
-        // Return success response
         return res.apiv3({ mimeType, isInline: updatedIsInline });
       }
 

+ 1 - 1
apps/app/src/server/routes/attachment/get.ts

@@ -112,7 +112,7 @@ const respondForRelayMode = async(crowi: Crowi, res: Response, fileUploadService
     attachment: IAttachmentDocument, opts?: RespondOptions): Promise<void> => {
   // apply content-* headers before response
   const isDownload = opts?.download ?? false;
-  const contentHeaders = new ContentHeaders(crowi.configManager as ConfigManager, attachment, { inline: !isDownload });
+  const contentHeaders = await ContentHeaders.create(crowi.configManager as ConfigManager, attachment, { inline: !isDownload });
   applyHeaders(res, contentHeaders.toExpressHttpHeaders());
 
   try {

+ 3 - 3
apps/app/src/server/service/file-uploader/azure.ts

@@ -140,7 +140,7 @@ class AzureFileUploader extends AbstractFileUploader {
     const filePath = getFilePathOnStorage(attachment);
     const containerClient = await getContainerClient();
     const blockBlobClient: BlockBlobClient = containerClient.getBlockBlobClient(filePath);
-    const contentHeaders = new ContentHeaders(this.configManager, attachment);
+    const contentHeaders = await ContentHeaders.create(this.configManager, attachment);
 
     await blockBlobClient.uploadStream(readable, undefined, undefined, {
       blobHTTPHeaders: {
@@ -163,7 +163,7 @@ class AzureFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override respond(): void {
+  override respond(): Promise<void> {
     throw new Error('AzureFileUploader does not support ResponseMode.DELEGATE.');
   }
 
@@ -218,7 +218,7 @@ class AzureFileUploader extends AbstractFileUploader {
       const userDelegationKey = await blobServiceClient.getUserDelegationKey(startsOn, expiresOn);
 
       const isDownload = opts?.download ?? false;
-      const contentHeaders = new ContentHeaders(this.configManager, attachment, { inline: !isDownload });
+      const contentHeaders = await ContentHeaders.create(this.configManager, attachment, { inline: !isDownload });
 
       // https://github.com/Azure/azure-sdk-for-js/blob/d4d55f73/sdk/storage/storage-blob/src/ContainerSASPermissions.ts#L24
       // r:read, a:add, c:create, w:write, d:delete, l:list

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

@@ -41,7 +41,7 @@ export interface FileUploader {
   doCheckLimit(uploadFileSize: number, maxFileSize: number, totalLimit: number): Promise<ICheckLimitResult>,
   determineResponseMode(): ResponseMode,
   uploadAttachment(readable: Readable, attachment: IAttachmentDocument): Promise<void>,
-  respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): void,
+  respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): Promise<void>;
   findDeliveryFile(attachment: IAttachmentDocument): Promise<NodeJS.ReadableStream>,
   generateTemporaryUrl(attachment: IAttachmentDocument, opts?: RespondOptions): Promise<TemporaryUrl>,
   createMultipartUploader: (uploadKey: string, maxPartSize: number) => MultipartUploader,
@@ -177,7 +177,7 @@ export abstract class AbstractFileUploader implements FileUploader {
   /**
    * Respond to the HTTP request.
    */
-  abstract respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): void;
+  abstract respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): Promise<void>;
 
   /**
    * Find the file and Return ReadStream

+ 2 - 2
apps/app/src/server/service/file-uploader/gridfs.ts

@@ -65,7 +65,7 @@ class GridfsFileUploader extends AbstractFileUploader {
   override async uploadAttachment(readable: Readable, attachment: IAttachmentDocument): Promise<void> {
     logger.debug(`File uploading: fileName=${attachment.fileName}`);
 
-    const contentHeaders = new ContentHeaders(configManager, attachment);
+    const contentHeaders = await ContentHeaders.create(configManager, attachment);
 
     return AttachmentFile.promisifiedWrite(
       {
@@ -80,7 +80,7 @@ class GridfsFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override respond(): void {
+  override respond(): Promise<void> {
     throw new Error('GridfsFileUploader does not support ResponseMode.DELEGATE.');
   }
 

+ 4 - 4
apps/app/src/server/service/file-uploader/local.ts

@@ -83,7 +83,7 @@ class LocalFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): void {
+  override respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): Promise<void> {
     throw new Error('Method not implemented.');
   }
 
@@ -221,7 +221,7 @@ module.exports = function(crowi: Crowi) {
    * @param {Response} res
    * @param {Response} attachment
    */
-  lib.respond = function(res, attachment, opts) {
+  lib.respond = async function(res, attachment, opts) {
     // Responce using internal redirect of nginx or Apache.
     const storagePath = getFilePathOnStorage(attachment);
     const relativePath = path.relative(crowi.publicDir, storagePath);
@@ -229,14 +229,14 @@ module.exports = function(crowi: Crowi) {
     const internalPath = urljoin(internalPathRoot, relativePath);
 
     const isDownload = opts?.download ?? false;
-    const contentHeaders = new ContentHeaders(configManager, attachment, { inline: !isDownload });
+    const contentHeaders = await ContentHeaders.create(configManager,attachment, { inline: !isDownload });
     applyHeaders(res, [
       ...contentHeaders.toExpressHttpHeaders(),
       { field: 'X-Accel-Redirect', value: internalPath },
       { field: 'X-Sendfile', value: storagePath },
     ]);
 
-    return res.end();
+    res.end();
   };
 
   /**

+ 25 - 18
apps/app/src/server/service/file-uploader/utils/headers.ts

@@ -21,74 +21,80 @@ export class ContentHeaders implements IContentHeaders {
 
   xContentTypeOptions?: ExpressHttpHeader<'X-Content-Type-Options'>;
 
-  private configManager: ConfigManager;
+  private configManager: ConfigManager; // Now explicitly typed and assigned in constructor
 
-  constructor(
-      configManager: ConfigManager,
+  // Refactored: Make constructor private and have it accept configManager
+  private constructor(configManager: ConfigManager) {
+    this.configManager = configManager;
+  }
+
+  // --- Factory Method (remains async) ---
+  static async create(
+      configManager: ConfigManager, // This parameter is now passed to the private constructor
       attachment: IAttachmentDocument,
       opts?: {
       inline?: boolean,
     },
-  ) {
-    this.configManager = configManager;
+  ): Promise<ContentHeaders> {
+    // Create instance, passing the configManager to the private constructor
+    const instance = new ContentHeaders(configManager);
 
     const attachmentContentType = attachment.fileFormat;
     const filename = attachment.originalName;
 
-    // Define the final content type value in a local variable.
     const actualContentTypeString: string = attachmentContentType || 'application/octet-stream';
 
-    this.contentType = {
+    instance.contentType = {
       field: 'Content-Type',
       value: actualContentTypeString,
     };
 
-    // Determine Content-Disposition based on allowlist and the 'inline' request flag
     const requestedInline = opts?.inline ?? false;
-
     const configKey = `attachments:contentDisposition:${actualContentTypeString}:inline` as ConfigKey;
 
-    // Get current inline setting
-    const rawConfigValue = this.configManager.getConfig(configKey);
+    // AWAIT the config value here
+    const rawConfigValue = await instance.configManager.getConfig(configKey); // Use instance's configManager
 
     let isConfiguredInline: boolean;
     if (typeof rawConfigValue === 'boolean') {
       isConfiguredInline = rawConfigValue;
     }
-
     else {
       isConfiguredInline = DEFAULT_ALLOWLIST_MIME_TYPES.has(actualContentTypeString);
     }
 
-    // Final decision
     const shouldBeInline = requestedInline
       && isConfiguredInline
       && SAFE_INLINE_CONFIGURABLE_MIME_TYPES.has(actualContentTypeString);
 
-    this.contentDisposition = {
+    console.log(shouldBeInline);
+    console.log(`Should be inline for ${attachmentContentType}: ${shouldBeInline}`); // Enhanced log
+
+    instance.contentDisposition = {
       field: 'Content-Disposition',
       value: shouldBeInline
         ? 'inline'
         : `attachment;filename*=UTF-8''${encodeURIComponent(filename)}`,
     };
 
-    this.contentSecurityPolicy = {
+    instance.contentSecurityPolicy = {
       field: 'Content-Security-Policy',
-      // eslint-disable-next-line max-len
       value: "script-src 'unsafe-hashes'; style-src 'self' 'unsafe-inline'; object-src 'none'; require-trusted-types-for 'script'; media-src 'self'; default-src 'none';",
     };
 
-    this.xContentTypeOptions = {
+    instance.xContentTypeOptions = {
       field: 'X-Content-Type-Options',
       value: 'nosniff',
     };
 
     if (attachment.fileSize) {
-      this.contentLength = {
+      instance.contentLength = {
         field: 'Content-Length',
         value: attachment.fileSize.toString(),
       };
     }
+
+    return instance;
   }
 
   /**
@@ -116,6 +122,7 @@ export const toExpressHttpHeaders = (records: Record<string, string | string[]>)
 };
 
 export const applyHeaders = (res: Response, headers: ExpressHttpHeader[]): void => {
+  console.log('Applying headers:', headers);
   headers.forEach((header) => {
     res.header(header.field, header.value);
   });