refactor attachment routes

apps/app/src/server/routes/attachment/attachment.js → apps/app/src/server/routes/attachment/api.js

@@ -133,7 +133,7 @@ const ApiResponse = require('../../util/apiResponse');
  *            example: "/download/5e0734e072560e001761fa67"
  */
 
-export const attachmentRoutesFactory = (crowi) => {
+export const routesFactory = (crowi) => {
   const Page = crowi.model('Page');
   const User = crowi.model('User');
   const GlobalNotificationSetting = crowi.model('GlobalNotificationSetting');
@@ -141,20 +141,6 @@ export const attachmentRoutesFactory = (crowi) => {
 
   const activityEvent = crowi.event('activity');
 
-  /**
-   * Check the user is accessible to the related page
-   *
-   * @param {User} user
-   * @param {Attachment} attachment
-   */
-  async function isAccessibleByViewer(user, attachment) {
-    if (attachment.page != null) {
-      // eslint-disable-next-line no-return-await
-      return await Page.isAccessiblePageByViewer(attachment.page, user);
-    }
-    return true;
-  }
-
   /**
    * Check the user is accessible to the related page
    *
@@ -188,16 +174,6 @@ export const attachmentRoutesFactory = (crowi) => {
   async function responseForAttachment(req, res, attachment, forceDownload) {
     const { fileUploadService } = crowi;
 
-    if (attachment == null) {
-      return res.json(ApiResponse.error('attachment not found'));
-    }
-
-    const user = req.user;
-    const isAccessible = await isAccessibleByViewer(user, attachment);
-    if (!isAccessible) {
-      return res.json(ApiResponse.error(`Forbidden to access to the attachment '${attachment.id}'. This attachment might belong to other pages.`));
-    }
-
     // add headers before evaluating 'req.fresh'
     setHeaderToRes(res, attachment, forceDownload);
 

apps/app/src/server/routes/attachment/get.ts

@@ -0,0 +1,46 @@
+import { getIdForRef, type IPage, type IUser } from '@growi/core';
+import type { NextFunction, Request, Response } from 'express';
+import mongoose from 'mongoose';
+
+import type { CrowiProperties } from '~/interfaces/crowi-request';
+import { Attachment, IAttachmentDocument } from '~/server/models';
+import ApiResponse from '~/server/util/apiResponse';
+
+
+interface PageModel {
+  isAccessiblePageByViewer: (pageId: string, user: IUser | undefined) => Promise<boolean>
+}
+
+type Req = CrowiProperties & Request<
+  { id: string },
+  undefined, undefined, undefined,
+  { attachment?: IAttachmentDocument }
+>;
+
+type AttachmentReq = Request<
+  undefined,
+  undefined, undefined, undefined,
+  { attachment: IAttachmentDocument }
+>;
+
+export const validateGetRequest = async(req: Req, res: Response, next: NextFunction): Promise<Response|void> => {
+  const id = req.params.id;
+  const attachment = await Attachment.findById(id);
+
+  if (attachment == null) {
+    return res.json(ApiResponse.error('attachment not found'));
+  }
+
+  const user = req.user;
+
+  // check viewer has permission
+  if (user != null && attachment.page != null) {
+    const Page = mongoose.model<IPage, PageModel>('Page');
+    const isAccessible = await Page.isAccessiblePageByViewer(getIdForRef(attachment.page), user);
+    if (!isAccessible) {
+      return res.json(ApiResponse.error(`Forbidden to access to the attachment '${attachment.id}'. This attachment might belong to other pages.`));
+    }
+  }
+
+  return next();
+};

apps/app/src/server/routes/attachment/index.ts

@@ -1 +1 @@
-export * from './attachment';
+export * from './get';

apps/app/src/server/routes/index.js

@@ -15,7 +15,8 @@ import {
   generateUnavailableWhenMaintenanceModeMiddleware, generateUnavailableWhenMaintenanceModeMiddlewareForApi,
 } from '../middlewares/unavailable-when-maintenance-mode';
 
-import { attachmentRoutesFactory } from './attachment';
+import * as attachment from './attachment';
+import { routesFactory as attachmentApiRoutesFactory } from './attachment/api';
 import * as forgotPassword from './forgot-password';
 import nextFactory from './next';
 import * as userActivation from './user-activation';
@@ -44,7 +45,7 @@ module.exports = function(crowi, app) {
   const loginPassport = require('./login-passport')(crowi, app);
   const me = require('./me')(crowi, app);
   const admin = require('./admin')(crowi, app);
-  const attachment = attachmentRoutesFactory(crowi);
+  const attachmentApi = attachmentApiRoutesFactory(crowi).api;
   const comment = require('./comment')(crowi, app);
   const tag = require('./tag')(crowi, app);
   const search = require('./search')(crowi, app);
@@ -110,7 +111,7 @@ module.exports = function(crowi, app) {
   app.post('/_api/admin/import/qiita'           , loginRequiredStrictly , adminRequired , csrfProtection, addActivity, admin.api.importDataFromQiita);
   app.post('/_api/admin/import/testQiitaAPI'    , loginRequiredStrictly , adminRequired , csrfProtection, addActivity, admin.api.testQiitaAPI);
 
-  app.get('/attachment/brand-logo' , certifyBrandLogo, loginRequired, attachment.api.getBrandLogo);
+  app.get('/attachment/brand-logo' , certifyBrandLogo, loginRequired, attachmentApi.getBrandLogo);
 
   /*
    * Routes below are unavailable when maintenance mode
@@ -144,11 +145,11 @@ module.exports = function(crowi, app) {
   apiV1Router.post('/comments.update'    , comment.api.validators.add(), accessTokenParser , loginRequiredStrictly , excludeReadOnlyUser, addActivity, comment.api.update);
   apiV1Router.post('/comments.remove'    , accessTokenParser , loginRequiredStrictly , excludeReadOnlyUser, addActivity, comment.api.remove);
 
-  apiV1Router.post('/attachments.add'                  , uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly , excludeReadOnlyUser, addActivity ,attachment.api.add);
-  apiV1Router.post('/attachments.uploadProfileImage'   , uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly , excludeReadOnlyUser, attachment.api.uploadProfileImage);
-  apiV1Router.post('/attachments.remove'               , accessTokenParser , loginRequiredStrictly , excludeReadOnlyUser, addActivity ,attachment.api.remove);
-  apiV1Router.post('/attachments.removeProfileImage'   , accessTokenParser , loginRequiredStrictly , excludeReadOnlyUser, attachment.api.removeProfileImage);
-  apiV1Router.get('/attachments.limit'   , accessTokenParser , loginRequiredStrictly, attachment.api.limit);
+  apiV1Router.post('/attachments.add'                  , uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly , excludeReadOnlyUser, addActivity , attachmentApi.add);
+  apiV1Router.post('/attachments.uploadProfileImage'   , uploads.single('file'), autoReap, accessTokenParser, loginRequiredStrictly , excludeReadOnlyUser, attachmentApi.uploadProfileImage);
+  apiV1Router.post('/attachments.remove'               , accessTokenParser , loginRequiredStrictly , excludeReadOnlyUser, addActivity ,attachmentApi.remove);
+  apiV1Router.post('/attachments.removeProfileImage'   , accessTokenParser , loginRequiredStrictly , excludeReadOnlyUser, attachmentApi.removeProfileImage);
+  apiV1Router.get('/attachments.limit'   , accessTokenParser , loginRequiredStrictly, attachmentApi.limit);
 
   // API v1
   app.use('/_api', unavailableWhenMaintenanceModeForApi, apiV1Router);
@@ -157,9 +158,9 @@ module.exports = function(crowi, app) {
 
   app.get('/me'                                   , loginRequiredStrictly, next.delegateToNext);
   app.get('/me/*'                                 , loginRequiredStrictly, next.delegateToNext);
-  app.get('/attachment/:id([0-9a-z]{24})'         , certifySharedPageAttachmentMiddleware , loginRequired, attachment.api.get);
-  app.get('/attachment/profile/:id([0-9a-z]{24})' , loginRequired, attachment.api.get);
-  app.get('/download/:id([0-9a-z]{24})'         , certifySharedPageAttachmentMiddleware, loginRequired, attachment.api.download);
+  app.get('/attachment/:id([0-9a-z]{24})'         , certifySharedPageAttachmentMiddleware , loginRequired, attachment.validateGetRequest, attachmentApi.get);
+  app.get('/attachment/profile/:id([0-9a-z]{24})' , loginRequired, attachment.validateGetRequest, attachmentApi.get);
+  app.get('/download/:id([0-9a-z]{24})'         , certifySharedPageAttachmentMiddleware, loginRequired, attachment.validateGetRequest, attachmentApi.download);
 
   app.get('/_search'                            , loginRequired, next.delegateToNext);