Bläddra i källkod

feat: add middleware to reject requests when link sharing is disabled
refactor: remove validateShareLink functionality and related tests

Shun Miyazawa 4 dagar sedan
förälder
incheckning
faec9f7ac2

+ 27 - 0
apps/app/src/server/middlewares/reject-link-sharing-disabled.ts

@@ -0,0 +1,27 @@
+import { ErrorV3 } from '@growi/core/dist/models';
+import type { RequestHandler } from 'express';
+
+import { configManager } from '~/server/service/config-manager';
+
+import type { ApiV3Response } from '../routes/apiv3/interfaces/apiv3-response';
+
+/**
+ * Middleware that rejects requests when link sharing is globally disabled.
+ * Place before certifySharedPage to skip unnecessary DB access.
+ */
+export const rejectLinkSharingDisabled: RequestHandler = (
+  _req,
+  res: ApiV3Response,
+  next,
+) => {
+  const disableLinkSharing = configManager.getConfig(
+    'security:disableLinkSharing',
+  );
+  if (disableLinkSharing) {
+    return res.apiv3Err(
+      new ErrorV3('Link sharing is disabled', 'link-sharing-disabled'),
+      403,
+    );
+  }
+  return next();
+};

+ 24 - 39
apps/app/src/server/routes/apiv3/page/get-page-by-share-link.ts

@@ -4,10 +4,9 @@ import { query } from 'express-validator';
 
 
 import type Crowi from '~/server/crowi';
 import type Crowi from '~/server/crowi';
 import { apiV3FormValidator } from '~/server/middlewares/apiv3-form-validator';
 import { apiV3FormValidator } from '~/server/middlewares/apiv3-form-validator';
-import ShareLink from '~/server/models/share-link';
+import { rejectLinkSharingDisabled } from '~/server/middlewares/reject-link-sharing-disabled';
 import { configManager } from '~/server/service/config-manager';
 import { configManager } from '~/server/service/config-manager';
 import { findPageAndMetaDataByViewer } from '~/server/service/page/find-page-and-meta-data-by-viewer';
 import { findPageAndMetaDataByViewer } from '~/server/service/page/find-page-and-meta-data-by-viewer';
-import { validateShareLink } from '~/server/service/share-link';
 import loggerFactory from '~/utils/logger';
 import loggerFactory from '~/utils/logger';
 
 
 import type { ApiV3Response } from '../interfaces/apiv3-response';
 import type { ApiV3Response } from '../interfaces/apiv3-response';
@@ -20,7 +19,14 @@ type ReqQuery = {
   shareLinkId: string;
   shareLinkId: string;
 };
 };
 
 
