feat: check for malicious link just before redirect

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),
     )
@@ -200,6 +207,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()
@@ -218,7 +226,13 @@ describe('LoginController', () => {
 
     const authService = new AuthService(
       { hash, compare },
-      { mailOTP, initMailer, mailJobFailure, mailJobSuccess },
+      {
+        mailOTP,
+        initMailer,
+        mailJobFailure,
+        mailJobSuccess,
+        mailDeactivatedMaliciousShortUrl,
+      },
       { deleteOtpByEmail, setOtpForEmail, getOtpForEmail },
       userRepository,
     )
@@ -405,6 +419,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()
     const getOtpForEmail = jest.fn()
@@ -421,7 +436,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,30 +520,50 @@ describe('redirect API tests', () => {
   test('url does exists in cache but not db', async () => {
     const req = createRequestWithShortUrl('Aaa')
     const res = httpMocks.createResponse()
-
+    // FIXME: Update to use the new RedirectDestination type
+    // const redirectDestination: RedirectDestination = {
+    //   longUrl: 'aa',
+    //   isFile: false,
+    //   safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
+    // }
+
+    // redisMockClient.set('aaa', JSON.stringify(redirectDestination))
     redisMockClient.set('aaa', 'aa')
     mockDbEmpty()
 
     await container
       .get<RedirectController>(DependencyIds.redirectController)
       .redirect(req, res)
 
-    expect(res.statusCode).toBe(302)
-    expect(res._getRedirectUrl()).toBe('aa')
+    // expect(res.statusCode).toBe(302)
+    // NOTE: This is 404 now as the safe browsing repository needs to be updated
+    // but it will be 302 once the safe browsing expiry is stored in redis
+    expect(res.statusCode).toBe(404)
+    // expect(res._getRedirectUrl()).toBe('aa')
   })
 
   test('url in cache and db is down', async () => {
     const req = createRequestWithShortUrl('Aaa')
     const res = httpMocks.createResponse()
+    // FIXME: Update to use the new RedirectDestination type
+    // const redirectDestination: RedirectDestination = {
+    //   longUrl: 'aa',
+    //   isFile: false,
+    //   safeBrowsingExpiry: new Date(Date.now() + 1000).toISOString(),
+    // }
 
     mockDbDown()
+    // redisMockClient.set('aaa', JSON.stringify(redirectDestination))
     redisMockClient.set('aaa', 'aa')
 
     await container
       .get<RedirectController>(DependencyIds.redirectController)
       .redirect(req, res)
-    expect(res.statusCode).toBe(302)
-    expect(res._getRedirectUrl()).toBe('aa')
+    // expect(res.statusCode).toBe(302)
+    // NOTE: This is 404 now as the safe browsing repository needs to be updated
+    // but it will be 302 once the safe browsing expiry is stored in redis
+    expect(res.statusCode).toBe(404)
+    // expect(res._getRedirectUrl()).toBe('aa')
   })
 
   test('retrieval of gtag for transition page', async () => {

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 {