Yuki Takei пре 2 година
родитељ
комит
0d00b698b2

+ 1 - 1
apps/app/src/server/middlewares/certify-shared-file/validate-referer/retrieve-site-url.ts

@@ -16,7 +16,7 @@ export const retrieveSiteUrl = (): URL | null => {
     return new URL(siteUrlString);
     return new URL(siteUrlString);
   }
   }
   catch (err) {
   catch (err) {
-    logger.error("The 'app:siteUrl' is invalid");
+    logger.error(`Parsing 'app:siteUrl' ('${siteUrlString}') has failed.`);
     return null;
     return null;
   }
   }
 };
 };

+ 55 - 8
apps/app/src/server/middlewares/certify-shared-file/validate-referer/validate-referer.spec.ts

@@ -1,3 +1,5 @@
+import { objectIdUtils } from '@growi/core/dist/utils';
+
 import { validateReferer } from './validate-referer';
 import { validateReferer } from './validate-referer';
 
 
 const mocks = vi.hoisted(() => {
 const mocks = vi.hoisted(() => {
@@ -11,6 +13,12 @@ vi.mock('./retrieve-site-url', () => ({ retrieveSiteUrl: mocks.retrieveSiteUrlMo
 
 
 describe('validateReferer', () => {
 describe('validateReferer', () => {
 
 
+  const isValidObjectIdSpy = vi.spyOn(objectIdUtils, 'isValidObjectId');
+
+  beforeEach(() => {
+    isValidObjectIdSpy.mockClear();
+  });
+
   describe('refurns false', () => {
   describe('refurns false', () => {
 
 
     it('when the referer argument is undefined', () => {
     it('when the referer argument is undefined', () => {
@@ -22,6 +30,7 @@ describe('validateReferer', () => {
       // then
       // then
       expect(result).toBeFalsy();
       expect(result).toBeFalsy();
       expect(mocks.retrieveSiteUrlMock).not.toHaveBeenCalled();
       expect(mocks.retrieveSiteUrlMock).not.toHaveBeenCalled();
+      expect(isValidObjectIdSpy).not.toHaveBeenCalled();
     });
     });
 
 
     it('when the referer is invalid', () => {
     it('when the referer is invalid', () => {
@@ -31,6 +40,7 @@ describe('validateReferer', () => {
       // then
       // then
       expect(result).toBeFalsy();
       expect(result).toBeFalsy();
       expect(mocks.retrieveSiteUrlMock).not.toHaveBeenCalledOnce();
       expect(mocks.retrieveSiteUrlMock).not.toHaveBeenCalledOnce();
+      expect(isValidObjectIdSpy).not.toHaveBeenCalled();
     });
     });
 
 
     it('when the siteUrl returns null', () => {
     it('when the siteUrl returns null', () => {
@@ -46,29 +56,66 @@ describe('validateReferer', () => {
       // then
       // then
       expect(result).toBeFalsy();
       expect(result).toBeFalsy();
       expect(mocks.retrieveSiteUrlMock).toHaveBeenCalledOnce();
       expect(mocks.retrieveSiteUrlMock).toHaveBeenCalledOnce();
+      expect(isValidObjectIdSpy).not.toHaveBeenCalled();
+    });
+
+    it('when the hostname of the referer does not match with siteUrl', () => {
+      // setup
+      mocks.retrieveSiteUrlMock.mockImplementation(() => {
+        return new URL('https://example.com');
+      });
+
+      // when
+      const refererString = 'https://example.org/share/xxxxx';
+      const result = validateReferer(refererString);
+
+      // then
+      expect(result).toBeFalsy();
+      expect(mocks.retrieveSiteUrlMock).toHaveBeenCalledOnce();
+      expect(isValidObjectIdSpy).not.toHaveBeenCalled();
     });
     });
 
 
-    it('when the domain of the referer does not match with siteUrl', () => {
+    it('when the port of the referer does not match with siteUrl', () => {
       // setup
       // setup
-      const siteUrl = 'https://example.com';
-      mocks.configManagerMock.getConfig.mockImplementation(() => {
-        return siteUrl;
+      mocks.retrieveSiteUrlMock.mockImplementation(() => {
+        return new URL('https://example.com');
       });
       });
 
 
       // when
       // when
-      const shareLinkId = '65436ba09ae6983bd608b89c';
-      const refererString = `https://example.org/share/${shareLinkId}`;
+      const refererString = 'https://example.com:8080/share/xxxxx';
       const result = validateReferer(refererString);
       const result = validateReferer(refererString);
 
 
       // then
       // then
       expect(result).toBeFalsy();
       expect(result).toBeFalsy();
-      expect(mocks.configManagerMock.getConfig).toHaveBeenCalledWith('crowi', 'app:siteUrl');
-      expect(mocks.configManagerMock.getConfig).toHaveBeenCalledOnce();
+      expect(mocks.retrieveSiteUrlMock).toHaveBeenCalledOnce();
+      expect(isValidObjectIdSpy).not.toHaveBeenCalled();
+    });
+
+    it('when the shareLinkId is invalid', () => {
+      // setup
+      mocks.retrieveSiteUrlMock.mockImplementation(() => {
+        return new URL('https://example.com');
+      });
+
+      // when
+      const refererString = 'https://example.com/share/FFFFFFFFFFFFFFFFFFFFFFFF';
+      const result = validateReferer(refererString);
+
+      // then
+      expect(result).toBeFalsy();
+      expect(mocks.retrieveSiteUrlMock).toHaveBeenCalledOnce();
+      expect(isValidObjectIdSpy).toHaveBeenCalledOnce();
     });
     });
 
 
   });
   });
 
 
   it('returns ValidReferer instance', () => {
   it('returns ValidReferer instance', () => {
+    // setup
+    mocks.retrieveSiteUrlMock.mockImplementation(() => {
+      return new URL('https://example.com');
+    });
+
+
     // when
     // when
     const shareLinkId = '65436ba09ae6983bd608b89c';
     const shareLinkId = '65436ba09ae6983bd608b89c';
     const refererString = `https://example.com/share/${shareLinkId}`;
     const refererString = `https://example.com/share/${shareLinkId}`;

+ 33 - 5
apps/app/src/server/middlewares/certify-shared-file/validate-referer/validate-referer.ts

@@ -1,3 +1,5 @@
+import { objectIdUtils } from '@growi/core/dist/utils';
+
 import loggerFactory from '~/utils/logger';
 import loggerFactory from '~/utils/logger';
 
 
 import { ValidReferer } from '../interfaces';
 import { ValidReferer } from '../interfaces';
@@ -11,7 +13,7 @@ const logger = loggerFactory('growi:middlewares:certify-shared-file:validate-ref
 export const validateReferer = (referer: string | undefined): ValidReferer | false => {
 export const validateReferer = (referer: string | undefined): ValidReferer | false => {
   // not null
   // not null
   if (referer == null) {
   if (referer == null) {
-    logger.debug('referer string is undefined');
+    logger.info('The referer string is undefined');
     return false;
     return false;
   }
   }
 
 
@@ -20,22 +22,48 @@ export const validateReferer = (referer: string | undefined): ValidReferer | fal
     refererUrl = new URL(referer);
     refererUrl = new URL(referer);
   }
   }
   catch (err) {
   catch (err) {
-    logger.error("The 'app:siteUrl' is invalid");
+    logger.info(`Parsing referer ('${referer}') has failed`);
     return false;
     return false;
   }
   }
 
 
   // siteUrl
   // siteUrl
   const siteUrl = retrieveSiteUrl();
   const siteUrl = retrieveSiteUrl();
   if (siteUrl == null) {
   if (siteUrl == null) {
+    logger.info('The siteUrl is null.');
+    return false;
+  }
+
+  // validate hostname and port
+  if (refererUrl.hostname !== siteUrl.hostname || refererUrl.port !== siteUrl.port) {
+    logger.warn('The hostname or port mismatched.', {
+      refererUrl: {
+        hostname: refererUrl.hostname,
+        port: refererUrl.port,
+      },
+      siteUrl: {
+        hostname: siteUrl.hostname,
+        port: siteUrl.port,
+      },
+    });
     return false;
     return false;
   }
   }
 
 
-  // if (refererUrl.hostname !== )
+  // validate pathname
+  // https://regex101.com/r/M5Bp6E/1
+  const match = refererUrl.pathname.match(/^\/share\/(?<shareLinkId>[a-f0-9]{24})$/i);
+  if (match == null || match.groups?.shareLinkId == null) {
+    logger.warn(`The pathname ('${refererUrl.pathname}') is invalid.`, match);
+    return false;
+  }
 
 
-  // starts with /share/
+  // validate shareLinkId is an correct ObjectId
+  if (!objectIdUtils.isValidObjectId(match.groups.shareLinkId)) {
+    logger.warn(`The shareLinkId ('${match.groups.shareLinkId}') is invalid as an ObjectId.`);
+    return false;
+  }
 
 
   return {
   return {
     referer,
     referer,
-    shareLinkId,
+    shareLinkId: match.groups.shareLinkId,
   };
   };
 };
 };