-type Req = Request<Record<string, string>, ApiV3Response, undefined, ReqQuery>;
+type Req = Request<
+  Record<string, string>,
+  ApiV3Response,
+  undefined,
+  ReqQuery
+> & {
+  isSharedPage?: boolean;
+};
 
 
 /**
 /**
  * @swagger
  * @swagger
@@ -61,8 +67,10 @@ export const getPageByShareLinkHandlerFactory = (
   crowi: Crowi,
   crowi: Crowi,
 ): RequestHandler[] => {
 ): RequestHandler[] => {
   const { pageService, pageGrantService } = crowi;
   const { pageService, pageGrantService } = crowi;
+  const certifySharedPage = require('../../../middlewares/certify-shared-page')(
+    crowi,
+  );
 
 
-  // Define validators for req.query - both parameters required
   const validator = [
   const validator = [
     query('shareLinkId').isMongoId().withMessage('shareLinkId is required'),
     query('shareLinkId').isMongoId().withMessage('shareLinkId is required'),
     query('pageId').isMongoId().withMessage('pageId is required'),
     query('pageId').isMongoId().withMessage('pageId is required'),
@@ -71,44 +79,22 @@ export const getPageByShareLinkHandlerFactory = (
   return [
   return [
     ...validator,
     ...validator,
     apiV3FormValidator,
     apiV3FormValidator,
+    rejectLinkSharingDisabled,
+    certifySharedPage,
     async (req: Req, res: ApiV3Response) => {
     async (req: Req, res: ApiV3Response) => {
-      const { pageId, shareLinkId } = req.query;
-
-      try {
-        // First gate: Check if link sharing is enabled globally
-        const disableLinkSharing = configManager.getConfig(
-          'security:disableLinkSharing',
-        );
-        if (disableLinkSharing) {
-          return res.apiv3Err(
-            new ErrorV3('Link sharing is disabled', 'link-sharing-disabled'),
-            403,
-          );
-        }
+      const { pageId } = req.query;
 
 
-        // Validate ShareLink by ID and page ID in a single query
-        const validationResult = await validateShareLink(
-          ShareLink,
-          shareLinkId,
-          pageId,
+      if (!req.isSharedPage) {
+        return res.apiv3Err(
+          new ErrorV3(
+            'Share link is not found or has expired',
+            'share-link-invalid',
+          ),
+          404,
         );
         );
+      }
 
 
-        if (validationResult.type === 'not-found') {
-          return res.apiv3Err(
-            new ErrorV3('Share link not found', 'share-link-not-found'),
-            404,
-          );
-        }
-
-        if (validationResult.type === 'expired') {
-          return res.apiv3Err(
-            new ErrorV3('Share link has expired', 'share-link-expired'),
-            403,
-          );
-        }
-
-        // ShareLink is valid - fetch page data
-        // No user context for share link access - null user for public access
+      try {
         const pageWithMeta = await findPageAndMetaDataByViewer(
         const pageWithMeta = await findPageAndMetaDataByViewer(
           pageService,
           pageService,
           pageGrantService,
           pageGrantService,
@@ -120,7 +106,6 @@ export const getPageByShareLinkHandlerFactory = (
           },
           },
         );
         );
 
 
-        // Send response with proper status codes and permission restrictions
         const disableUserPages = configManager.getConfig(
         const disableUserPages = configManager.getConfig(
           'security:disableUserPages',
           'security:disableUserPages',
         );
         );

+ 0 - 2
apps/app/src/server/service/share-link/index.ts

@@ -1,2 +0,0 @@
-export type { ValidateShareLinkResult } from './validate-share-link';
-export { validateShareLink } from './validate-share-link';

+ 0 - 107
apps/app/src/server/service/share-link/validate-share-link.spec.ts

@@ -1,107 +0,0 @@
-import type { HydratedDocument } from 'mongoose';
-import { beforeEach, describe, expect, it, vi } from 'vitest';
-
-import type { ShareLinkDocument } from '~/server/models/share-link';
-
-import { validateShareLink } from './validate-share-link';
-
-describe('validateShareLink', () => {
-  const mockShareLinkId = '507f1f77bcf86cd799439011';
-  const mockPageId = '507f1f77bcf86cd799439012';
-
-  describe('success case', () => {
-    it('should return success result when ShareLink exists, relatedPage matches, and is not expired', async () => {
-      // Arrange
-      const mockShareLink = {
-        _id: mockShareLinkId,
-        relatedPage: mockPageId,
-        isExpired: () => false,
-      } as unknown as HydratedDocument<ShareLinkDocument>;
-
-      const mockFindOne = vi.fn().mockResolvedValue(mockShareLink);
-      const mockShareLinkModel = { findOne: mockFindOne } as any;
-
-      // Act
-      const result = await validateShareLink(
-        mockShareLinkModel,
-        mockShareLinkId,
-        mockPageId,
-      );
-
-      // Assert
-      expect(result.type).toBe('success');
-      if (result.type === 'success') {
-        expect(result.shareLink).toEqual(mockShareLink);
-      }
-      expect(mockFindOne).toHaveBeenCalledWith({
-        _id: mockShareLinkId,
-        relatedPage: mockPageId,
-      });
-    });
-  });
-
-  describe('not-found case', () => {
-    it('should return not-found result when ShareLink does not exist', async () => {
-      // Arrange
-      const mockFindOne = vi.fn().mockResolvedValue(null);
-      const mockShareLinkModel = { findOne: mockFindOne } as any;
-
-      // Act
-      const result = await validateShareLink(
-        mockShareLinkModel,
-        mockShareLinkId,
-        mockPageId,
-      );
-
-      // Assert
-      expect(result.type).toBe('not-found');
-    });
-
-    it('should return not-found result when relatedPage does not match', async () => {
-      // Arrange
-      const anotherPageId = '507f1f77bcf86cd799439099';
-      const mockShareLink = {
-        _id: mockShareLinkId,
-        relatedPage: anotherPageId,
-        isExpired: () => false,
-      } as unknown as HydratedDocument<ShareLinkDocument>;
-
-      const mockFindOne = vi.fn().mockResolvedValue(null);
-      const mockShareLinkModel = { findOne: mockFindOne } as any;
-
-      // Act
-      const result = await validateShareLink(
-        mockShareLinkModel,
-        mockShareLinkId,
-        mockPageId,
-      );
-
-      // Assert
-      expect(result.type).toBe('not-found');
-    });
-  });
-
-  describe('expired case', () => {
-    it('should return expired result when ShareLink is expired', async () => {
-      // Arrange
-      const mockShareLink = {
-        _id: mockShareLinkId,
-        relatedPage: mockPageId,
-        isExpired: () => true,
-      } as unknown as HydratedDocument<ShareLinkDocument>;
-
-      const mockFindOne = vi.fn().mockResolvedValue(mockShareLink);
-      const mockShareLinkModel = { findOne: mockFindOne } as any;
-
-      // Act
-      const result = await validateShareLink(
-        mockShareLinkModel,
-        mockShareLinkId,
-        mockPageId,
-      );
-
-      // Assert
-      expect(result.type).toBe('expired');
-    });
-  });
-});

+ 0 - 47
apps/app/src/server/service/share-link/validate-share-link.ts

@@ -1,47 +0,0 @@
-import type { HydratedDocument } from 'mongoose';
-
-import type {
-  ShareLinkDocument,
-  ShareLinkModel,
-} from '~/server/models/share-link';
-
-export type ValidateShareLinkResult =
-  | { type: 'success'; shareLink: HydratedDocument<ShareLinkDocument> }
-  | { type: 'not-found' }
-  | { type: 'expired' };
-
-/**
- * Validate a ShareLink by ID and related page ID.
- *
- * Performs a single database query to check for existence and page matching,
- * then evaluates expiration status.
- *
- * @param shareLinkModel - The ShareLink Mongoose model
- * @param shareLinkId - The ShareLink ID to validate
- * @param pageId - The related page ID to match
- * @returns A discriminated union indicating validation result
- */
-export async function validateShareLink(
-  shareLinkModel: ShareLinkModel,
-  shareLinkId: string,
-  pageId: string,
-): Promise<ValidateShareLinkResult> {
-  // Query with both _id and relatedPage for single-pass validation
-  // Use $eq to force literal comparisons for untrusted inputs.
-  const shareLink = await shareLinkModel.findOne({
-    _id: { $eq: shareLinkId },
-    relatedPage: { $eq: pageId },
-  });
-
-  // Handle not found or page mismatch
-  if (shareLink == null) {
-    return { type: 'not-found' };
-  }
-
-  // Check if expired
-  if (shareLink.isExpired()) {
-    return { type: 'expired' };
-  }
-
-  return { type: 'success', shareLink };
-}