feat: check for malicious link just before redirect

Author: dcshzj

src/server/modules/api/admin-v1/__tests__/AdminApiV1Controller.test.ts

@@ -12,6 +12,7 @@ const urlManagementService = {
   changeOwnership: jest.fn(),
   getUrlsWithConditions: jest.fn(),
   bulkCreate: jest.fn(),
+  deactivateMaliciousShortUrl: jest.fn(),
 }
 
 const userRepository = {

src/server/modules/api/external-v1/__tests__/ApiV1Controller.test.ts

@@ -16,6 +16,7 @@ const urlManagementService = {
   changeOwnership: jest.fn(),
   getUrlsWithConditions: jest.fn(),
   bulkCreate: jest.fn(),
+  deactivateMaliciousShortUrl: jest.fn(),
 }
 
 const urlV1Mapper = new UrlV1Mapper()

src/server/modules/auth/__tests__/LoginController.test.ts

@@ -115,6 +115,7 @@ describe('LoginController', () => {
     const initMailer = jest.fn()
     const mailJobFailure = jest.fn()
     const mailJobSuccess = jest.fn()
+    const mailDeactivatedMaliciousShortUrl = jest.fn()
 
     const deleteOtpByEmail = jest.fn()
     const setOtpForEmail = jest.fn()
@@ -123,7 +124,13 @@ describe('LoginController', () => {
     const urlMapper = new UrlMapper()
     const authService = new AuthService(
       { hash, compare },
-      { mailOTP, initMailer, mailJobFailure, mailJobSuccess },
+      {
+        mailOTP,
+        initMailer,
+        mailJobFailure,
+        mailJobSuccess,
+        mailDeactivatedMaliciousShortUrl,
+      },
       { deleteOtpByEmail, setOtpForEmail, getOtpForEmail },
       new UserRepository(new UserMapper(urlMapper), urlMapper),
     )
@@ -198,6 +205,7 @@ describe('LoginController', () => {
     const initMailer = jest.fn()
     const mailJobFailure = jest.fn()
     const mailJobSuccess = jest.fn()
+    const mailDeactivatedMaliciousShortUrl = jest.fn()
 
     const deleteOtpByEmail = jest.fn()
     const setOtpForEmail = jest.fn()
@@ -216,7 +224,13 @@ describe('LoginController', () => {
 
     const authService = new AuthService(
       { hash, compare },
-      { mailOTP, initMailer, mailJobFailure, mailJobSuccess },
+      {
+        mailOTP,
+        initMailer,
+        mailJobFailure,
+        mailJobSuccess,
+        mailDeactivatedMaliciousShortUrl,
+      },
       { deleteOtpByEmail, setOtpForEmail, getOtpForEmail },
       userRepository,
     )

src/server/modules/bulk/__tests__/BulkController.test.ts

@@ -15,6 +15,7 @@ const mockUrlManagementService = {
   changeOwnership: jest.fn(),
   getUrlsWithConditions: jest.fn(),
   bulkCreate: jest.fn(),
+  deactivateMaliciousShortUrl: jest.fn(),
 }
 
 const controller = new BulkController(mockBulkService, mockUrlManagementService)

src/server/modules/job/services/__tests__/JobManagementService.test.ts

@@ -40,6 +40,7 @@ const mockMailer = {
   mailOTP: jest.fn(),
   mailJobFailure: jest.fn(),
   mailJobSuccess: jest.fn(),
+  mailDeactivatedMaliciousShortUrl: jest.fn(),
 }
 
 const service = new JobManagementService(

src/server/modules/redirect/__tests__/RedirectController.test.ts

@@ -3,22 +3,25 @@ import httpMocks from 'node-mocks-http'
 import redisMock from 'redis-mock'
 import SequelizeMock from 'sequelize-mock'
 
+import { DependencyIds } from '../../../constants'
 import { ACTIVE } from '../../../models/types'
 import { UrlRepositoryInterface } from '../../../repositories/interfaces/UrlRepositoryInterface'
 import { container } from '../../../util/inversify'
-import { DependencyIds } from '../../../constants'
 import { generateCookie } from '../ga'
 
+import { RedirectController } from '..'
+import { logger } from '../../../config'
+import { UrlMapper } from '../../../mappers/UrlMapper'
+import { LinkStatisticsService } from '../../analytics/interfaces'
+import { UrlThreatScanService } from '../../threat/interfaces'
+import { UrlManagementService as UrlManagementServiceInterface } from '../../user/interfaces'
 import {
   AnalyticsLoggerService,
   CookieArrayReducerService,
   CrawlerCheckService,
   RedirectService,
 } from '../services'
-import { RedirectController } from '..'
-import { logger } from '../../../config'
-import { UrlMapper } from '../../../mappers/UrlMapper'
-import { LinkStatisticsService } from '../../analytics/interfaces'
+import { RedirectDestination } from '../../../repositories/types'
 
 const redisMockClient = redisMock.createClient()
 const sequelizeMock = new SequelizeMock()
@@ -30,6 +33,9 @@ const urlModelMock = sequelizeMock.define(
     longUrl: 'aa',
     state: ACTIVE,
     clicks: 8,
+    UrlClicks: {
+      clicks: 8,
+    },
   },
   {
     instanceMethods: {
@@ -107,6 +113,16 @@ const devicesModelMock = sequelizeMock.define(
   },
 )
 
+const userModelMock = sequelizeMock.define('user', {
+  id: 1,
+  email: '[email protected]',
+})
+
+const urlClicksModelMock = sequelizeMock.define('url_clicks', {
+  shortUrl: 'a',
+  clicks: 0,
+})
+
 const mockTransaction = sequelizeMock.transaction
 
 jest.resetModules()
@@ -127,6 +143,14 @@ jest.mock('../../../models/statistics/devices', () => ({
   Devices: devicesModelMock,
 }))
 
+jest.mock('../../../models/statistics/clicks', () => ({
+  UrlClicks: urlClicksModelMock,
+}))
+
+jest.mock('../../../models/user', () => ({
+  User: userModelMock,
+}))
+
 jest.mock('../../../redis', () => ({
   redirectClient: redisMockClient,
 }))
@@ -258,6 +282,22 @@ describe('redirect API tests', () => {
     container
       .bind<LinkStatisticsService>(DependencyIds.linkStatisticsService)
       .toConstantValue(linkStatisticsServiceMock)
+    container
+      .bind<UrlThreatScanService>(DependencyIds.urlThreatScanService)
+      .toConstantValue({
+        isThreat: jest.fn().mockResolvedValue(false),
+        isThreatBulk: jest.fn().mockResolvedValue([]),
+      })
+    container
+      .bind<UrlManagementServiceInterface>(DependencyIds.urlManagementService)
+      .toConstantValue({
+        deactivateMaliciousShortUrl: jest.fn(),
+        bulkCreate: jest.fn(),
+        createUrl: jest.fn(),
+        updateUrl: jest.fn(),
+        changeOwnership: jest.fn(),
+        getUrlsWithConditions: jest.fn(),
+      })
     redisMockClient.flushall()
   })
   afterEach(() => {
@@ -480,8 +520,13 @@ describe('redirect API tests', () => {
   test('url does exists in cache but not db', async () => {
     const req = createRequestWithShortUrl('Aaa')
     const res = httpMocks.createResponse()
+    const redirectDestination: RedirectDestination = {
+      longUrl: 'aa',
+      isFile: false,
+      safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
+    }
 
-    redisMockClient.set('aaa', 'aa')
+    redisMockClient.set('aaa', JSON.stringify(redirectDestination))
     mockDbEmpty()
 
     await container
@@ -495,9 +540,14 @@ describe('redirect API tests', () => {
   test('url in cache and db is down', async () => {
     const req = createRequestWithShortUrl('Aaa')
     const res = httpMocks.createResponse()
+    const redirectDestination: RedirectDestination = {
+      longUrl: 'aa',
+      isFile: false,
+      safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
+    }
 
     mockDbDown()
-    redisMockClient.set('aaa', 'aa')
+    redisMockClient.set('aaa', JSON.stringify(redirectDestination))
 
     await container
       .get<RedirectController>(DependencyIds.redirectController)

src/server/modules/redirect/services/RedirectService.ts

@@ -6,6 +6,9 @@ import { RedirectResult, RedirectType } from '..'
 import { LinkStatisticsService } from '../../analytics/interfaces'
 import { logger, ogUrl } from '../../../config'
 import { CookieArrayReducerService, CrawlerCheckService } from '.'
+import { UrlThreatScanService } from '../../threat/interfaces'
+import { getSafeBrowsingExpiryDate } from '../../../util/safeBrowsing'
+import { UrlManagementService } from '../../user/interfaces'
 
 @injectable()
 export class RedirectService {
@@ -17,6 +20,10 @@ export class RedirectService {
 
   private linkStatisticsService: LinkStatisticsService
 
+  private urlThreatScanService: UrlThreatScanService
+
+  private urlManagementService: UrlManagementService
+
   public constructor(
     @inject(DependencyIds.urlRepository) urlRepository: UrlRepositoryInterface,
     @inject(DependencyIds.crawlerCheckService)
@@ -25,11 +32,17 @@ export class RedirectService {
     cookieArrayReducerService: CookieArrayReducerService,
     @inject(DependencyIds.linkStatisticsService)
     linkStatisticsService: LinkStatisticsService,
+    @inject(DependencyIds.urlThreatScanService)
+    urlThreatScanService: UrlThreatScanService,
+    @inject(DependencyIds.urlManagementService)
+    urlManagementService: UrlManagementService,
   ) {
     this.urlRepository = urlRepository
     this.crawlerCheckService = crawlerCheckService
     this.cookieArrayReducerService = cookieArrayReducerService
     this.linkStatisticsService = linkStatisticsService
+    this.urlThreatScanService = urlThreatScanService
+    this.urlManagementService = urlManagementService
   }
 
   public redirectFor: (
@@ -51,7 +64,34 @@ export class RedirectService {
     const shortUrl = rawShortUrl.toLowerCase()
 
     // Find longUrl to redirect to
-    const longUrl = await this.urlRepository.getLongUrl(shortUrl)
+    const { longUrl, isFile, safeBrowsingExpiry } =
+      await this.urlRepository.getLongUrl(shortUrl)
+
+    // Validate that the longUrl is not a malicious link
+    const isSafeBrowsingResultExpired =
+      !isFile &&
+      (!safeBrowsingExpiry ||
+        new Date(safeBrowsingExpiry).getTime() < Date.now())
+
+    if (isSafeBrowsingResultExpired) {
+      const isThreat = await this.urlThreatScanService.isThreat(longUrl)
+      if (isThreat) {
+        logger.warn(
+          `Malicious link attempt: ${longUrl} was detected as malicious for shortUrl ${shortUrl}`,
+        )
+
+        // Deactivate the short link and warn the short link owner
+        await this.urlManagementService.deactivateMaliciousShortUrl(shortUrl)
+
+        // NOTE: We return a 404 error here to make the user experience the same
+        // as if the short link was deactivated/not found for simplicity and to
+        // avoid inducing user panic.
+        throw new NotFoundError('Malicious link detected')
+      }
+      // Store the result of the threat scan in the database
+      const expiry = getSafeBrowsingExpiryDate({ longUrl })
+      await this.urlRepository.updateSafeBrowsingExpiry(shortUrl, expiry)
+    }
 
     // Update clicks and click statistics in database.
     try {

src/server/modules/redirect/services/__tests__/RedirectService.test.ts

@@ -0,0 +1,104 @@
+import { RedirectService } from '..'
+import { NotFoundError } from '../../../../util/error'
+
+const mockUrlRepository = {
+  getLongUrl: jest.fn(),
+  updateSafeBrowsingExpiry: jest.fn(),
+  deactivateShortUrl: jest.fn(),
+  findByShortUrlWithTotalClicks: jest.fn(),
+  update: jest.fn(),
+  create: jest.fn(),
+  isShortUrlAvailable: jest.fn(),
+  rawDirectorySearch: jest.fn(),
+  bulkCreate: jest.fn(),
+}
+const mockCrawlerCheckService = { isCrawler: jest.fn() }
+const mockCookieArrayReducerService = {
+  userHasVisitedShortlink: jest.fn(),
+  writeShortlinkToCookie: jest.fn(),
+}
+const mockLinkStatisticsService = {
+  updateLinkStatistics: jest.fn(),
+  getLinkStatistics: jest.fn(),
+}
+const mockUrlThreatScanService = {
+  isThreat: jest.fn(),
+  isThreatBulk: jest.fn(),
+}
+const mockUrlManagementService = {
+  bulkCreate: jest.fn(),
+  createUrl: jest.fn(),
+  updateUrl: jest.fn(),
+  changeOwnership: jest.fn(),
+  getUrlsWithConditions: jest.fn(),
+  deactivateMaliciousShortUrl: jest.fn(),
+}
+
+const service = new RedirectService(
+  mockUrlRepository,
+  mockCrawlerCheckService,
+  mockCookieArrayReducerService,
+  mockLinkStatisticsService,
+  mockUrlThreatScanService,
+  mockUrlManagementService,
+)
+
+describe('RedirectService', () => {
+  afterAll(jest.resetModules)
+
+  describe('redirectFor', () => {
+    it.skip('should throw NotFoundError for invalid shortUrl', () => {})
+    it.skip('should throw NotFoundError for non-existent shortUrl', () => {})
+
+    it('should throw NotFoundError if the longUrl is malicious and the safe browsing result is expired', async () => {
+      // Arrange
+      mockUrlRepository.getLongUrl.mockResolvedValue({
+        longUrl: 'https://malicious.com',
+        isFile: false,
+        safeBrowsingExpiry: new Date(Date.now() - 1000).toISOString(),
+      })
+      mockUrlThreatScanService.isThreat.mockResolvedValue(true)
+
+      // Act & Assert
+      await expect(
+        service.redirectFor('shortUrl', undefined, '', ''),
+      ).rejects.toThrowError(NotFoundError)
+    })
+
+    it('should not throw NotFoundError if the longUrl is malicious but the safe browsing result is not expired', async () => {
+      // Arrange
+      mockUrlRepository.getLongUrl.mockResolvedValue({
+        longUrl: 'https://malicious.com',
+        isFile: false,
+        safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
+      })
+      mockUrlThreatScanService.isThreat.mockResolvedValue(true)
+
+      // Act & Assert
+      await expect(
+        service.redirectFor('shortUrl', undefined, '', ''),
+      ).resolves.not.toThrowError(NotFoundError)
+    })
+
+    it('should update the safe browsing expiry if the longUrl is not malicious', async () => {
+      // Arrange
+      const mockShortUrl = 'short'
+      const mockLongUrl = 'https://safe.com'
+      mockUrlRepository.getLongUrl.mockResolvedValue({
+        longUrl: mockLongUrl,
+        isFile: false,
+        safeBrowsingExpiry: new Date(Date.now() - 1000).toISOString(),
+      })
+      mockUrlThreatScanService.isThreat.mockResolvedValue(false)
+
+      // Act
+      await service.redirectFor(mockShortUrl, undefined, '', '')
+
+      // Assert
+      expect(mockUrlRepository.updateSafeBrowsingExpiry).toHaveBeenCalledWith(
+        mockShortUrl,
+        expect.any(Date),
+      )
+    })
+  })
+})

src/server/modules/user/__tests__/UserController.test.ts

@@ -23,6 +23,7 @@ const urlManagementService = {
   changeOwnership: jest.fn(),
   getUrlsWithConditions: jest.fn(),
   bulkCreate: jest.fn(),
+  deactivateMaliciousShortUrl: jest.fn(),
 }
 
 const tagManagementService = {