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

Merge branch 'dev/7.1.x' into feat/openai-vector-searching

Yuki Takei 1 год назад
Родитель
Сommit
bc5ae8fd2e

+ 28 - 16
apps/app/src/server/routes/ogp.ts

@@ -1,6 +1,7 @@
 import * as fs from 'fs';
 import path from 'path';
 
+import { getIdStringForRef, type IUser } from '@growi/core';
 import { DevidedPagePath } from '@growi/core/dist/models';
 // eslint-disable-next-line no-restricted-imports
 import axios from 'axios';
@@ -9,11 +10,15 @@ import type {
 } from 'express';
 import type { ValidationError } from 'express-validator';
 import { param, validationResult } from 'express-validator';
+import type { HydratedDocument } from 'mongoose';
+import mongoose from 'mongoose';
 
 import loggerFactory from '~/utils/logger';
 import { projectRoot } from '~/utils/project-dir-utils';
 
 import { Attachment } from '../models/attachment';
+import type { PageDocument, PageModel } from '../models/page';
+import { configManager } from '../service/config-manager';
 import { convertStreamToBuffer } from '../util/stream';
 
 const logger = loggerFactory('growi:routes:ogp');
@@ -56,21 +61,27 @@ module.exports = function(crowi) {
 
   const renderOgp = async(req: Request, res: Response) => {
 
-    const { configManager } = crowi;
+    const User = mongoose.model<IUser>('User');
     const ogpUri = configManager.getConfig('crowi', 'app:ogpUri');
-    const page = req.body.page;
+    const page: PageDocument = req.body.page; // asserted by ogpValidator
 
-    let user;
-    let pageTitle: string;
-    let bufferedUserImage: Buffer;
+    const title = (new DevidedPagePath(page.path)).latter;
 
-    try {
-      const User = crowi.model('User');
-      user = await User.findById(page.creator._id.toString());
+    let user: IUser | null = null;
+    let userName = '(unknown)';
+    let userImage: Buffer = bufferedDefaultUserImageCache;
 
-      bufferedUserImage = user.imageUrlCached === DEFAULT_USER_IMAGE_URL ? bufferedDefaultUserImageCache : (await getBufferedUserImage(user.imageUrlCached));
-      // todo: consider page title
-      pageTitle = (new DevidedPagePath(page.path)).latter;
+    try {
+      if (page.creator != null) {
+        user = await User.findById(getIdStringForRef(page.creator));
+
+        if (user != null) {
+          userName = user.username;
+          userImage = user.imageUrlCached !== DEFAULT_USER_IMAGE_URL
+            ? bufferedDefaultUserImageCache
+            : await getBufferedUserImage(user.imageUrlCached);
+        }
+      }
     }
     catch (err) {
       logger.error(err);
@@ -82,9 +93,9 @@ module.exports = function(crowi) {
       result = await axios.post(
         ogpUri, {
           data: {
-            title: pageTitle,
-            userName: user.username,
-            userImage: bufferedUserImage,
+            title,
+            userName,
+            userImage,
           },
         }, {
           responseType: 'stream',
@@ -118,9 +129,10 @@ module.exports = function(crowi) {
 
     if (errors.isEmpty()) {
 
+      const Page = mongoose.model<HydratedDocument<PageDocument>, PageModel>('Page');
+
       try {
-        const Page = crowi.model('Page');
-        const page = await Page.findByIdAndViewer(req.params.pageId);
+        const page = await Page.findByIdAndViewer(req.params.pageId, null);
 
         if (page == null || page.status !== Page.STATUS_PUBLISHED || (page.grant !== Page.GRANT_PUBLIC && page.grant !== Page.GRANT_RESTRICTED)) {
           return res.status(400).send('the page does not exist');

+ 1 - 1
packages/remark-attachment-refs/package.json

@@ -52,7 +52,7 @@
     "hast-util-select": "^6.0.2",
     "express": "^4.20.0",
     "mongoose": "^6.11.3",
-    "swr": "^2.0.3",
+    "swr": "^2.2.2",
     "universal-bunyan": "^0.9.2",
     "xss": "^1.0.15"
   },

+ 3 - 1
packages/remark-lsx/package.json

@@ -38,9 +38,11 @@
     "@growi/ui": "link:../ui",
     "escape-string-regexp": "^4.0.0",
     "express": "^4.20.0",
+    "express-validator": "^6.14.0",
     "http-errors": "^2.0.0",
     "mongoose": "^6.11.3",
-    "swr": "^2.2.2"
+    "swr": "^2.2.2",
+    "xss": "^1.0.15"
   },
   "devDependencies": {
     "eslint-plugin-regex": "^1.8.0",

+ 39 - 2
packages/remark-lsx/src/server/index.ts

@@ -1,4 +1,8 @@
-import type { Request, Response } from 'express';
+import type { NextFunction, Request, Response } from 'express';
+import { query, validationResult } from 'express-validator';
+import { FilterXSS } from 'xss';
+
+import type { LsxApiOptions } from '../interfaces/api';
 
 import { listPages } from './routes/list-pages';
 
@@ -6,12 +10,45 @@ const loginRequiredFallback = (req: Request, res: Response) => {
   return res.status(403).send('login required');
 };
 
+const filterXSS = new FilterXSS();
+
+const lsxValidator = [
+  query('pagePath').notEmpty().isString(),
+  query('offset').optional().isInt(),
+  query('limit').optional().isInt(),
+  query('options')
+    .optional()
+    .customSanitizer((options) => {
+      try {
+        const jsonData: LsxApiOptions = JSON.parse(options);
+
+        Object.keys(jsonData).forEach((key) => {
+          jsonData[key] = filterXSS.process(jsonData[key]);
+        });
+
+        return jsonData;
+      }
+      catch (err) {
+        throw new Error('Invalid JSON format in options');
+      }
+    }),
+  query('options.*').optional().isString(),
+];
+
+const paramValidator = (req: Request, _: Response, next: NextFunction) => {
+  const errObjArray = validationResult(req);
+  if (errObjArray.isEmpty()) {
+    return next();
+  }
+  return new Error('Invalid lsx parameter');
+};
+
 // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
 const middleware = (crowi: any, app: any): void => {
   const loginRequired = crowi.require('../middlewares/login-required')(crowi, true, loginRequiredFallback);
   const accessTokenParser = crowi.require('../middlewares/access-token-parser')(crowi);
 
-  app.get('/_api/lsx', accessTokenParser, loginRequired, listPages);
+  app.get('/_api/lsx', accessTokenParser, loginRequired, lsxValidator, paramValidator, listPages);
 };
 
 export default middleware;

+ 8 - 5
packages/remark-lsx/src/server/routes/list-pages/index.spec.ts

@@ -3,12 +3,15 @@ import type { Request, Response } from 'express';
 import createError from 'http-errors';
 import { mock } from 'vitest-mock-extended';
 
-import type { LsxApiResponseData } from '../../../interfaces/api';
+import type { LsxApiResponseData, LsxApiParams } from '../../../interfaces/api';
 
 import type { PageQuery, PageQueryBuilder } from './generate-base-query';
 
 import { listPages } from '.';
 
+interface IListPagesRequest extends Request<undefined, undefined, undefined, LsxApiParams> {
+  user: IUser,
+}
 
 // mocking modules
 const mocks = vi.hoisted(() => {
@@ -30,7 +33,7 @@ describe('listPages', () => {
 
   it("returns 400 HTTP response when the query 'pagePath' is undefined", async() => {
     // setup
-    const reqMock = mock<Request & { user: IUser }>();
+    const reqMock = mock<IListPagesRequest>();
     const resMock = mock<Response>();
     const resStatusMock = mock<Response>();
     resMock.status.calledWith(400).mockReturnValue(resStatusMock);
@@ -46,7 +49,7 @@ describe('listPages', () => {
 
   describe('with num option', () => {
 
-    const reqMock = mock<Request & { user: IUser }>();
+    const reqMock = mock<IListPagesRequest>();
     reqMock.query = { pagePath: '/Sandbox' };
 
     const builderMock = mock<PageQueryBuilder>();
@@ -97,7 +100,7 @@ describe('listPages', () => {
 
     it('returns 500 HTTP response when an unexpected error occured', async() => {
       // setup
-      const reqMock = mock<Request & { user: IUser }>();
+      const reqMock = mock<IListPagesRequest>();
       reqMock.query = { pagePath: '/Sandbox' };
 
       // an Error instance will be thrown by addNumConditionMock
@@ -124,7 +127,7 @@ describe('listPages', () => {
 
     it('returns 400 HTTP response when the value is invalid', async() => {
       // setup
-      const reqMock = mock<Request & { user: IUser }>();
+      const reqMock = mock<IListPagesRequest>();
       reqMock.query = { pagePath: '/Sandbox' };
 
       // an http-errors instance will be thrown by addNumConditionMock

+ 10 - 7
packages/remark-lsx/src/server/routes/list-pages/index.ts

@@ -56,20 +56,23 @@ function addExceptCondition(query, pagePath, optionsFilter): PageQuery {
   return addFilterCondition(query, pagePath, optionsFilter, true);
 }
 
+interface IListPagesRequest extends Request<undefined, undefined, undefined, LsxApiParams> {
+  user: IUser,
+}
+
 
-export const listPages = async(req: Request & { user: IUser }, res: Response): Promise<Response> => {
+export const listPages = async(req: IListPagesRequest, res: Response): Promise<Response> => {
   const user = req.user;
 
-  // TODO: use express-validator
   if (req.query.pagePath == null) {
-    return res.status(400).send("The 'pagePath' query must not be null.");
+    return res.status(400).send("the 'pagepath' query must not be null.");
   }
 
   const params: LsxApiParams = {
-    pagePath: removeTrailingSlash(req.query.pagePath.toString()),
-    offset: req.query?.offset != null ? Number(req.query.offset) : undefined,
-    limit: req.query?.limit != null ? Number(req.query?.limit) : undefined,
-    options: req.query?.options != null ? JSON.parse(req.query.options.toString()) : {},
+    pagePath: removeTrailingSlash(req.query.pagePath),
+    offset: req.query?.offset,
+    limit: req.query?.limit,
+    options: req.query?.options ?? {},
   };
 
   const {

+ 2 - 2
yarn.lock

@@ -2228,7 +2228,7 @@
     express "^4.20.0"
     hast-util-select "^6.0.2"
     mongoose "^6.11.3"
-    swr "^2.0.3"
+    swr "^2.2.2"
     universal-bunyan "^0.9.2"
     xss "^1.0.15"
 
@@ -17607,7 +17607,7 @@ swagger2openapi@^7.0.8:
     yaml "^1.10.0"
     yargs "^17.0.1"
 
-swr@^2.0.3, swr@^2.2.2:
+swr@^2.2.2:
   version "2.2.4"
   resolved "https://registry.yarnpkg.com/swr/-/swr-2.2.4.tgz#03ec4c56019902fbdc904d78544bd7a9a6fa3f07"
   integrity sha512-njiZ/4RiIhoOlAaLYDqwz5qH/KZXVilRLvomrx83HjzCWTfa+InyfAjv05PSFxnmLzZkNO9ZfvgoqzAaEI4sGQ==